From c3837fa32c6af1244bc2a2923a50b1e85137a528 Mon Sep 17 00:00:00 2001 From: J3vb <192430104+J3vb@users.noreply.github.com> Date: Fri, 14 Aug 2026 08:11:16 +0200 Subject: [PATCH] fix(client): batch of 15 client correctness fixes (#1367) * fix(client): 1 defect(s) (OC-0078) * fix(client): 1 defect(s) (OC-0147) * fix(client): 1 defect(s) (OC-0141) * fix(voice): 1 defect(s) (OC-0125) * fix(client): 1 defect(s) (OC-0138) * fix(client): 1 defect(s) (OC-0136) * fix(client): 1 defect(s) (OC-0121) * fix(voice): 1 defect(s) (OC-0132) * fix(client): 1 defect(s) (OC-0057) * fix(client): 1 defect(s) (OC-0122) * fix(client): 1 defect(s) (OC-0130) * fix(client): 1 defect(s) (OC-0060) * fix(client): 1 defect(s) (OC-0123) * fix(client): 1 defect(s) (OC-0124) * fix(ws): 1 defect(s) (OC-0056) --------- Co-authored-by: Claude --- .../src/components/MessageList.ts | 13 ++ .../src/components/SearchOverlay.ts | 30 +++-- .../src/components/settings/AdvancedTab.ts | 13 +- .../src/components/settings/VoiceAudioTab.ts | 8 +- Client/tauri-client/src/lib/api.ts | 13 +- .../src/lib/channel-navigation.ts | 38 +++++- .../tauri-client/src/lib/connectionStats.ts | 28 +++-- Client/tauri-client/src/lib/context-menu.ts | 41 ++++++- Client/tauri-client/src/lib/dispatcher.ts | 9 ++ .../tauri-client/src/lib/media-visibility.ts | 12 +- Client/tauri-client/src/lib/profiles.ts | 26 +++- Client/tauri-client/src/lib/read-state.ts | 25 +++- Client/tauri-client/src/lib/streamPreview.ts | 1 + Client/tauri-client/src/lib/ws.ts | 23 ++++ Client/tauri-client/src/styles/app.css | 8 ++ .../tests/unit/advanced-tab.test.ts | 101 ++++++++++++++++ Client/tauri-client/tests/unit/api.test.ts | 30 +++++ .../tests/unit/channel-navigation.test.ts | 38 ++++++ .../tests/unit/connection-stats.test.ts | 60 ++++++++++ .../tests/unit/context-menu.test.ts | 112 ++++++++++++++++++ .../tests/unit/dispatcher.test.ts | 47 ++++++++ .../tests/unit/media-visibility.test.ts | 20 ++++ .../tests/unit/message-list.test.ts | 42 +++++++ .../tauri-client/tests/unit/profiles.test.ts | 102 ++++++++++++++++ .../tests/unit/read-state.test.ts | 40 ++++++- .../tests/unit/search-overlay.test.ts | 55 +++++++++ .../tests/unit/status-picker-userbar.test.ts | 19 +++ .../tests/unit/stream-preview.test.ts | 60 ++++++++++ .../tests/unit/voice-audio-tab.test.ts | 26 ++++ .../tests/unit/ws-lifecycle.test.ts | 66 +++++++++++ 30 files changed, 1060 insertions(+), 46 deletions(-) diff --git a/Client/tauri-client/src/components/MessageList.ts b/Client/tauri-client/src/components/MessageList.ts index e73ca155..bc9612bf 100644 --- a/Client/tauri-client/src/components/MessageList.ts +++ b/Client/tauri-client/src/components/MessageList.ts @@ -657,6 +657,13 @@ export function createMessageList(options: MessageListOptions): MessageListCompo let renderAllRunning = false; let renderAllCount = 0; let renderAllResetTimer = 0; + // Set when the rapid-fire breaker below drops a renderAll() call on the + // floor. The store change that triggered the dropped call is still live — + // without this, the DOM is left showing pre-burst state until some later, + // unrelated store event happens to call renderAll() again. The 2s reset + // timeout checks this flag and issues one final renderAll() so the burst's + // last state always makes it to the screen. + let renderAllSuppressed = false; function renderAll(): void { if (root === null) return; @@ -667,12 +674,18 @@ export function createMessageList(options: MessageListOptions): MessageListCompo renderAllCount++; if (renderAllCount > 20) { log.error("[MessageList] renderAll called >20 times in 2s — breaking loop"); + renderAllSuppressed = true; return; } if (renderAllResetTimer === 0) { renderAllResetTimer = window.setTimeout(() => { renderAllCount = 0; renderAllResetTimer = 0; + if (renderAllSuppressed) { + // Render the burst's final state once, now that it's over. + renderAllSuppressed = false; + renderAll(); + } }, 2000); } diff --git a/Client/tauri-client/src/components/SearchOverlay.ts b/Client/tauri-client/src/components/SearchOverlay.ts index 8d48815e..42a1a9d8 100644 --- a/Client/tauri-client/src/components/SearchOverlay.ts +++ b/Client/tauri-client/src/components/SearchOverlay.ts @@ -93,15 +93,6 @@ export function createSearchOverlay(options: SearchOverlayOptions): MountableCom appendChildren(item, header, content); - item.addEventListener( - "click", - () => { - options.onSelectResult(r); - options.onClose(); - }, - { signal }, - ); - resultsDiv.appendChild(item); } } @@ -200,6 +191,26 @@ export function createSearchOverlay(options: SearchOverlayOptions): MountableCom } } + // Single delegated listener for the results container, registered once at + // mount time. renderResults() re-creates row elements on every search and + // on every arrow-key navigation, so binding a listener directly to each row + // would re-register (and never release) one abort algorithm per discarded + // row for the lifetime of the overlay. + function handleResultsClick(e: MouseEvent): void { + const target = e.target; + if (!(target instanceof Element)) return; + const row = target.closest(".search-result-item"); + if (!row) return; + const testId = row.getAttribute("data-testid"); + if (!testId) return; + const idx = Number(testId.slice("search-result-".length)); + const r = results[idx]; + if (r !== undefined) { + options.onSelectResult(r); + options.onClose(); + } + } + function mount(container: Element): void { root = createElement("div", { class: "search-overlay open", @@ -234,6 +245,7 @@ export function createSearchOverlay(options: SearchOverlayOptions): MountableCom input.addEventListener("input", handleInput, { signal }); input.addEventListener("keydown", handleKeydown, { signal }); root.addEventListener("click", handleBackdropClick, { signal }); + resultsDiv.addEventListener("click", handleResultsClick, { signal }); requestAnimationFrame(() => input.focus()); } diff --git a/Client/tauri-client/src/components/settings/AdvancedTab.ts b/Client/tauri-client/src/components/settings/AdvancedTab.ts index 9ff49cc9..b998576c 100644 --- a/Client/tauri-client/src/components/settings/AdvancedTab.ts +++ b/Client/tauri-client/src/components/settings/AdvancedTab.ts @@ -238,9 +238,15 @@ function buildAutostartRow(signal: AbortSignal): HTMLDivElement { // Starts off; corrected to the real OS state once the plugin answers. let enabled = false; + // Set as soon as the user interacts with the toggle. Guards the init + // read-back below so a slow `isEnabled()` resolving after the user has + // already flipped the switch can't clobber their change with a stale + // value (see OC-0141). + let touched = false; const toggle = createToggle(false, { signal, onChange: (nowOn) => { + touched = true; void (async () => { try { const { enable, disable } = await import("@tauri-apps/plugin-autostart"); @@ -261,7 +267,12 @@ function buildAutostartRow(signal: AbortSignal): HTMLDivElement { void (async () => { try { const { isEnabled } = await import("@tauri-apps/plugin-autostart"); - enabled = await isEnabled(); + const initialEnabled = await isEnabled(); + // If the user already toggled this before the read-back resolved, + // their change (and whatever it settles to) wins — don't overwrite it + // with the value read before that change was applied. + if (touched) return; + enabled = initialEnabled; toggle.classList.toggle("on", enabled); toggle.setAttribute("aria-checked", String(enabled)); } catch { diff --git a/Client/tauri-client/src/components/settings/VoiceAudioTab.ts b/Client/tauri-client/src/components/settings/VoiceAudioTab.ts index 735d894d..9a5ab48c 100644 --- a/Client/tauri-client/src/components/settings/VoiceAudioTab.ts +++ b/Client/tauri-client/src/components/settings/VoiceAudioTab.ts @@ -487,9 +487,11 @@ function buildVoiceAudioTabInner( startCameraPreview(savedVideoDevice); } - signal.addEventListener("abort", () => { - stopCameraPreview(); - }); + // Camera teardown on overlay close is already covered by the factory's + // single signal.addEventListener("abort", cleanupMic) — registering here + // too would add one more permanent listener (and retain this build's DOM + // subtree via closure) every time the tab is rebuilt, since `signal` is + // shared for the whole overlay lifetime, not per-build. // Start mic level monitoring for visual feedback void (async () => { diff --git a/Client/tauri-client/src/lib/api.ts b/Client/tauri-client/src/lib/api.ts index 75453c26..2502f645 100644 --- a/Client/tauri-client/src/lib/api.ts +++ b/Client/tauri-client/src/lib/api.ts @@ -79,7 +79,18 @@ const log = createLogger("api"); export function createApiClient(initialConfig: ApiClientConfig, onUnauthorized?: OnUnauthorized) { // oxlint-disable-next-line consistent-function-scoping -- co-located with createApiClient for encapsulation function isValidHost(host: string): boolean { - return /^[\w.-]+(:\d+)?$/.test(host) && host.length <= 253; + if (host.length > 253) return false; + // Bracketed IPv6 literal ("[::1]" or "[::1]:8443") — same convention as + // livekitSession.ts's ensureLiveKitProxy and http_proxy.rs / + // livekit_proxy.rs's validate_remote_host + parse_server_name. + if (/^\[[0-9A-Fa-f:.]+\](:\d+)?$/.test(host)) return true; + // Bare (unbracketed) IPv6 literal, e.g. "2001:db8::1" or "::1". More than + // one colon means the whole string is the address — a single colon is + // reserved for the host:port separator below, matching how + // ensureLiveKitProxy tells "[::1]:port" apart from "host:port". + if ((host.match(/:/g) ?? []).length > 1 && /^[0-9A-Fa-f:.]+$/.test(host)) return true; + // DNS name or IPv4 literal, optionally with a port. + return /^[\w.-]+(:\d+)?$/.test(host); } let config = { ...initialConfig }; diff --git a/Client/tauri-client/src/lib/channel-navigation.ts b/Client/tauri-client/src/lib/channel-navigation.ts index 529d43ee..a372f7aa 100644 --- a/Client/tauri-client/src/lib/channel-navigation.ts +++ b/Client/tauri-client/src/lib/channel-navigation.ts @@ -5,17 +5,34 @@ */ import { setActiveChannel, clearUnread, channelsStore } from "@stores/channels.store"; -import { clearDmUnread } from "@stores/dm.store"; +import { clearDmUnread, dmStore, dmDisplayName } from "@stores/dm.store"; +// lib -> pages import: addDmToChannelsStore is the only place that +// synthesizes a DM's channelsStore mirror row. dispatcher.ts already crosses +// this same boundary for exactly this reason (see its DM_CHANNEL_CLOSE +// handler) — a DM that `ready` reported in dmStore but that the user has not +// yet opened this session has no mirror row until one of these two call +// sites creates it. +import { addDmToChannelsStore } from "@pages/main-page/SidebarDmHelpers"; /** * Activate `channelId`, clearing its unread and mention badges. * - * No-op for an id the channel store does not know: the caller resolved a name - * that no longer exists, and blanking the active channel would be worse than - * staying put. + * A channel absent from channelsStore is not necessarily invisible to the + * user: a DM's row there is only synthesized on open (addDmToChannelsStore), + * while dmStore carries every DM the user is a member of from the moment + * `ready` lands. Fall back to dmStore and synthesize the mirror row so a + * jump (permalink, search hit, pinned, reply) into a DM the user has not + * clicked yet this session still lands, instead of degrading as if the + * channel did not exist. True no-op only when neither store has it: the + * caller resolved an id that no longer exists, and blanking the active + * channel would be worse than staying put. */ export function navigateToChannel(channelId: number): void { - if (!channelsStore.getState().channels.has(channelId)) return; + if (!channelsStore.getState().channels.has(channelId)) { + const dm = dmStore.getState().channels.find((c) => c.channelId === channelId); + if (dm === undefined) return; + addDmToChannelsStore(dm); + } setActiveChannel(channelId); clearUnread(channelId); // findChannelById does not filter out DM mirrors, so a jump (permalink, @@ -32,10 +49,19 @@ export function navigateToChannel(channelId: number): void { * than a name (message permalinks). Returns null when the channel is not in * this user's channel list — a permalink to somewhere they cannot see must * degrade quietly, not render a chip that goes nowhere. + * + * Falls back to dmStore when channelsStore has no row: a DM's channelsStore + * mirror is synthesized only on open (addDmToChannelsStore), but dmStore + * already knows every DM the user belongs to from `ready`. Without this, a + * jump into a DM never opened this session reads as "not visible" even + * though the user is a member and the server will happily serve its + * messages. */ export function findChannelById(channelId: number): { id: number; name: string } | null { const ch = channelsStore.getState().channels.get(channelId); - return ch === undefined ? null : { id: ch.id, name: ch.name }; + if (ch !== undefined) return { id: ch.id, name: ch.name }; + const dm = dmStore.getState().channels.find((c) => c.channelId === channelId); + return dm === undefined ? null : { id: dm.channelId, name: dmDisplayName(dm) }; } /** diff --git a/Client/tauri-client/src/lib/connectionStats.ts b/Client/tauri-client/src/lib/connectionStats.ts index b252a25a..b32aaa65 100644 --- a/Client/tauri-client/src/lib/connectionStats.ts +++ b/Client/tauri-client/src/lib/connectionStats.ts @@ -163,17 +163,27 @@ export function createConnectionStatsPoller(getRoom: () => Room | null): Connect listeners.forEach((cb) => cb(current)); - // Debounced quality change notification (prevents toast spam on flapping) + // Debounced quality change notification (prevents toast spam on flapping). + // Only arm the timer when none is already pending — POLL_INTERVAL_MS (2000) + // is shorter than QUALITY_DEBOUNCE_MS (3000), so re-arming on every poll + // that still disagrees with lastQuality would push the deadline out + // forever and the timer would never fire. If quality returns to + // lastQuality before the timer elapses, cancel it instead. const newQuality = current.quality; if (newQuality !== lastQuality) { - if (qualityDebounceTimer !== null) clearTimeout(qualityDebounceTimer); - qualityDebounceTimer = setTimeout(() => { - if (current.quality !== lastQuality) { - const prevQuality = lastQuality; - lastQuality = current.quality; - qualityChangeListeners.forEach((cb) => cb(current.quality, prevQuality)); - } - }, QUALITY_DEBOUNCE_MS); + if (qualityDebounceTimer === null) { + qualityDebounceTimer = setTimeout(() => { + qualityDebounceTimer = null; + if (current.quality !== lastQuality) { + const prevQuality = lastQuality; + lastQuality = current.quality; + qualityChangeListeners.forEach((cb) => cb(current.quality, prevQuality)); + } + }, QUALITY_DEBOUNCE_MS); + } + } else if (qualityDebounceTimer !== null) { + clearTimeout(qualityDebounceTimer); + qualityDebounceTimer = null; } } diff --git a/Client/tauri-client/src/lib/context-menu.ts b/Client/tauri-client/src/lib/context-menu.ts index 29c6ab71..319b48b2 100644 --- a/Client/tauri-client/src/lib/context-menu.ts +++ b/Client/tauri-client/src/lib/context-menu.ts @@ -23,6 +23,12 @@ export interface ContextMenuOptions { readonly className?: string; } +// Tracks each open menu's per-invocation dismiss controller, so a menu swept +// away by a same-class reopen (see below) can release its own teardown +// listener on the caller's signal instead of leaving it pinned until the +// caller's signal eventually aborts. +const dismissControllers = new WeakMap(); + /** * Show a context menu at the given coordinates. * Automatically removes any existing menu with the same className. @@ -32,13 +38,21 @@ export function showContextMenu(opts: ContextMenuOptions): void { const { x, y, items, signal, className } = opts; const menuClass = className ?? "context-menu"; - // Remove any existing context menu with same class - document.querySelectorAll(`.${menuClass}`).forEach((el) => el.remove()); + // Remove any existing context menu with same class, releasing its dismiss + // controller so its teardown listener on the caller's signal is dropped + // now rather than lingering until the caller itself is destroyed. + document.querySelectorAll(`.${menuClass}`).forEach((el) => { + dismissControllers.get(el)?.abort(); + el.remove(); + }); const menu = createElement("div", { class: `context-menu ${menuClass}` }); menu.style.left = `${x}px`; menu.style.top = `${y}px`; + const dismissAc = new AbortController(); + dismissControllers.set(menu, dismissAc); + let hasSeparator = false; for (const item of items) { if (hasSeparator && item.danger) { @@ -69,7 +83,6 @@ export function showContextMenu(opts: ContextMenuOptions): void { document.body.appendChild(menu); // Close on click outside (deferred so the opening click doesn't immediately close) - const dismissAc = new AbortController(); setTimeout(() => { if (dismissAc.signal.aborted) return; document.addEventListener( @@ -84,9 +97,25 @@ export function showContextMenu(opts: ContextMenuOptions): void { ); }, 0); - // Clean up if parent component is destroyed - signal.addEventListener("abort", () => { + // Clean up if parent component is destroyed. If the caller's signal is + // already aborted, "abort" already fired and would never reach a listener + // added now, so tear down immediately instead of registering one. When it + // isn't, tie the listener's own lifetime to dismissAc: once the menu is + // dismissed some other way (item click, outside click), dismissAc aborts + // and this listener is dropped from the caller's signal instead of + // lingering — with its closure over `menu` — for the rest of the caller's + // lifetime. + if (signal.aborted) { menu.remove(); dismissAc.abort(); - }); + } else { + signal.addEventListener( + "abort", + () => { + menu.remove(); + dismissAc.abort(); + }, + { once: true, signal: dismissAc.signal }, + ); + } } diff --git a/Client/tauri-client/src/lib/dispatcher.ts b/Client/tauri-client/src/lib/dispatcher.ts index dce8fb24..80e760eb 100644 --- a/Client/tauri-client/src/lib/dispatcher.ts +++ b/Client/tauri-client/src/lib/dispatcher.ts @@ -55,6 +55,7 @@ import { setDmChannels, addDmChannel, closeDmLocally, + clearDmUnread, updateDmLastMessage, updateDmLastMessagePreview, incrementDmMention, @@ -447,6 +448,14 @@ export function wireDispatcher( // activating it lands on an id ChannelController can't resolve and // blanks the chat area with no way to recover. addDmToChannelsStore(remaining[0]!); + // A DM's unread badge lives in dmStore, not the channelsStore + // mirror — setActiveChannel only zeroes the latter. Every other + // "open this DM" path (selectDmConversation, navigateToChannel, + // markChannelRead) pairs activation with clearDmUnread for exactly + // this reason; without it here the badge on the DM we're about to + // treat as active survives forever (new messages take the + // isDmActive branch below and never increment it back). + clearDmUnread(remaining[0]!.channelId); setActiveChannel(remaining[0]!.channelId); return; } diff --git a/Client/tauri-client/src/lib/media-visibility.ts b/Client/tauri-client/src/lib/media-visibility.ts index 8d90191e..c2a62a04 100644 --- a/Client/tauri-client/src/lib/media-visibility.ts +++ b/Client/tauri-client/src/lib/media-visibility.ts @@ -67,13 +67,11 @@ function freezeImage(img: HTMLImageElement, entry: MediaEntry): void { } entry.isPlaying = false; - if (img.src === entry.originalSrc) { - if (entry.frozenSrc === null) { - entry.frozenSrc = captureStaticFrame(img); - } - if (entry.frozenSrc !== null) { - img.src = entry.frozenSrc; - } + if (entry.frozenSrc === null) { + entry.frozenSrc = captureStaticFrame(img); + } + if (entry.frozenSrc !== null && img.src !== entry.frozenSrc) { + img.src = entry.frozenSrc; } updateButton(entry); } diff --git a/Client/tauri-client/src/lib/profiles.ts b/Client/tauri-client/src/lib/profiles.ts index f7678feb..676fecb3 100644 --- a/Client/tauri-client/src/lib/profiles.ts +++ b/Client/tauri-client/src/lib/profiles.ts @@ -103,6 +103,21 @@ function isValidStoredData(data: unknown): data is StoredData { ); } +/** + * Validates only the persistence envelope shape (schema version + a + * profiles array), without requiring every individual profile inside it to + * be well-formed. Used to tell "nothing/garbage was stored" apart from "a + * valid envelope containing some malformed entries" — the latter should + * have only the bad entries dropped, not the whole envelope discarded. + */ +function isValidStoredEnvelope( + data: unknown, +): data is { schemaVersion: number; profiles: unknown[] } { + if (typeof data !== "object" || data === null) return false; + const obj = data as Record; + return typeof obj.schemaVersion === "number" && Array.isArray(obj.profiles); +} + // --------------------------------------------------------------------------- // Default Tauri persistence backend // --------------------------------------------------------------------------- @@ -114,8 +129,15 @@ export function createTauriBackend(): PersistenceBackend { const settings = await invoke>("get_settings"); const raw = settings[STORAGE_KEY]; if (raw === undefined || raw === null) return null; - if (isValidStoredData(raw)) return raw; - return null; + if (!isValidStoredEnvelope(raw)) return null; + // The envelope itself is well-formed; salvage whichever individual + // profiles are valid rather than discarding the entire stored list + // because one entry is malformed (see OC-0060). Mirrors the per-item + // tolerance importProfiles() already has. + return { + schemaVersion: raw.schemaVersion, + profiles: raw.profiles.filter(isValidProfileShape), + }; }, async save(data: StoredData): Promise { const { invoke } = await import("@tauri-apps/api/core"); diff --git a/Client/tauri-client/src/lib/read-state.ts b/Client/tauri-client/src/lib/read-state.ts index 0c4b72f9..afeb49ee 100644 --- a/Client/tauri-client/src/lib/read-state.ts +++ b/Client/tauri-client/src/lib/read-state.ts @@ -95,6 +95,15 @@ function cancelPendingMarkAll(): void { pendingMarkAll = []; } +/** Sum of a channel or DM's unread + mention counts, from whichever store + * knows it. 0 for a channel this client does not (or no longer) know. */ +function unreadTotal(channelId: number): number { + const ch = channelsStore.getState().channels.get(channelId); + if (ch !== undefined) return ch.unreadCount + ch.mentionCount; + const dm = dmStore.getState().channels.find((c) => c.channelId === channelId); + return dm !== undefined ? dm.unreadCount + dm.mentionCount : 0; +} + /** * Mark every unread channel and DM read. Returns how many were marked, so the * caller can stay silent when there was nothing to do. @@ -103,6 +112,15 @@ function cancelPendingMarkAll(): void { * apart — see the budget note above. Each channel's local badge is cleared at * the moment its own frame actually goes out, not up front, so a channel * whose send hasn't fired yet still shows unread rather than lying about it. + * + * The deferred tail snapshots each channel's unread+mention total at click + * time and skips its send if that total has grown by the time the timer + * fires: a message that arrives during the pacing window postdates the click + * and was never seen, so marking it read would silently wipe a genuinely-new + * badge (and tell the server the user has read a message they never saw). + * The synchronous first burst has no such window — nothing can arrive between + * scheduling and firing in the same tick — so it stays unconditional, as does + * every other caller of `markChannelRead`. */ export function markAllRead(): number { // A second click supersedes the first: its own `unreadChannelIds()` already @@ -115,7 +133,12 @@ export function markAllRead(): number { if (delay === 0) { markChannelRead(id); } else { - pendingMarkAll.push(setTimeout(() => markChannelRead(id), delay)); + const snapshot = unreadTotal(id); + pendingMarkAll.push( + setTimeout(() => { + if (unreadTotal(id) <= snapshot) markChannelRead(id); + }, delay), + ); } } return ids.length; diff --git a/Client/tauri-client/src/lib/streamPreview.ts b/Client/tauri-client/src/lib/streamPreview.ts index cf558df7..ec6d9def 100644 --- a/Client/tauri-client/src/lib/streamPreview.ts +++ b/Client/tauri-client/src/lib/streamPreview.ts @@ -247,6 +247,7 @@ function hidePreview(row: HTMLElement): void { const state = previewTimers.get(row); if (state !== undefined) { clearTimeout(state.debounce); + clearTimeout(state.animation); if (state.trackCleanup !== null) { state.trackCleanup(); state.trackCleanup = null; diff --git a/Client/tauri-client/src/lib/ws.ts b/Client/tauri-client/src/lib/ws.ts index 4b4a88a1..bda148e4 100644 --- a/Client/tauri-client/src/lib/ws.ts +++ b/Client/tauri-client/src/lib/ws.ts @@ -483,6 +483,10 @@ export function createWsClient() { async function connect(cfg: WsClientConfig): Promise { wsGeneration++; + // Captured so a disconnect() landing mid-await (this function has three + // await points below) can be detected on resume — disconnect() bumps + // wsGeneration too, so a mismatch here means this attempt was cancelled. + const gen = wsGeneration; config = cfg; intentionalClose = false; // Belt-and-braces: a fresh connect (even one not routed through @@ -494,6 +498,11 @@ export function createWsClient() { setState("connecting"); await ensureTauriApis(); + if (gen !== wsGeneration) { + // A disconnect() (or a newer connect()) landed while we were + // suspended here — this attempt is cancelled, do not proceed. + return; + } if (tauriInvoke === null) { log.error("Tauri APIs not available, cannot connect WebSocket"); setState("disconnected"); @@ -510,6 +519,14 @@ export function createWsClient() { // Set up event listeners before connecting cleanupEventListeners(); await setupEventListeners(); + if (gen !== wsGeneration) { + // Cancelled while awaiting the Tauri IPC round trips inside + // setupEventListeners(). Tear down the listeners this (now-stale) + // attempt just registered instead of leaving them until the next + // connect() happens to clean them up. + cleanupEventListeners(); + return; + } try { await tauriInvoke("ws_connect", { url: wsUrl }); @@ -583,6 +600,12 @@ export function createWsClient() { } function disconnect(): void { + // Invalidate any connect() suspended mid-await (e.g. cancelled + // auto-login, logout racing a fresh connect) so it notices on resume + // instead of finishing setup and opening the very socket this teardown + // was meant to prevent. See setupEventListeners()'s tauriListen guards + // and connect()'s own gen checks. + wsGeneration++; intentionalClose = true; log.info("WebSocket disconnecting (intentional)", { host: config?.host ?? "unknown" }); certMismatchBlock = false; diff --git a/Client/tauri-client/src/styles/app.css b/Client/tauri-client/src/styles/app.css index facbe45a..16054974 100644 --- a/Client/tauri-client/src/styles/app.css +++ b/Client/tauri-client/src/styles/app.css @@ -666,6 +666,14 @@ bottom: -1px; right: -1px; } +/* UserBar.ts toggles this whenever canSetStatus() is false (socket down, or + no ws send path at all) -- without it the dot, dropdown options and + custom-status input all stay fully interactive, so anything picked or + typed while disconnected is silently dropped instead of refused. */ +.user-bar .ub-status-picker-wrap.ub-status-picker--disabled { + pointer-events: none; + opacity: 0.5; +} .user-bar .ub-info { flex: 1; min-width: 0; diff --git a/Client/tauri-client/tests/unit/advanced-tab.test.ts b/Client/tauri-client/tests/unit/advanced-tab.test.ts index 6c6d98be..a47ce1e4 100644 --- a/Client/tauri-client/tests/unit/advanced-tab.test.ts +++ b/Client/tauri-client/tests/unit/advanced-tab.test.ts @@ -9,6 +9,9 @@ const { mockClearEmbedCaches, mockClearMediaCaches, deleteDbState, + mockIsEnabled, + mockEnable, + mockDisable, } = vi.hoisted(() => ({ mockReadDir: vi.fn().mockResolvedValue([]), mockRemove: vi.fn().mockResolvedValue(undefined), @@ -26,6 +29,9 @@ const { | "blocked-double" | "success-then-blocked", }, + mockIsEnabled: vi.fn().mockResolvedValue(false), + mockEnable: vi.fn().mockResolvedValue(undefined), + mockDisable: vi.fn().mockResolvedValue(undefined), })); // Mock Tauri APIs @@ -43,6 +49,11 @@ vi.mock("@tauri-apps/plugin-fs", () => ({ vi.mock("@tauri-apps/plugin-process", () => ({ relaunch: mockRelaunch, })); +vi.mock("@tauri-apps/plugin-autostart", () => ({ + isEnabled: mockIsEnabled, + enable: mockEnable, + disable: mockDisable, +})); vi.mock("@lib/logger", () => ({ createLogger: () => ({ debug: vi.fn(), @@ -731,3 +742,93 @@ describe("AdvancedTab — Toggles & Structure", () => { expect(invoke).toHaveBeenCalledWith("open_devtools"); }); }); + +describe("AdvancedTab — Launch on Login (autostart)", () => { + let container: HTMLDivElement; + const ac = new AbortController(); + + beforeEach(() => { + container = document.createElement("div"); + document.body.appendChild(container); + mockIsEnabled.mockReset().mockResolvedValue(false); + mockEnable.mockReset().mockResolvedValue(undefined); + mockDisable.mockReset().mockResolvedValue(undefined); + }); + + afterEach(() => { + container.remove(); + }); + + function getAutostartToggle(): HTMLElement { + const section = buildAdvancedTab(ac.signal); + container.appendChild(section); + const rows = Array.from(container.querySelectorAll(".setting-row")); + const row = rows.find( + (r) => r.querySelector(".setting-label")?.textContent === "Launch on Login", + ); + expect(row).toBeDefined(); + return row!.querySelector(".toggle") as HTMLElement; + } + + async function tick(times = 1): Promise { + for (let i = 0; i < times; i++) { + await new Promise((r) => setTimeout(r, 0)); + } + } + + it("keeps the toggle ON when the user enables autostart before the init read-back resolves", async () => { + let resolveIsEnabled!: (v: boolean) => void; + const isEnabledPromise = new Promise((r) => { + resolveIsEnabled = r; + }); + mockIsEnabled.mockReturnValueOnce(isEnabledPromise); + + const toggle = getAutostartToggle(); + + // Let the init IIFE's dynamic import resolve and its isEnabled() call get + // issued, but don't resolve it yet — this is the window the finding + // describes. + await tick(2); + + // User clicks during that window: turns autostart ON. + toggle.click(); + expect(toggle.classList.contains("on")).toBe(true); + + // Let the click's own async chain (dynamic import + enable()) fully + // resolve, so the OS-level write really has completed. + await tick(3); + expect(mockEnable).toHaveBeenCalledTimes(1); + + // Now the stale init read finally resolves with the pre-click value. + resolveIsEnabled(false); + await tick(3); + + // The click already turned autostart on for real — the late, stale read + // must not clobber that visual/state. + expect(toggle.classList.contains("on")).toBe(true); + expect(toggle.getAttribute("aria-checked")).toBe("true"); + }); + + it("still applies a fresh init read-back when the user hasn't touched the toggle", async () => { + mockIsEnabled.mockResolvedValueOnce(true); + + const toggle = getAutostartToggle(); + await tick(3); + + expect(toggle.classList.contains("on")).toBe(true); + expect(toggle.getAttribute("aria-checked")).toBe("true"); + }); + + it("still reverts the toggle when the OS write itself fails", async () => { + mockEnable.mockRejectedValueOnce(new Error("not permitted")); + + const toggle = getAutostartToggle(); + await tick(3); + + toggle.click(); + await tick(3); + + expect(toggle.classList.contains("on")).toBe(false); + expect(toggle.getAttribute("aria-checked")).toBe("false"); + }); +}); diff --git a/Client/tauri-client/tests/unit/api.test.ts b/Client/tauri-client/tests/unit/api.test.ts index 9f1c9577..250c635e 100644 --- a/Client/tauri-client/tests/unit/api.test.ts +++ b/Client/tauri-client/tests/unit/api.test.ts @@ -345,6 +345,36 @@ describe("API Client", () => { const headers = fetchCallOpts().headers as Record; expect(headers["Authorization"]).toBe("Bearer fresh-token"); }); + + // OC-0136: every other layer of this client (livekitSession.ts's + // ensureLiveKitProxy, http_proxy.rs / livekit_proxy.rs's + // validate_remote_host + parse_server_name) deliberately accepts IPv6 + // literals, bracketed or bare. setConfig's host gate must not be the one + // place that refuses — otherwise login/register/auto-login to an IPv6 + // server throws "Invalid host format" even though the health check + // (which tunnels through the same Rust proxy) already reported it + // reachable. + describe("host validation accepts IPv6 literals", () => { + it("accepts a bracketed IPv6 literal with a port", () => { + expect(() => api.setConfig({ host: "[::1]:8443" })).not.toThrow(); + }); + + it("accepts a bracketed IPv6 literal without a port", () => { + expect(() => api.setConfig({ host: "[fd00::1]" })).not.toThrow(); + }); + + it("accepts a bare (unbracketed) IPv6 literal", () => { + expect(() => api.setConfig({ host: "2001:db8::1" })).not.toThrow(); + }); + + it("still rejects hosts with disallowed characters", () => { + expect(() => api.setConfig({ host: "evil host name" })).toThrow("Invalid host format"); + }); + + it("still rejects hosts that could inject headers", () => { + expect(() => api.setConfig({ host: "evil\r\nhost:8443" })).toThrow("Invalid host format"); + }); + }); }); describe("user endpoints", () => { diff --git a/Client/tauri-client/tests/unit/channel-navigation.test.ts b/Client/tauri-client/tests/unit/channel-navigation.test.ts index 9e81b15a..6b9ddc05 100644 --- a/Client/tauri-client/tests/unit/channel-navigation.test.ts +++ b/Client/tauri-client/tests/unit/channel-navigation.test.ts @@ -103,6 +103,29 @@ describe("channel-navigation", () => { expect(dmStore.getState().channels.find((c) => c.channelId === 50)?.unreadCount).toBe(3); }); + + // OC-0121: a DM present in dmStore from `ready` but never opened this + // session (so never selected via selectDmConversation) has no mirror row + // in channelsStore yet. A jump affordance (permalink, search hit, pinned, + // reply) must still be able to activate it instead of silently no-op'ing. + it("activates a DM known only to dmStore by synthesizing its channelsStore mirror", () => { + setDmChannels([ + makeDm({ + channelId: 50, + recipient: { id: 10, username: "bob", avatar: "", status: "online" }, + unreadCount: 3, + mentionCount: 1, + }), + ]); + + navigateToChannel(50); + + expect(channelsStore.getState().activeChannelId).toBe(50); + expect(channelsStore.getState().channels.get(50)?.name).toBe("bob"); + const dm = dmStore.getState().channels.find((c) => c.channelId === 50); + expect(dm?.unreadCount).toBe(0); + expect(dm?.mentionCount).toBe(0); + }); }); describe("findChannelById", () => { @@ -118,6 +141,21 @@ describe("channel-navigation", () => { it("returns null for an unknown id", () => { expect(findChannelById(999)).toBeNull(); }); + + // OC-0121: findChannelById gated on channelsStore alone, so a DM whose + // mirror row has not been synthesized yet (never opened this session) + // read as "not visible" even though the user is a member — the first + // check in MessageJump.jumpTo rejects it before getMessagesAround is + // ever called. + it("resolves a DM known only to dmStore, not yet mirrored into channelsStore", () => { + setDmChannels([ + makeDm({ + channelId: 50, + recipient: { id: 10, username: "bob", avatar: "", status: "online" }, + }), + ]); + expect(findChannelById(50)).toEqual({ id: 50, name: "bob" }); + }); }); describe("findChannelByName", () => { diff --git a/Client/tauri-client/tests/unit/connection-stats.test.ts b/Client/tauri-client/tests/unit/connection-stats.test.ts index 742508d8..39a037a3 100644 --- a/Client/tauri-client/tests/unit/connection-stats.test.ts +++ b/Client/tauri-client/tests/unit/connection-stats.test.ts @@ -361,6 +361,66 @@ describe("createConnectionStatsPoller", () => { expect(qualityCb).toHaveBeenCalledWith("bad", "excellent"); }); + it("fires quality change callback once the debounce elapses, even though the room stays active and every 2s poll keeps re-reporting the changed quality (OC-0132)", async () => { + // Regression test for OC-0132: POLL_INTERVAL_MS (2000) < QUALITY_DEBOUNCE_MS (3000) + // means that while the room stays active and quality remains different from + // lastQuality, every poll must NOT keep clearing/rescheduling the debounce + // timer — otherwise the timer can never reach its 3s deadline and the + // quality-change notification never fires. + let currentRtt = 0.01; + const room = { + engine: { + pcManager: { + publisher: { + pc: { + getStats: vi.fn().mockImplementation(() => { + const report = new Map(); + report.set("cp1", { + type: "candidate-pair", + currentRoundTripTime: currentRtt, + bytesSent: 0, + bytesReceived: 0, + }); + return Promise.resolve(report); + }), + }, + }, + }, + }, + }; + + const qualityCb = vi.fn(); + poller = createConnectionStatsPoller(() => room as any); + poller.onQualityChanged(qualityCb); + poller.start(); + + // t=2000: first poll establishes "excellent" baseline. + await vi.advanceTimersByTimeAsync(2100); + expect(qualityCb).not.toHaveBeenCalled(); + + // Degrade — the room stays active, so every subsequent 2s poll will keep + // observing "bad" (different from lastQuality "excellent") for as long as + // the connection stays degraded. + currentRtt = 0.5; + + // t=4000: quality first observed as "bad" -> debounce timer armed for t=7000. + await vi.advanceTimersByTimeAsync(2100); + expect(qualityCb).not.toHaveBeenCalled(); + + // t=6000: quality is STILL "bad" (still != lastQuality). Under the bug this + // poll clears the pending timer and reschedules a fresh 3s one, pushing the + // deadline out indefinitely for as long as the connection stays bad. + await vi.advanceTimersByTimeAsync(2100); + expect(qualityCb).not.toHaveBeenCalled(); + + // t=8000: the original debounce deadline (t=7000) has now passed. The room + // never went inactive and never stopped reporting "bad" in between, so the + // only way this fires is if same-quality polls stopped resetting the timer. + await vi.advanceTimersByTimeAsync(2100); + expect(qualityCb).toHaveBeenCalledTimes(1); + expect(qualityCb).toHaveBeenCalledWith("bad", "excellent"); + }); + it("unsubscribed onUpdate callback is not called", async () => { const room = createMockRoom([ { diff --git a/Client/tauri-client/tests/unit/context-menu.test.ts b/Client/tauri-client/tests/unit/context-menu.test.ts index 7c7c7be4..d86edf9b 100644 --- a/Client/tauri-client/tests/unit/context-menu.test.ts +++ b/Client/tauri-client/tests/unit/context-menu.test.ts @@ -315,4 +315,116 @@ describe("showContextMenu", () => { const menu = document.body.querySelector(".context-menu"); expect(menu).not.toBeNull(); }); + + describe("OC-0057: abort-listener teardown on the caller signal", () => { + it("does not re-invoke menu cleanup off the caller signal after the menu was already dismissed", () => { + showContextMenu({ + x: 0, + y: 0, + items: [{ label: "Action", onClick: vi.fn() }], + signal: ac.signal, + className: "oc0057-menu-a", + }); + + const menu = document.querySelector(".oc0057-menu-a") as HTMLElement; + expect(menu).not.toBeNull(); + + // Dismiss the menu through the normal item-click path (NOT via ac.abort()). + const item = menu.querySelector(".context-menu-item") as HTMLElement; + const removeSpy = vi.spyOn(menu, "remove"); + item.click(); + expect(removeSpy).toHaveBeenCalledTimes(1); + + // The component that owns `ac` is destroyed sometime later. A correctly + // torn-down showContextMenu invocation must have released its "abort" + // listener on `ac.signal` when the menu was dismissed above, so this + // must NOT invoke the stale closure's menu.remove() a second time. + ac.abort(); + + expect(removeSpy).toHaveBeenCalledTimes(1); + }); + + it("does not accumulate a live abort listener on the caller signal per invocation", () => { + // Open and dismiss (via item click) several menus on the same + // long-lived caller signal, as DmSidebar does across repeated + // right-clicks without the sidebar being rebuilt. + const menus: HTMLElement[] = []; + for (let i = 0; i < 3; i++) { + showContextMenu({ + x: 0, + y: 0, + items: [{ label: "Action", onClick: vi.fn() }], + signal: ac.signal, + className: "oc0057-menu-b", + }); + const menu = document.querySelector(".oc0057-menu-b") as HTMLElement; + menus.push(menu); + const item = menu.querySelector(".context-menu-item") as HTMLElement; + item.click(); + } + + const removeSpies = menus.map((m) => vi.spyOn(m, "remove")); + + // Simulate the parent component finally being destroyed. + ac.abort(); + + // None of the already-dismissed menus' remove() should fire again — + // each invocation's abort listener should have been released when that + // specific menu was dismissed, not held until component teardown. + for (const spy of removeSpies) { + expect(spy).not.toHaveBeenCalled(); + } + }); + + it("cleans up immediately when the signal is already aborted before the menu is shown", () => { + ac.abort(); + + showContextMenu({ + x: 0, + y: 0, + items: [{ label: "Action", onClick: vi.fn() }], + signal: ac.signal, + className: "oc0057-menu-c", + }); + + // An already-aborted parent signal means the menu must never be left + // dangling in the DOM — "abort" already fired before we could listen + // for it, so the code must check signal.aborted explicitly. + expect(document.querySelector(".oc0057-menu-c")).toBeNull(); + }); + + it("releases the old menu's abort listener when it is swept away by a same-class reopen", () => { + showContextMenu({ + x: 0, + y: 0, + items: [{ label: "First", onClick: vi.fn() }], + signal: ac.signal, + className: "oc0057-menu-d", + }); + + const firstMenu = document.querySelector(".oc0057-menu-d") as HTMLElement; + expect(firstMenu).not.toBeNull(); + const removeSpy = vi.spyOn(firstMenu, "remove"); + + // Reopening with the same className sweeps the first menu out via the + // querySelectorAll(...).remove() path at the top of the function, not + // via item click or outside click. + showContextMenu({ + x: 10, + y: 10, + items: [{ label: "Second", onClick: vi.fn() }], + signal: ac.signal, + className: "oc0057-menu-d", + }); + + expect(removeSpy).toHaveBeenCalledTimes(1); + + // The parent component is destroyed later. The swept-away first menu's + // abort listener must have been released at sweep time, not left + // pinned on `ac.signal` until now. + ac.abort(); + + expect(removeSpy).toHaveBeenCalledTimes(1); + }); + }); }); diff --git a/Client/tauri-client/tests/unit/dispatcher.test.ts b/Client/tauri-client/tests/unit/dispatcher.test.ts index ddb83f80..128e8c92 100644 --- a/Client/tauri-client/tests/unit/dispatcher.test.ts +++ b/Client/tauri-client/tests/unit/dispatcher.test.ts @@ -2965,6 +2965,53 @@ describe("WS Dispatcher", () => { expect(channelsStore.getState().activeChannelId).toBe(60); }); + // setActiveChannel only zeroes the channelsStore mirror row's counts; a + // DM's badge lives in dmStore (SidebarDmSection reads dmStore.unreadCount, + // not the channelsStore mirror). Every other "open this DM" path + // (selectDmConversation, navigateToChannel, markChannelRead) pairs + // activation with clearDmUnread for exactly this reason — the + // dm_channel_close fallback must too, or the badge on the DM it just + // activated survives forever (it's now "active", so new messages take the + // isDmActive branch and never increment it back). + it("clears the dmStore unread badge on the DM it falls back to activating", () => { + dmStore.setState(() => ({ + channels: [ + { + channelId: 50, + recipient: { id: 10, username: "bob", avatar: "", status: "online" }, + participants: [], + name: "", + isGroup: false, + lastMessageId: null, + lastMessage: "", + lastMessageAt: "", + unreadCount: 0, + mentionCount: 0, + }, + { + channelId: 60, + recipient: { id: 11, username: "carl", avatar: "", status: "online" }, + participants: [], + name: "", + isGroup: false, + lastMessageId: null, + lastMessage: "", + lastMessageAt: "", + unreadCount: 3, + mentionCount: 1, + }, + ], + })); + channelsStore.setState((prev) => ({ ...prev, activeChannelId: 50 })); + + mock.dispatch("dm_channel_close", { channel_id: 50 }); + + expect(channelsStore.getState().activeChannelId).toBe(60); + const dm = dmStore.getState().channels.find((c) => c.channelId === 60); + expect(dm?.unreadCount).toBe(0); + expect(dm?.mentionCount).toBe(0); + }); + // The channelsStore mirror row for a DM is only ever synthesized by // addDmToChannelsStore (on open, via selectDmConversation) — a DM present // in dmStore from `ready` but never opened this session has none. Without diff --git a/Client/tauri-client/tests/unit/media-visibility.test.ts b/Client/tauri-client/tests/unit/media-visibility.test.ts index b4430058..3b64e528 100644 --- a/Client/tauri-client/tests/unit/media-visibility.test.ts +++ b/Client/tauri-client/tests/unit/media-visibility.test.ts @@ -152,6 +152,26 @@ describe("media-visibility", () => { cleanup(); }); + it("freezes a GIF whose src the DOM normalizes away from the raw originalSrc (OC-0130)", () => { + const cleanup = setupCanvasMocks(); + // Mixed-case host: the element's src getter returns this + // lowercased per the WHATWG URL spec, so it no longer string-matches + // the raw originalSrc the caller passed in. + const raw = "https://EXAMPLE.com/anim.gif"; + const img = createFakeImg(raw); + const wrap = createWrapper(); + observeMedia(img, raw, wrap); + // Sanity check: the DOM really did normalize it away from `raw`. + expect(img.src).toBe("https://example.com/anim.gif"); + expect(img.src).not.toBe(raw); + + const btn = wrap.querySelector(".gif-play-btn") as HTMLButtonElement; + btn.click(); + expect(img.src).toBe("data:image/png;base64,frozen"); + expect(wrap.classList.contains("gif-paused")).toBe(true); + cleanup(); + }); + it("pause button click freezes immediately", () => { const cleanup = setupCanvasMocks(); const img = createFakeImg("https://example.com/cat.gif"); diff --git a/Client/tauri-client/tests/unit/message-list.test.ts b/Client/tauri-client/tests/unit/message-list.test.ts index e96a5623..860bef35 100644 --- a/Client/tauri-client/tests/unit/message-list.test.ts +++ b/Client/tauri-client/tests/unit/message-list.test.ts @@ -678,4 +678,46 @@ describe("MessageList", () => { expect(row1!.textContent).toContain("Edited"); }); }); + + describe("renderAll rapid-fire breaker", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("renders the final state once the 2s burst window resets, instead of staying stuck at the pre-trip state", () => { + setMessages(1, [makeMessage({ id: 1, content: "v0" })]); + msgList.mount(container); // 1st renderAll call, starts the 2s window + + // Fire 25 non-append updates (edits) back-to-back, well inside the 2s + // window. tryAppendMessages() returns false for every one of these + // (same-length array, content changed) so each forces a renderAll(). + // Combined with the mount's call, this is 26 renderAll invocations — + // calls 21+ trip the >20-in-2s breaker and must return without + // rendering. + for (let i = 1; i <= 25; i++) { + setMessages(1, [makeMessage({ id: 1, content: `v${i}` })]); + messagesStore.flush(); + } + + const rowDuringBurst = container.querySelector("[data-testid='message-1']"); + expect(rowDuringBurst).not.toBeNull(); + // The breaker tripped partway through, so the DOM is stuck behind the + // final store state (still showing an earlier version, not "v25"). + expect(rowDuringBurst!.textContent).not.toContain("v25"); + + // Let the 2s reset window elapse with no further store updates. + vi.advanceTimersByTime(2100); + + // Once the burst is over, the list must reflect the final state that + // triggered the last suppressed renderAll — not stay frozen on + // whatever rendered right before the breaker tripped. + const rowAfterReset = container.querySelector("[data-testid='message-1']"); + expect(rowAfterReset).not.toBeNull(); + expect(rowAfterReset!.textContent).toContain("v25"); + }); + }); }); diff --git a/Client/tauri-client/tests/unit/profiles.test.ts b/Client/tauri-client/tests/unit/profiles.test.ts index a381cb1f..75484489 100644 --- a/Client/tauri-client/tests/unit/profiles.test.ts +++ b/Client/tauri-client/tests/unit/profiles.test.ts @@ -789,5 +789,107 @@ describe("ProfileManager", () => { expect(result).toBeNull(); }); + + it("load salvages the valid profiles when one stored entry is malformed (OC-0060)", async () => { + const storedData = { + schemaVersion: 1, + profiles: [ + { + id: "good-1", + name: "Good One", + host: "good1.example.com:443", + username: "user1", + color: "#111111", + autoConnect: false, + rememberPassword: false, + lastConnected: null, + }, + { + // Malformed: written by a build predating the color field / partial write. + id: "bad-1", + name: "", + host: "bad1.example.com:443", + username: "user2", + autoConnect: false, + rememberPassword: false, + lastConnected: null, + }, + { + id: "good-2", + name: "Good Two", + host: "good2.example.com:443", + username: "user3", + color: "#222222", + autoConnect: false, + rememberPassword: false, + lastConnected: null, + }, + ], + }; + mockInvoke.mockResolvedValueOnce({ "owncord:profiles": storedData }); + + const backend = createTauriBackend(); + const result = await backend.load(); + + // A single corrupt entry must not null out the whole store — only the + // bad entry should be dropped, salvaging the other two valid profiles. + expect(result).not.toBeNull(); + expect(result!.profiles.map((p) => p.id)).toEqual(["good-1", "good-2"]); + }); + }); + + // ── OC-0060: one bad stored profile must not evict the good ones ── + + describe("OC-0060 malformed profile salvage (end-to-end)", () => { + beforeEach(() => { + mockInvoke.mockReset(); + }); + + it("does not let a single malformed stored profile wipe out the rest on the next save", async () => { + const validProfiles = Array.from({ length: 5 }, (_, i) => ({ + id: `orig-${i}`, + name: `Original ${i}`, + host: `orig${i}.example.com:443`, + username: `user${i}`, + color: "#abcdef", + autoConnect: false, + rememberPassword: false, + lastConnected: null, + })); + // Corrupt one entry the way a hand-edit / partial write would: empty name. + const corrupted = [...validProfiles.slice(0, 4), { ...validProfiles[4], name: "" }]; + + mockInvoke.mockImplementation((cmd: string) => { + if (cmd === "get_settings") { + return Promise.resolve({ + "owncord:profiles": { schemaVersion: 1, profiles: corrupted }, + }); + } + if (cmd === "save_settings") { + return Promise.resolve(undefined); + } + return Promise.resolve(undefined); + }); + + const backend = createTauriBackend(); + const m = createProfileManager(backend, mockFetch as unknown as FetchFn); + + await m.loadProfiles(); + // The four well-formed originals must have survived the load. + expect(m.getAll()).toHaveLength(4); + + m.addProfile(sampleData); + await m.saveProfiles(); + + const saveCall = mockInvoke.mock.calls.find(([cmd]) => cmd === "save_settings"); + expect(saveCall).toBeDefined(); + const savedValue = saveCall![1] as { value: { profiles: ServerProfile[] } }; + // The persisted set must still contain the four originals plus the new + // profile — not just the newly added one. + expect(savedValue.value.profiles).toHaveLength(5); + expect(savedValue.value.profiles.map((p) => p.id)).toEqual( + expect.arrayContaining(["orig-0", "orig-1", "orig-2", "orig-3"]), + ); + }); }); }); diff --git a/Client/tauri-client/tests/unit/read-state.test.ts b/Client/tauri-client/tests/unit/read-state.test.ts index 1ca0424f..3e2b99ea 100644 --- a/Client/tauri-client/tests/unit/read-state.test.ts +++ b/Client/tauri-client/tests/unit/read-state.test.ts @@ -12,7 +12,7 @@ import { setMarkReadSender, unreadChannelIds, } from "@lib/read-state"; -import { channelsStore, setChannels } from "@stores/channels.store"; +import { channelsStore, setChannels, incrementUnread } from "@stores/channels.store"; import { dmStore, setDmChannels } from "@stores/dm.store"; import type { ReadyChannel } from "@lib/types"; import type { DmChannel } from "@stores/dm.store"; @@ -211,6 +211,44 @@ describe("unreadChannelIds / markAllRead", () => { } }); + // OC-0123: a message that arrives in a channel while its own send is still + // sitting in the paced tail must not be wiped out by that stale click. The + // click predates the message; marking it read would silently swallow a + // genuinely-new unread the user has not seen. + it("does not mark read a channel that got a new message during the pacing window", () => { + vi.useFakeTimers(); + try { + setChannels([ + channel(1, 1), + channel(2, 1), + channel(3, 1), + channel(4, 1), + channel(5, 1), + channel(6, 1), + ]); + + expect(markAllRead()).toBe(6); + const firstBurst = new Set(sent); + const deferredId = [1, 2, 3, 4, 5, 6].find((id) => !firstBurst.has(id))!; + + // A new message lands in the deferred channel before its paced send fires + // — exactly what the dispatcher's chat_message handler does. + incrementUnread(deferredId); + expect(channelsStore.getState().channels.get(deferredId)?.unreadCount).toBe(2); + + vi.advanceTimersByTime(2000); + + // The paced send must skip this channel: the message postdates the click, + // so neither the local badge nor the server's read state should advance + // past it. + expect(sent).not.toContain(deferredId); + expect(hasUnread(deferredId)).toBe(true); + expect(channelsStore.getState().channels.get(deferredId)?.unreadCount).toBe(2); + } finally { + vi.useRealTimers(); + } + }); + it("supersedes an in-flight burst rather than double-sending it", () => { vi.useFakeTimers(); try { diff --git a/Client/tauri-client/tests/unit/search-overlay.test.ts b/Client/tauri-client/tests/unit/search-overlay.test.ts index 0c004f20..57f3794d 100644 --- a/Client/tauri-client/tests/unit/search-overlay.test.ts +++ b/Client/tauri-client/tests/unit/search-overlay.test.ts @@ -294,6 +294,61 @@ describe("createSearchOverlay", () => { overlay.destroy?.(); }); + it("does not attach a new click listener to each row on every re-render (OC-0147)", async () => { + // Per-row click listeners registered on the component-lifetime AbortSignal + // never get cleaned up when a row is discarded by a re-render — only + // destroy() aborts that signal. Re-renders triggered by ArrowDown/ArrowUp + // (which don't create new rows via a fresh search) must not register any + // additional listeners directly on ".search-result-item" elements; the + // fix delegates a single listener onto the results container instead. + const results = [ + makeResult({ message_id: 1 }), + makeResult({ message_id: 2 }), + makeResult({ message_id: 3 }), + ]; + const onSearch = vi.fn().mockResolvedValue(results); + const opts = makeOptions({ onSearch }); + const overlay = createSearchOverlay(opts); + overlay.mount(container); + + const input = container.querySelector(".search-overlay-input") as HTMLInputElement; + input.value = "test"; + input.dispatchEvent(new Event("input")); + await vi.advanceTimersByTimeAsync(300); + + expect(container.querySelectorAll(".search-result-item")).toHaveLength(3); + + // Only start counting after the initial render so we isolate what the + // subsequent re-renders (via arrow-key navigation) register. vi.spyOn's + // mock.instances isn't reliably typed/populated for non-constructor + // methods, so track `this` via a manual monkey-patch instead. + const perRowClickRegistrations: Element[] = []; + const originalAddEventListener = Element.prototype.addEventListener; + Element.prototype.addEventListener = function ( + this: Element, + type: string, + listener: EventListenerOrEventListenerObject, + options?: boolean | AddEventListenerOptions, + ): void { + if (type === "click" && this.classList.contains("search-result-item")) { + perRowClickRegistrations.push(this); + } + originalAddEventListener.call(this, type, listener, options); + }; + + try { + for (let i = 0; i < 5; i++) { + input.dispatchEvent(new KeyboardEvent("keydown", { key: "ArrowDown", bubbles: true })); + } + } finally { + Element.prototype.addEventListener = originalAddEventListener; + } + + expect(perRowClickRegistrations).toHaveLength(0); + + overlay.destroy?.(); + }); + it("destroy removes overlay from DOM", () => { const opts = makeOptions(); const overlay = createSearchOverlay(opts); diff --git a/Client/tauri-client/tests/unit/status-picker-userbar.test.ts b/Client/tauri-client/tests/unit/status-picker-userbar.test.ts index 44f7b0db..2432759d 100644 --- a/Client/tauri-client/tests/unit/status-picker-userbar.test.ts +++ b/Client/tauri-client/tests/unit/status-picker-userbar.test.ts @@ -222,4 +222,23 @@ describe("StatusPicker wired to UserBar", () => { ); } }); + + // UserBar's updatePickerDisabled toggles ub-status-picker--disabled on the + // wrap element whenever canSetStatus() is false, but that class has no + // effect at all unless app.css actually makes it inert -- otherwise the + // dropdown and its custom-status input stay fully clickable while the + // socket is down, and anything typed there is silently dropped (never sent + // now, never re-sent on reconnect since restoreSavedPresence only re-sends + // `status`, not `custom_status`). + it("ub-status-picker--disabled actually disables the picker in app.css", () => { + const css = readFileSync(join(process.cwd(), "src/styles/app.css"), "utf8"); + const match = /\.ub-status-picker--disabled\s*\{([^}]*)\}/.exec(css); + expect( + match, + "expected a `.ub-status-picker--disabled { ... }` rule in app.css", + ).not.toBeNull(); + expect(match![1], "disabled state must reject pointer input").toMatch( + /pointer-events\s*:\s*none/, + ); + }); }); diff --git a/Client/tauri-client/tests/unit/stream-preview.test.ts b/Client/tauri-client/tests/unit/stream-preview.test.ts index efbd64ad..afcae62d 100644 --- a/Client/tauri-client/tests/unit/stream-preview.test.ts +++ b/Client/tauri-client/tests/unit/stream-preview.test.ts @@ -289,6 +289,66 @@ describe("streamPreview", () => { expect(getPreview(row)).toBeNull(); }); + // OC-0124: hidePreview overwrites state.animation without clearing the + // timer already sitting there. When stopPreviewDelayed's 150ms grace timer + // (T1) is still pending and something else (scroll/focusout) calls + // hidePreview directly, T1 survives uncancelled, fires later, and calls + // hidePreview a second time — orphaning that call's own 200ms removal + // timer in turn. One of those orphaned removal timers eventually runs + // `previewTimers.delete(row)` against whatever state a *later* hover + // installed, deleting it before its debounce ever fires. showPreview then + // finds no state to store trackCleanup on, so the ended/mute listeners it + // just registered on the live MediaStreamTrack become permanently + // unreachable by hidePreview, clearPreviewState, and the abort handler. + it("does not leak track listeners when hidePreview interrupts a pending stopPreviewDelayed timer (OC-0124)", () => { + const stream = createMockMediaStream(); + const track = stream.getVideoTracks()[0]!; + const removeSpy = vi.spyOn(track, "removeEventListener"); + mockGetRemoteVideoStream.mockReturnValue(stream); + const row = createRow(42); + attachStreamPreview(row, 42, "Alice", false, true, ac.signal); + + // t=0: hover -> debounce armed. + row.dispatchEvent(new MouseEvent("mouseenter")); + // t=300: debounce fires -> preview shown, ended/mute listeners #1 registered. + vi.advanceTimersByTime(300); + expect(getPreview(row)).not.toBeNull(); + + // t=400: mouse leaves -> stopPreviewDelayed arms its 150ms grace timer. + vi.advanceTimersByTime(100); + row.dispatchEvent(new MouseEvent("mouseleave")); + + // t=450: a second, independent trigger (e.g. focusout from a Tab, or a + // scroll-collapse) calls hidePreview directly while the grace timer from + // t=400 is still pending. This must cancel that timer, not just + // overwrite the handle to it. + vi.advanceTimersByTime(50); + row.dispatchEvent(new FocusEvent("focusout")); + // trackCleanup #1 runs synchronously inside this hidePreview call. + expect(removeSpy).toHaveBeenCalledTimes(2); // "ended" + "mute" for listener #1 + + // t=650: the removal timer armed by the t=450 hidePreview call fires and + // tears down the (now empty) preview + state. + vi.advanceTimersByTime(200); + expect(getPreview(row)).toBeNull(); + + // t=660: user hovers again -> a fresh debounce/state is installed. + vi.advanceTimersByTime(10); + row.dispatchEvent(new MouseEvent("mouseenter")); + + // t=960: the new debounce fires and showPreview runs again, registering + // ended/mute listeners #2 on the same track and trying to store + // trackCleanup on the freshly-installed state. + vi.advanceTimersByTime(300); + expect(getPreview(row)).not.toBeNull(); + + // Close the second preview. If the fresh state survived intact, + // trackCleanup #2 fires here, removing listener set #2 as well. + row.dispatchEvent(new FocusEvent("focusout")); + + expect(removeSpy).toHaveBeenCalledTimes(4); // "ended" + "mute" for BOTH listener sets + }); + // Abort-listener accumulation (leak fix) it("registers only one abort listener per signal, not one per attach call", () => { mockGetRemoteVideoStream.mockReturnValue(null); diff --git a/Client/tauri-client/tests/unit/voice-audio-tab.test.ts b/Client/tauri-client/tests/unit/voice-audio-tab.test.ts index 35fe2e71..e11a0a47 100644 --- a/Client/tauri-client/tests/unit/voice-audio-tab.test.ts +++ b/Client/tauri-client/tests/unit/voice-audio-tab.test.ts @@ -900,4 +900,30 @@ describe("VoiceAudioTab UI structure", () => { expect(() => tab.build()).not.toThrow(); ac.abort(); }); + + it("does not register a new permanent abort listener on every rebuild (OC-0125)", () => { + // Simulates the settings overlay staying open for a session while the + // user re-opens the Voice & Audio tab (e.g. hide()/show() cycles): + // SettingsOverlay's single AbortController lives for the whole session, + // and build() is called again each time the tab is (re)rendered. + stubNavigator(); + const ac = new AbortController(); + const addEventListenerSpy = vi.spyOn(ac.signal, "addEventListener"); + const abortListenerCount = (): number => + addEventListenerSpy.mock.calls.filter(([type]) => type === "abort").length; + + const tab = createVoiceAudioTab(ac.signal); + const afterCreate = abortListenerCount(); + + tab.build(); + tab.build(); + tab.build(); + + // Rebuilding the tab three times must not add three more permanent + // "abort" listeners to the overlay-lifetime signal — only the single + // listener the factory registers once at creation should exist. + expect(abortListenerCount()).toBe(afterCreate); + + ac.abort(); + }); }); diff --git a/Client/tauri-client/tests/unit/ws-lifecycle.test.ts b/Client/tauri-client/tests/unit/ws-lifecycle.test.ts index 952a8bc9..7f3d0132 100644 --- a/Client/tauri-client/tests/unit/ws-lifecycle.test.ts +++ b/Client/tauri-client/tests/unit/ws-lifecycle.test.ts @@ -595,6 +595,72 @@ describe("wsGeneration stale listener guard", () => { }); }); +describe("disconnect() cancelling an in-flight connect()", () => { + let client: ReturnType; + let originalMockListenImpl: (typeof mockListen)["getMockImplementation"] extends () => infer R + ? R + : never; + + beforeEach(() => { + vi.useFakeTimers(); + mockInvoke.mockReset(); + mockInvoke.mockResolvedValue(undefined); + originalMockListenImpl = mockListen.getMockImplementation()!; + mockListen.mockClear(); + eventHandlers.clear(); + client = createWsClient(); + }); + + afterEach(() => { + mockListen.mockImplementation(originalMockListenImpl!); + client.disconnect(); + vi.useRealTimers(); + }); + + // Mirrors main.ts's onAutoLoginCancel: by the time the Cancel button is + // clickable, wirePostAuth has already called ws.connect() and connect() + // is suspended mid-await (setupEventListeners' tauriListen round trips). + // disconnect() runs synchronously while that await is pending, then the + // suspended connect() resumes. + it("prevents ws_connect from being invoked after disconnect() runs mid-connect()", async () => { + let releaseListen: (() => void) | null = null; + mockListen.mockImplementation( + async (event: string, handler: (e: { payload: unknown }) => void) => { + if (event === "ws-message" && releaseListen === null) { + // Pause connect() here, mimicking the Cancel click landing while + // connect() is still awaiting its Tauri IPC round trips. + await new Promise((resolve) => { + releaseListen = resolve; + }); + } + return originalMockListenImpl!(event, handler); + }, + ); + + client.connect({ host: "localhost:8443", token: "t" }); + // Let connect() run past ensureTauriApis()/cleanupEventListeners() and + // into the paused first tauriListen("ws-message", ...) call. + await vi.advanceTimersByTimeAsync(10); + expect(releaseListen).not.toBeNull(); + + // Cancel arrives while connect() is suspended mid-await. + client.disconnect(); + expect(client.getState()).toBe("disconnected"); + + // Resume the suspended connect() — it must notice the cancellation and + // bail out instead of completing setupEventListeners() and invoking + // ws_connect. + releaseListen!(); + await vi.advanceTimersByTimeAsync(10); + + const wsConnectCalls = mockInvoke.mock.calls.filter((c) => c[0] === "ws_connect"); + expect(wsConnectCalls).toHaveLength(0); + // The cancelled attempt must not have flipped the state back out of + // "disconnected" (e.g. to "authenticating"/"reconnecting"). + expect(client.getState()).toBe("disconnected"); + }); +}); + describe("heartbeat proxyOpen guard", () => { let client: ReturnType;