diff --git a/agents-docs/LESSONS.md b/agents-docs/LESSONS.md index 3890b26..64270a8 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 +### Store clientInstanceId in sessionStorage not localStorage [realtime] [multi-device] + +- **Trigger:** same user logged in on two tabs, browsers, or synced profiles sees alternating "Disconnected from signaling server" and no cross-device chat/voice sync. +- **Rule:** persist `metoyou.clientInstanceId` in `sessionStorage` (one id per tab/window) and clear any legacy `localStorage` copy on first read. +- **Why:** server identify evicts stale sockets with the same `(oderId, connectionScope, clientInstanceId)` tuple; a shared localStorage id makes each client kick the other in a reconnect loop. +- **Example:** `ClientInstanceService.getClientInstanceId()` writes to `sessionStorage`; two tabs get different ids and stay connected simultaneously. + ### Revalidate IndexedDB scope without reinitializing on every read [persistence] [performance] - **Trigger:** `DatabaseService.ensureReady()` called `initialize()` before every delegated read/write to fix user-scope races. diff --git a/toju-app/CONTEXT.md b/toju-app/CONTEXT.md index 196aa82..0c5284e 100644 --- a/toju-app/CONTEXT.md +++ b/toju-app/CONTEXT.md @@ -24,7 +24,7 @@ Owns the user-facing Angular 21 desktop chat experience: rendering and orchestra | **Custom emoji** | User-created image emoji assets stored locally, synced peer-to-peer, and referenced from messages/reactions by stable `:emoji[id](name)` tokens. | "sticker", "emote" | | **App locale** | The active UI language for the product client, resolved by `resolveAppLocale()` in `core/i18n/`; only `en` is shipped today. | "language", "i18n locale" | | **Translation catalog** | JSON string tables under `public/i18n/catalog/*.json`, merged to `public/i18n/en.json` via `npm run i18n:sync`, loaded at startup by `AppI18nService`. | "locale file", "messages file" | -| **Client instance** | Stable per-install UUID (`metoyou.clientInstanceId`) sent on WebSocket `identify` and voice-state payloads so the signaling server can route multi-device sessions. | "device id", "session id" | +| **Client instance** | Stable per-tab UUID (`metoyou.clientInstanceId` in `sessionStorage`) sent on WebSocket `identify` and voice-state payloads so the signaling server can route multi-device sessions without evicting other tabs or synced profiles. | "device id", "session id" | | **Voice owner connection** | The single client instance whose `clientInstanceId` matches the user's active `voiceState.clientInstanceId` and therefore owns mic/WebRTC for that identity. | "active voice client" | ## Relationships diff --git a/toju-app/src/app/core/platform/client-instance.service.spec.ts b/toju-app/src/app/core/platform/client-instance.service.spec.ts index 06ab226..8487bc6 100644 --- a/toju-app/src/app/core/platform/client-instance.service.spec.ts +++ b/toju-app/src/app/core/platform/client-instance.service.spec.ts @@ -7,17 +7,27 @@ import { } from 'vitest'; import { ClientInstanceService } from './client-instance.service'; -const STORAGE_KEY = 'metoyou.clientInstanceId'; +const SESSION_STORAGE_KEY = 'metoyou.clientInstanceId'; +const LEGACY_LOCAL_STORAGE_KEY = 'metoyou.clientInstanceId'; describe('ClientInstanceService', () => { - const storage = new Map(); + const sessionStorage = new Map(); + const localStorage = new Map(); beforeEach(() => { - storage.clear(); + sessionStorage.clear(); + localStorage.clear(); + + vi.stubGlobal('sessionStorage', { + getItem: (key: string) => sessionStorage.get(key) ?? null, + setItem: (key: string, value: string) => { sessionStorage.set(key, value); }, + removeItem: (key: string) => { sessionStorage.delete(key); } + }); + vi.stubGlobal('localStorage', { - getItem: (key: string) => storage.get(key) ?? null, - setItem: (key: string, value: string) => { storage.set(key, value); }, - removeItem: (key: string) => { storage.delete(key); } + getItem: (key: string) => localStorage.get(key) ?? null, + setItem: (key: string, value: string) => { localStorage.set(key, value); }, + removeItem: (key: string) => { localStorage.delete(key); } }); }); @@ -25,19 +35,37 @@ describe('ClientInstanceService', () => { vi.unstubAllGlobals(); }); - it('creates and persists a stable client instance id', () => { + it('creates and persists a stable id for the same tab session', () => { const service = new ClientInstanceService(); const first = service.getClientInstanceId(); const second = new ClientInstanceService().getClientInstanceId(); expect(first).toMatch(/^[0-9a-f-]{36}$/i); expect(second).toBe(first); - expect(storage.get(STORAGE_KEY)).toBe(first); + expect(sessionStorage.get(SESSION_STORAGE_KEY)).toBe(first); }); - it('reuses a stored client instance id', () => { - storage.set(STORAGE_KEY, 'device-123'); + it('uses independent ids across separate tab sessions', () => { + sessionStorage.set(SESSION_STORAGE_KEY, 'tab-a'); - expect(new ClientInstanceService().getClientInstanceId()).toBe('device-123'); + const tabA = new ClientInstanceService().getClientInstanceId(); + + sessionStorage.clear(); + sessionStorage.set(SESSION_STORAGE_KEY, 'tab-b'); + + const tabB = new ClientInstanceService().getClientInstanceId(); + + expect(tabA).toBe('tab-a'); + expect(tabB).toBe('tab-b'); + }); + + it('does not reuse legacy localStorage ids that collide across tabs or synced browsers', () => { + localStorage.set(LEGACY_LOCAL_STORAGE_KEY, 'synced-device-id'); + + const id = new ClientInstanceService().getClientInstanceId(); + + expect(id).not.toBe('synced-device-id'); + expect(sessionStorage.get(SESSION_STORAGE_KEY)).toBe(id); + expect(localStorage.has(LEGACY_LOCAL_STORAGE_KEY)).toBe(false); }); }); diff --git a/toju-app/src/app/core/platform/client-instance.service.ts b/toju-app/src/app/core/platform/client-instance.service.ts index 32a8d6f..aed1c25 100644 --- a/toju-app/src/app/core/platform/client-instance.service.ts +++ b/toju-app/src/app/core/platform/client-instance.service.ts @@ -1,7 +1,14 @@ import { Injectable } from '@angular/core'; -const STORAGE_KEY = 'metoyou.clientInstanceId'; +const SESSION_STORAGE_KEY = 'metoyou.clientInstanceId'; +const LEGACY_LOCAL_STORAGE_KEY = 'metoyou.clientInstanceId'; +/** + * Stable id for this browser tab/window session. + * + * Stored in sessionStorage so multiple tabs or synced browser profiles do not + * share the same id and evict each other on the signaling server. + */ @Injectable({ providedIn: 'root' }) export class ClientInstanceService { private cachedId: string | null = null; @@ -18,9 +25,11 @@ export class ClientInstanceService { return stored; } + this.clearLegacyLocalStorageId(); + const created = crypto.randomUUID(); - localStorage.setItem(STORAGE_KEY, created); + this.writeStoredId(created); this.cachedId = created; return created; @@ -28,11 +37,27 @@ export class ClientInstanceService { private readStoredId(): string | null { try { - const raw = localStorage.getItem(STORAGE_KEY)?.trim(); + const raw = sessionStorage.getItem(SESSION_STORAGE_KEY)?.trim(); return raw || null; } catch { return null; } } + + private writeStoredId(id: string): void { + try { + sessionStorage.setItem(SESSION_STORAGE_KEY, id); + } catch { + // Ignore quota / private-mode failures; in-memory cache still works for this session. + } + } + + private clearLegacyLocalStorageId(): void { + try { + localStorage.removeItem(LEGACY_LOCAL_STORAGE_KEY); + } catch { + // Ignore storage access failures. + } + } } diff --git a/toju-app/src/app/domains/chat/domain/rules/message-revision.builder.rules.spec.ts b/toju-app/src/app/domains/chat/domain/rules/message-revision.builder.rules.spec.ts index bd43b77..8921d2a 100644 --- a/toju-app/src/app/domains/chat/domain/rules/message-revision.builder.rules.spec.ts +++ b/toju-app/src/app/domains/chat/domain/rules/message-revision.builder.rules.spec.ts @@ -55,7 +55,21 @@ describe('message-revision.builder.rules', () => { expect(revision.content).toBe('edited'); }); - it('materializes message state from a revision', async () => { + it('materializes create revisions without an editedAt label timestamp', async () => { + const revision = await buildMessageRevision({ + message: createMessage(), + type: 'create', + actorId: 'user-1', + editedAt: 1_000 + }); + const materialized = materializeMessageFromRevision(null, revision); + + expect(materialized.timestamp).toBe(1_000); + expect(materialized.editedAt).toBeUndefined(); + expect(materialized.revision).toBe(0); + }); + + it('materializes message state from an edit revision', async () => { const revision = await buildMessageRevision({ message: createMessage(), type: 'author-edit', @@ -67,6 +81,7 @@ describe('message-revision.builder.rules', () => { expect(materialized.revision).toBe(1); expect(materialized.content).toBe('edited'); + expect(materialized.editedAt).toBe(2_000); expect(materialized.headHash).toBe(revision.headHash); }); diff --git a/toju-app/src/app/domains/chat/domain/rules/message-revision.builder.rules.ts b/toju-app/src/app/domains/chat/domain/rules/message-revision.builder.rules.ts index d614786..4661871 100644 --- a/toju-app/src/app/domains/chat/domain/rules/message-revision.builder.rules.ts +++ b/toju-app/src/app/domains/chat/domain/rules/message-revision.builder.rules.ts @@ -85,7 +85,7 @@ export function materializeMessageFromRevision( senderId: revision.senderId, senderName: revision.senderName ?? base.senderName, content: revision.isDeleted ? DELETED_MESSAGE_CONTENT : (revision.content ?? base.content), - editedAt: revision.editedAt, + editedAt: revision.type === 'create' ? undefined : revision.editedAt, revision: revision.revision, headHash: revision.headHash, isDeleted: revision.isDeleted, diff --git a/toju-app/src/app/domains/chat/domain/rules/message.rules.spec.ts b/toju-app/src/app/domains/chat/domain/rules/message.rules.spec.ts new file mode 100644 index 0000000..61c3261 --- /dev/null +++ b/toju-app/src/app/domains/chat/domain/rules/message.rules.spec.ts @@ -0,0 +1,59 @@ +import { + describe, + it, + expect +} from 'vitest'; +import type { Message } from '../../../../shared-kernel'; +import { shouldShowMessageEditedLabel } from './message.rules'; + +function createMessage(overrides: Partial = {}): Message { + return { + id: 'message-1', + roomId: 'room-1', + senderId: 'user-1', + senderName: 'User 1', + content: 'hello', + timestamp: 1_000, + reactions: [], + isDeleted: false, + ...overrides + }; +} + +describe('message.rules', () => { + describe('shouldShowMessageEditedLabel', () => { + it('returns false for newly created messages without an edit revision', () => { + expect(shouldShowMessageEditedLabel(createMessage())).toBe(false); + }); + + it('returns false when editedAt equals the original timestamp (legacy create rows)', () => { + expect(shouldShowMessageEditedLabel(createMessage({ + editedAt: 1_000, + timestamp: 1_000, + revision: 0 + }))).toBe(false); + }); + + it('returns true when a message has an edit revision', () => { + expect(shouldShowMessageEditedLabel(createMessage({ + editedAt: 2_000, + revision: 1 + }))).toBe(true); + }); + + it('returns true for legacy edited messages with editedAt after timestamp', () => { + expect(shouldShowMessageEditedLabel(createMessage({ + editedAt: 2_000, + timestamp: 1_000 + }))).toBe(true); + }); + + it('returns false for deleted messages even when editedAt is set', () => { + expect(shouldShowMessageEditedLabel(createMessage({ + editedAt: 2_000, + revision: 1, + isDeleted: true + }))).toBe(false); + }); + }); +}); diff --git a/toju-app/src/app/domains/chat/domain/rules/message.rules.ts b/toju-app/src/app/domains/chat/domain/rules/message.rules.ts index b8c7fdb..d5ef433 100644 --- a/toju-app/src/app/domains/chat/domain/rules/message.rules.ts +++ b/toju-app/src/app/domains/chat/domain/rules/message.rules.ts @@ -1,10 +1,26 @@ import { DELETED_MESSAGE_CONTENT, type Message } from '../../../../shared-kernel'; +import { getMessageRevision } from './message-integrity.rules'; /** Extracts the effective timestamp from a message (editedAt takes priority). */ export function getMessageTimestamp(msg: Message): number { return msg.editedAt || msg.timestamp || 0; } +/** Whether the UI should show the "(edited)" label for a message. */ +export function shouldShowMessageEditedLabel( + message: Pick +): boolean { + if (message.isDeleted || message.editedAt === undefined) { + return false; + } + + if (getMessageRevision(message) > 0) { + return true; + } + + return typeof message.timestamp === 'number' && message.editedAt > message.timestamp; +} + /** Computes the most recent timestamp across a batch of messages. */ export function getLatestTimestamp(messages: Message[]): number { return messages.reduce((max, msg) => Math.max(max, getMessageTimestamp(msg)), 0); diff --git a/toju-app/src/app/domains/chat/feature/chat-messages/components/message-item/chat-message-item.component.html b/toju-app/src/app/domains/chat/feature/chat-messages/components/message-item/chat-message-item.component.html index e573218..ec1c228 100644 --- a/toju-app/src/app/domains/chat/feature/chat-messages/components/message-item/chat-message-item.component.html +++ b/toju-app/src/app/domains/chat/feature/chat-messages/components/message-item/chat-message-item.component.html @@ -69,7 +69,7 @@ >{{ msg.senderName }} {{ formatTimestamp(msg.timestamp) }} - @if (msg.editedAt && !msg.isDeleted) { + @if (showEditedLabel(msg)) { {{ 'chat.message.edited' | translate }} } diff --git a/toju-app/src/app/domains/chat/feature/chat-messages/components/message-item/chat-message-item.component.ts b/toju-app/src/app/domains/chat/feature/chat-messages/components/message-item/chat-message-item.component.ts index 3adc516..b0aa5c2 100644 --- a/toju-app/src/app/domains/chat/feature/chat-messages/components/message-item/chat-message-item.component.ts +++ b/toju-app/src/app/domains/chat/feature/chat-messages/components/message-item/chat-message-item.component.ts @@ -52,6 +52,7 @@ import { ExperimentalMediaSettingsService } from '../../../../../experimental-me import { ExperimentalVlcPlayerComponent } from '../../../../../experimental-media/feature/experimental-vlc-player/experimental-vlc-player.component'; import { KlipyService } from '../../../../application/services/klipy.service'; import { hasDedicatedChatEmbed } from '../../../../domain/rules/link-embed.rules'; +import { shouldShowMessageEditedLabel } from '../../../../domain/rules/message.rules'; import { AppI18nService, APP_TRANSLATE_IMPORTS } from '../../../../../../core/i18n'; import { Message, User } from '../../../../../../shared-kernel'; import { ThemeNodeDirective } from '../../../../../theme'; @@ -594,6 +595,10 @@ export class ChatMessageItemComponent implements OnDestroy { })); } + showEditedLabel(message: Message): boolean { + return shouldShowMessageEditedLabel(message); + } + formatTimestamp(timestamp: number): string { const date = new Date(timestamp); const now = new Date(); diff --git a/toju-app/src/app/infrastructure/realtime/README.md b/toju-app/src/app/infrastructure/realtime/README.md index 8b80274..d9dcd5f 100644 --- a/toju-app/src/app/infrastructure/realtime/README.md +++ b/toju-app/src/app/infrastructure/realtime/README.md @@ -164,9 +164,9 @@ The browser also sends a lightweight `keepalive` message on the signaling socket ### Server-side connection hygiene -Browsers do not reliably fire WebSocket close events during page refresh or navigation (especially Chromium). On `identify`, the server evicts stale sockets that share the same `(oderId, connectionScope, clientInstanceId)` tuple so a refreshed tab does not leave a zombie connection behind. +Browsers do not reliably fire WebSocket close events during page refresh or navigation (especially Chromium). On `identify`, the server evicts stale sockets that share the same `(oderId, connectionScope, clientInstanceId)` tuple so a refreshed tab does not leave a zombie connection behind. Each browser tab/window stores its own `clientInstanceId` in `sessionStorage` so multiple tabs or synced browser profiles do not share an id and evict each other in a reconnect loop. -Multi-device sessions keep **multiple** open connections for the same `oderId` (different `clientInstanceId` values). Server broadcasts exclude only the sending **connection id**, not the whole identity, so chat/typing/voice-state updates reach every logged-in device. Presence `user_joined` / `user_left` broadcasts still exclude the whole identity so other users never see duplicate join/leave events. +Multi-device sessions keep **multiple** open connections for the same `oderId` (different `clientInstanceId` values per tab/device). Server broadcasts exclude only the sending **connection id**, not the whole identity, so chat/typing/voice-state updates reach every logged-in device. Presence `user_joined` / `user_left` broadcasts still exclude the whole identity so other users never see duplicate join/leave events. RTC offers/answers/ICE are routed to the connection marked `voiceActive` for the target user (fallback: any open connection). Voice ownership is tracked per connection from `voice_state` payloads that include `clientInstanceId`.