docs(bugs): reconcile LOTUS_BUGS statuses with wave-1 fixes
Mark items resolved by commitsb7e1f89c/d2946c00/0394fce9/203568c9as FIXED, and record the false-positives surfaced during the audit (useMatrixEventRenderer null contract, Lobby getRoom already memoized, RoomTimeline/RoomInput already wrapped by RoomView's ErrorBoundary). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+21
-21
@@ -39,11 +39,11 @@ This document tracks identified bugs, edge cases, and architectural discrepancie
|
|||||||
|
|
||||||
### 3. Avatar Decorations in Element Call
|
### 3. Avatar Decorations in Element Call
|
||||||
|
|
||||||
- **File:** `cinny/src/app/components/avatar-decoration/AvatarDecoration.tsx`
|
- **File:** `cinny/src/app/features/call/CallMemberCard.tsx`
|
||||||
- **Status:** **OPEN**
|
- **Status:** **FIXED ⚠️ UNTESTED** — needs verification in a live call with a participant who has a decoration set
|
||||||
- **Issue:** Avatar decorations are failing to render within the call/room interface member lists.
|
- **Issue:** Avatar decorations are failing to render within the call/room interface member lists.
|
||||||
- **Root Cause:** Likely a mismatch between the expected `member` object structure required by the `AvatarDecoration` component and the data actually provided by the call/room UI components. Matrix event data for decorations might not be propagating correctly to these UI member objects.
|
- **Root Cause:** Member lists and the people drawer already wrapped avatars in `<AvatarDecoration userId={...}>`, but the call participant tile (`CallMemberCard`) rendered a bare `<UserAvatar>` with no decoration wrapper — so decorations were absent specifically on call tiles. (Note: avatars rendered _inside_ the Element Call iframe are EC-rendered and out of our control; this fix covers our own participant roster / prescreen.)
|
||||||
- **Proposed Fix:** Analyze the data propagation chain from Matrix events to the member object in `cinny/src/app/components/call` and `room`, ensuring that decoration-related properties are correctly mapped and passed to the `AvatarDecoration` component.
|
- **Fix Applied:** Wrapped the call-tile avatar in `<AvatarDecoration userId={userId}>` (commit `0394fce9`), matching the member-list pattern.
|
||||||
|
|
||||||
### 4. DM and Group Message Calls
|
### 4. DM and Group Message Calls
|
||||||
|
|
||||||
@@ -131,19 +131,19 @@ This document tracks identified bugs, edge cases, and architectural discrepancie
|
|||||||
|
|
||||||
| Category | Issue Description | File Path | Status |
|
| Category | Issue Description | File Path | Status |
|
||||||
| :---------------------- | :------------------------------------------------------------------------------------------------------------------------------------------------------- | :---------------------------------------------- | :-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
| :---------------------- | :------------------------------------------------------------------------------------------------------------------------------------------------------- | :---------------------------------------------- | :-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||||
| State Sync | Fire-and-forget network call to set offline presence during `pagehide` event may not complete reliably, potentially causing UI drift in presence status. | `cinny/src/app/hooks/usePresenceUpdater.ts` | OPEN |
|
| State Sync | Fire-and-forget network call to set offline presence during `pagehide` event may not complete reliably, potentially causing UI drift in presence status. | `cinny/src/app/hooks/usePresenceUpdater.ts` | FIXED (`d2946c00`) — unload path now uses `fetch({ keepalive: true })` so the request survives page teardown (`sendBeacon` was unusable here: it can't set the auth header). |
|
||||||
| State Sync | Fire-and-forget network call `setPresence().catch(...)` suppresses errors, meaning the app may falsely assume presence update success. | `cinny/src/app/hooks/usePresenceUpdater.ts` | OPEN |
|
| State Sync | Fire-and-forget network call `setPresence().catch(...)` suppresses errors, meaning the app may falsely assume presence update success. | `cinny/src/app/hooks/usePresenceUpdater.ts` | FIXED (`d2946c00`) — errors are now surfaced via `warnPresenceFailure` (redacted logging) instead of being silently swallowed. |
|
||||||
| Memory Leak | Decrypted Media Memory Leak (Gallery & Lightbox) due to missing virtualization and blob revocation. | `cinny/src/app/features/room/MediaGallery.tsx` | PARTIALLY FIXED ⚠️ UNTESTED — Blob revocation was already correct; added `enabled` param to `useDecryptedMediaUrl` and `useNearViewport(300px)` to each `GalleryTile` to gate decryption until near-viewport, reducing burst on pagination. True virtualization (windowing) deferred — requires significant refactor. |
|
| Memory Leak | Decrypted Media Memory Leak (Gallery & Lightbox) due to missing virtualization and blob revocation. | `cinny/src/app/features/room/MediaGallery.tsx` | PARTIALLY FIXED ⚠️ UNTESTED — Blob revocation was already correct; added `enabled` param to `useDecryptedMediaUrl` and `useNearViewport(300px)` to each `GalleryTile` to gate decryption until near-viewport, reducing burst on pagination. True virtualization (windowing) deferred — requires significant refactor. |
|
||||||
| Data Persistence | Scheduled Messages are ephemeral (lost on refresh) due to fragile `localStorage` parsing. | `cinny/src/app/state/scheduledMessages.ts` | FIXED — now uses `atomWithStorage` + `createJSONStorage` (Jotai's built-in persistence with error-safe JSON parsing) |
|
| Data Persistence | Scheduled Messages are ephemeral (lost on refresh) due to fragile `localStorage` parsing. | `cinny/src/app/state/scheduledMessages.ts` | FIXED — now uses `atomWithStorage` + `createJSONStorage` (Jotai's built-in persistence with error-safe JSON parsing) |
|
||||||
| Memory Leak | Potential memory leak due to uncleaned `handleMouseMove` listener in `usePan`. | `cinny/src/app/hooks/usePan.ts` | FALSE POSITIVE — `usePan` already uses `attachedRef` to track listeners and cleans them up in an unmount `useEffect`. No change needed. |
|
| Memory Leak | Potential memory leak due to uncleaned `handleMouseMove` listener in `usePan`. | `cinny/src/app/hooks/usePan.ts` | FALSE POSITIVE — `usePan` already uses `attachedRef` to track listeners and cleans them up in an unmount `useEffect`. No change needed. |
|
||||||
| Asset Optimization | Large unoptimized media asset (213KB) found in `public/res`. | `public/res/Lotus.png` | OPEN |
|
| Asset Optimization | Large unoptimized media asset (213KB) found in `public/res`. | `public/res/Lotus.png` | OPEN |
|
||||||
| Data Persistence | Non-atomic `localStorage` updates in session management can lead to inconsistent state. | `cinny/src/app/state/sessions.ts` | OPEN |
|
| Data Persistence | Non-atomic `localStorage` updates in session management can lead to inconsistent state. | `cinny/src/app/state/sessions.ts` | OPEN |
|
||||||
| Data Persistence | Lack of cross-tab synchronization for `localStorage` updates in session management risks race conditions. | `cinny/src/app/state/sessions.ts` | OPEN |
|
| Data Persistence | Lack of cross-tab synchronization for `localStorage` updates in session management risks race conditions. | `cinny/src/app/state/sessions.ts` | OPEN |
|
||||||
| Network Resilience | `uploadContent` lacks retry logic, failing immediately upon network error. | `cinny/src/app/utils/matrix.ts` | OPEN |
|
| Network Resilience | `uploadContent` lacks retry logic, failing immediately upon network error. | `cinny/src/app/utils/matrix.ts` | FIXED (`d2946c00`) — bounded retry (`UPLOAD_MAX_RETRY_COUNT=3`) gated by `isRetryableUploadError` (transient/network/5xx/429 only, not 4xx), reusing the `rateLimitedActions` capped-exponential backoff. |
|
||||||
| Network Resilience | `rateLimitedActions` uses basic retry logic without exponential backoff, which may exacerbate 429 issues. | `cinny/src/app/utils/matrix.ts` | FIXED — fallback delay now uses capped exponential backoff (`min(1000 * 2^retryCount, 30_000)ms`) when server doesn't send `Retry-After`; server header still takes precedence via `getRetryAfterMs()`. |
|
| Network Resilience | `rateLimitedActions` uses basic retry logic without exponential backoff, which may exacerbate 429 issues. | `cinny/src/app/utils/matrix.ts` | FIXED — fallback delay now uses capped exponential backoff (`min(1000 * 2^retryCount, 30_000)ms`) when server doesn't send `Retry-After`; server header still takes precedence via `getRetryAfterMs()`. |
|
||||||
| Matrix Event Robustness | `useMatrixEventRenderer` handles unknown events gracefully by returning `null`, which may hide potentially important unrendered data. | `cinny/src/app/hooks/useMatrixEventRenderer.ts` | OPEN |
|
| Matrix Event Robustness | `useMatrixEventRenderer` handles unknown events gracefully by returning `null`, which may hide potentially important unrendered data. | `cinny/src/app/hooks/useMatrixEventRenderer.ts` | FALSE POSITIVE — returning `null` for unrendered types is the intended contract. Callers opt into rendering unknowns via the `renderStateEvent` / `renderEvent` fallback params; `null` only results when the caller deliberately supplies no fallback. No change warranted. |
|
||||||
| Data Contract | `MatrixError` instantiation with `UploadResponse` might be brittle. | `cinny/src/app/utils/matrix.ts` | OPEN |
|
| Data Contract | `MatrixError` instantiation with `UploadResponse` might be brittle. | `cinny/src/app/utils/matrix.ts` | FIXED (`d2946c00`) — replaced the brittle direct construction with `matrixErrorFromUploadResponse` / `matrixErrorFromUnknown` guards that validate shape before building a `MatrixError`. |
|
||||||
| Type Safety | `addRoomIdToMDirect` uses `as any` cast for `AccountDataEvent.Direct`, bypassing type contract validation. | `cinny/src/app/utils/matrix.ts` | OPEN |
|
| Type Safety | `addRoomIdToMDirect` uses `as any` cast for `AccountDataEvent.Direct`, bypassing type contract validation. | `cinny/src/app/utils/matrix.ts` | FIXED (`d2946c00`) — `addRoomIdToMDirect` / `removeRoomIdFromMDirect` now use `EventType.Direct` + a typed `MDirectContent`, dropping the `as any` cast. |
|
||||||
| Robustness | `rateLimitedActions` relies on `MatrixError.httpStatus` which might not exist on all error variants. | `cinny/src/app/utils/matrix.ts` | FALSE POSITIVE — `MatrixError.httpStatus` is defined as `readonly httpStatus?: number` in `matrix-js-sdk/lib/http-api/errors.d.ts`. It is optional (not on all instances) but the `?.` optional chain already guards against undefined. No change needed. |
|
| Robustness | `rateLimitedActions` relies on `MatrixError.httpStatus` which might not exist on all error variants. | `cinny/src/app/utils/matrix.ts` | FALSE POSITIVE — `MatrixError.httpStatus` is defined as `readonly httpStatus?: number` in `matrix-js-sdk/lib/http-api/errors.d.ts`. It is optional (not on all instances) but the `?.` optional chain already guards against undefined. No change needed. |
|
||||||
| Type Contract | Custom types in `cinny/src/types/matrix` mirror SDK types instead of using them, risking drift and contract mismatches. | `cinny/src/types/matrix/` | OPEN |
|
| Type Contract | Custom types in `cinny/src/types/matrix` mirror SDK types instead of using them, risking drift and contract mismatches. | `cinny/src/types/matrix/` | OPEN |
|
||||||
|
|
||||||
@@ -193,10 +193,10 @@ This document tracks identified bugs, edge cases, and architectural discrepancie
|
|||||||
| Localization | Hardcoded UI string: "Upload Failed" | `src/app/components/upload-board/UploadBoard.tsx` | OPEN |
|
| Localization | Hardcoded UI string: "Upload Failed" | `src/app/components/upload-board/UploadBoard.tsx` | OPEN |
|
||||||
| Localization | Hardcoded UI string: "Password" | `src/app/components/uia-stages/PasswordStage.tsx` | OPEN |
|
| Localization | Hardcoded UI string: "Password" | `src/app/components/uia-stages/PasswordStage.tsx` | OPEN |
|
||||||
| Bundle Size | Large unoptimized media asset (213KB) | `public/res/Lotus.png` | OPEN |
|
| Bundle Size | Large unoptimized media asset (213KB) | `public/res/Lotus.png` | OPEN |
|
||||||
| Matrix Logic | Inefficient repeated `mx.getRoom()` calls in component render loops | `src/app/features/lobby/Lobby.tsx` | OPEN |
|
| Matrix Logic | Inefficient repeated `mx.getRoom()` calls in component render loops | `src/app/features/lobby/Lobby.tsx` | FALSE POSITIVE — `Lobby` already routes its render loop through the memoized `useGetRoom(allJoinedRooms)`. The two remaining `mx.getRoom()` calls are inside drag/drop event handlers (not render loops) and are O(1) SDK map lookups. No change warranted. |
|
||||||
| Matrix Logic | Inefficient repeated `mx.getRoom()` calls in component render loops | `src/app/components/emoji-board/EmojiBoard.tsx` | OPEN |
|
| Matrix Logic | Inefficient repeated `mx.getRoom()` calls in component render loops | `src/app/components/emoji-board/EmojiBoard.tsx` | FIXED (`b7e1f89c`) — pack-label `mx.getRoom()` lookups in `EmojiSidebar`/`StickerSidebar` hoisted into a `useMemo`'d `Map` built once per pack list. |
|
||||||
| Performance | Numerous event handlers (e.g., handleUserClick, handleReplyClick) lack `useCallback`, leading to unnecessary re-renders of message components. | `cinny/src/app/features/room/RoomTimeline.tsx` | OPEN |
|
| Performance | Numerous event handlers (e.g., handleUserClick, handleReplyClick) lack `useCallback`, leading to unnecessary re-renders of message components. | `cinny/src/app/features/room/RoomTimeline.tsx` | FIXED (`b7e1f89c`) — `handleJumpToLatest`/`handleJumpToUnread`/`handleMarkAsRead` wrapped in `useCallback`. |
|
||||||
| Performance | The `submit` function and file handling callbacks (e.g., handleSendUpload) are re-created on every render, causing re-renders of the editor and toolbar components. | `cinny/src/app/features/room/RoomInput.tsx` | OPEN |
|
| Performance | The `submit` function and file handling callbacks (e.g., handleSendUpload) are re-created on every render, causing re-renders of the editor and toolbar components. | `cinny/src/app/features/room/RoomInput.tsx` | FIXED (`b7e1f89c`) — `handleCancelUpload`/`handleSendUpload`/`handleShareLocation`/`handleEmoticonSelect`/`handleStickerSelect` wrapped in `useCallback`. |
|
||||||
| Accessibility | `button` for edit history lacks `aria-label` | `cinny/src/app/components/message/content/FallbackContent.tsx` | FIXED ⚠️ UNTESTED — added `aria-label="View edit history"` |
|
| Accessibility | `button` for edit history lacks `aria-label` | `cinny/src/app/components/message/content/FallbackContent.tsx` | FIXED ⚠️ UNTESTED — added `aria-label="View edit history"` |
|
||||||
| Accessibility | `button` for reaction lacks `aria-label` | `cinny/src/app/components/message/Reaction.tsx` | **FIXED ⚠️ UNTESTED** — `Reaction` component now computes `aria-label="{shortcode} reaction, N people"` internally using `getShortcodeFor`; custom (mxc://) emoji falls back to "custom emoji reaction". |
|
| Accessibility | `button` for reaction lacks `aria-label` | `cinny/src/app/components/message/Reaction.tsx` | **FIXED ⚠️ UNTESTED** — `Reaction` component now computes `aria-label="{shortcode} reaction, N people"` internally using `getShortcodeFor`; custom (mxc://) emoji falls back to "custom emoji reaction". |
|
||||||
| Accessibility | `button` for ThreadIndicator lacks `aria-label` | `cinny/src/app/components/message/Reply.tsx` | FIXED ⚠️ UNTESTED — added `aria-label="View thread"` |
|
| Accessibility | `button` for ThreadIndicator lacks `aria-label` | `cinny/src/app/components/message/Reply.tsx` | FIXED ⚠️ UNTESTED — added `aria-label="View thread"` |
|
||||||
@@ -226,18 +226,18 @@ This document tracks identified bugs, edge cases, and architectural discrepancie
|
|||||||
| Matrix SDK | Inefficient listener management (`setMaxListeners: 150`) and incomplete SDK state transition handling. | `src/client/initMatrix.ts` | OPEN |
|
| Matrix SDK | Inefficient listener management (`setMaxListeners: 150`) and incomplete SDK state transition handling. | `src/client/initMatrix.ts` | OPEN |
|
||||||
| PWA Robustness | Service worker lacks caching strategy for application assets, resulting in no offline capability. | `cinny/src/sw.ts` | OPEN |
|
| PWA Robustness | Service worker lacks caching strategy for application assets, resulting in no offline capability. | `cinny/src/sw.ts` | OPEN |
|
||||||
| PWA Integrity | `manifest: false` in `vite.config.js` might prevent correct PWA installation if not handled externally. | `cinny/vite.config.js` | OPEN |
|
| PWA Integrity | `manifest: false` in `vite.config.js` might prevent correct PWA installation if not handled externally. | `cinny/vite.config.js` | OPEN |
|
||||||
| PII Leakage | Potential PII exposure via console.error (parameter e likely contains event data). | `cinny/src/app/plugins/call/CallEmbed.ts` | OPEN |
|
| PII Leakage | Potential PII exposure via console.error (parameter e likely contains event data). | `cinny/src/app/plugins/call/CallEmbed.ts` | VERIFIED COMPLIANT — reviewed during the logging pass (`203568c9`); the existing log path already records only `e.message`, not raw event payloads. No change needed. |
|
||||||
| PII Leakage | Potential PII exposure via console.warn (parameter imgError/videoError/thumbError object). | `cinny/src/app/features/room/msgContent.ts` | OPEN |
|
| PII Leakage | Potential PII exposure via console.warn (parameter imgError/videoError/thumbError object). | `cinny/src/app/features/room/msgContent.ts` | FIXED (`203568c9`) — media-error warnings now log only `error.name` + `error.message`, never the raw error/event object. |
|
||||||
| PII Leakage | Potential PII exposure via console.error (parameter e likely contains event data). | `cinny/src/app/features/room/RoomInput.tsx` | OPEN |
|
| PII Leakage | Potential PII exposure via console.error (parameter e likely contains event data). | `cinny/src/app/features/room/RoomInput.tsx` | VERIFIED COMPLIANT — reviewed during the logging pass (`203568c9`); the existing log path already records only `e.message`. No change needed. |
|
||||||
|
|
||||||
## 🏗️ Architectural & Resilience Audit
|
## 🏗️ Architectural & Resilience Audit
|
||||||
|
|
||||||
| Category | Issue Description | File Path | Status |
|
| Category | Issue Description | File Path | Status |
|
||||||
| :----------------------- | :----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | :------------------------------------------------------ | :----- |
|
| :----------------------- | :----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | :------------------------------------------------------ | :----- |
|
||||||
| Element Call Integration | Lacks robust iframe failure monitoring beyond initial 'preparing' event; can result in a permanently hung 'Loading...' state with no user-visible error or recovery path. | `src/app/plugins/call/CallEmbed.ts` | OPEN |
|
| Element Call Integration | Lacks robust iframe failure monitoring beyond initial 'preparing' event; can result in a permanently hung 'Loading...' state with no user-visible error or recovery path. | `src/app/plugins/call/CallEmbed.ts` | FIXED (`0394fce9`) — added a `CALL_LOAD_WATCHDOG_MS` (25s) timeout that settles on ready/capabilities/joined and fails on iframe error/timeout, exposing a `loadFailed` getter + `onLoadError(cb)`. `CallView` renders a `CallLoadErrorMessage` overlay (Retry/Leave) instead of a permanent spinner. ⚠️ UNTESTED — needs a live call. |
|
||||||
| Component Resilience | `RoomTimeline` has no `ErrorBoundary` wrapper — a single malformed event crashing the renderer takes down the entire timeline with no fallback UI. | `src/app/features/room/RoomTimeline.tsx` | OPEN |
|
| Component Resilience | `RoomTimeline` has no `ErrorBoundary` wrapper — a single malformed event crashing the renderer takes down the entire timeline with no fallback UI. | `src/app/features/room/RoomTimeline.tsx` | FALSE POSITIVE — `RoomView.tsx` (lines 113–137) already wraps `<RoomTimeline>` in a react-error-boundary `ErrorBoundary` with a "Timeline unavailable" fallback. A wave-1 agent's redundant nested boundary was reverted. No change needed. |
|
||||||
| Component Resilience | `RoomInput` has no `ErrorBoundary` wrapper — a crash in the composer leaves users unable to send messages. | `src/app/features/room/RoomInput.tsx` | OPEN |
|
| Component Resilience | `RoomInput` has no `ErrorBoundary` wrapper — a crash in the composer leaves users unable to send messages. | `src/app/features/room/RoomInput.tsx` | FALSE POSITIVE — `RoomView.tsx` (lines 151–171) already wraps `<RoomInput>` in an `ErrorBoundary` with a "Message composer encountered an error" `RoomInputPlaceholder` fallback. No change needed. |
|
||||||
| Fallback Logic | No explicit empty/error fallback for Matrix SDK data calls in `RoomTimeline`; relies purely on SDK internal error propagation, meaning silent failures show a blank timeline. | `src/app/features/room/RoomTimeline.tsx` | OPEN |
|
| Fallback Logic | No explicit empty/error fallback for Matrix SDK data calls in `RoomTimeline`; relies purely on SDK internal error propagation, meaning silent failures show a blank timeline. | `src/app/features/room/RoomTimeline.tsx` | ADDRESSED — the `RoomView` `ErrorBoundary` (above) provides the explicit render-error fallback; a thrown SDK/render error now surfaces "Timeline unavailable" rather than a blank timeline. |
|
||||||
| Dependency | Potential for complex dependency chains due to deep nesting in `src/app/features/` and `src/app/hooks/`. | `src/app/` | OPEN |
|
| Dependency | Potential for complex dependency chains due to deep nesting in `src/app/features/` and `src/app/hooks/`. | `src/app/` | OPEN |
|
||||||
| Hydration/Race Condition | The SyncState listener registered by useSyncState may miss the initial 'PREPARED' event if the client initializes synchronously from IndexedDB before the effect runs, leading to an infinite loading state. | `cinny/src/app/pages/client/ClientRoot.tsx` | OPEN |
|
| Hydration/Race Condition | The SyncState listener registered by useSyncState may miss the initial 'PREPARED' event if the client initializes synchronously from IndexedDB before the effect runs, leading to an infinite loading state. | `cinny/src/app/pages/client/ClientRoot.tsx` | OPEN |
|
||||||
| Structure | High number of small, highly coupled utility hooks (`src/app/hooks/`) may obscure dependency graphs. | `src/app/hooks/` | OPEN |
|
| Structure | High number of small, highly coupled utility hooks (`src/app/hooks/`) may obscure dependency graphs. | `src/app/hooks/` | OPEN |
|
||||||
|
|||||||
Reference in New Issue
Block a user