Skip to content

feat: consolidate local branch cleanup#25

Closed
Just-Insane wants to merge 738 commits into
devfrom
codex/combined-cleanup-no-launchpad
Closed

feat: consolidate local branch cleanup#25
Just-Insane wants to merge 738 commits into
devfrom
codex/combined-cleanup-no-launchpad

Conversation

@Just-Insane

Copy link
Copy Markdown
Collaborator

Description

Consolidates the local feature and fix branches into one cleaned branch, excluding the incomplete LaunchPad branch.

The cleanup resolves overlapping branch changes, removes stale/dead merge artifacts caught during validation, rebuilds service-worker/media/session handling coherently, restores settings and notification paths, and keeps Tauri/personal configuration changes in the rebuilt integration set.

Changeset: feat: consolidate local feature and fix branches

Related context from pre-PR search:

Fixes #

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

Validation:

  • pnpm lint (passes with the existing 8 warnings)
  • pnpm fmt:check
  • pnpm typecheck
  • pnpm test:run
  • pnpm knip
  • pnpm build (passes with existing Vite chunk/dynamic-import warnings)

AI disclosure:

  • Partially AI assisted (clarify which code was AI assisted and briefly explain what it does).
  • Fully AI generated (explain what all the generated code does in moderate detail).

AI assistance was used for branch consolidation, conflict resolution, cleanup of stale merged code, and validation fixes across media rendering, service-worker session handling, settings, notifications, routing, and tests.

- Thread events: route to correct timeline set with threadId
- TURN server: stop polling unavailable endpoint
- config.json: add retry logic with exponential backoff
- Event fetch: verify exists before timeline jump
- Thread events: route to correct timeline set with threadId
- TURN server: only advertise capability when homeserver provides TURN
- config.json: add retry logic with exponential backoff
- Event fetch: verify exists before timeline jump
Previously, permalink/bookmark jumps would show 'Jumped to latest messages'
banner and fall back to live timeline immediately when the /context endpoint
returned a disconnected fragment. This was incorrect — disconnected fragments
are a VALID and EXPECTED response for old permalinks.

Changes:
- Remove containsLive check and all fallback logic from useEventTimelineLoader
- Render disconnected fragments successfully (pagination connects them)
- Increase timeout from 12s to 20s to allow slower servers more time
- Banner now only shows for actual failures (404, network error, timeout)
- Remove onDisconnectedFragment callback (no longer needed)

The correct flow is now:
1. Show skeleton/loading state
2. Fetch timeline via GET /context
3. If succeeds → render fragment (even if disconnected from live)
4. Only show banner if /context fails (404, network error, or 20s timeout)

Disconnected fragments are normal for old messages and will connect via
pagination as the user scrolls. The premature fallback was causing the app
to jump away from the target message before even attempting to load it.
Previously, when a permalink/bookmark jump succeeded:
1. focusItem was set → event scrolled into view
2. After highlight ended (~2-4s), focusItem was cleared
3. This triggered the recovery scroll effect to run again
4. It started a 1-second timer
5. Timer fired and scrolled to bottom, even though jump succeeded

Now we track whether the jump ever succeeded (focusItem was set) and
skip recovery scroll once jumpSucceededRef is true. The ref is reset
only when the eventId or room changes, so it persists across the
focusItem lifecycle (set → scrollTo:false → cleared).

This ensures recovery scroll only fires for genuinely failed jumps
(404, network error, timeout), not successful ones where the highlight
simply ended.
Fixes two causes of jumpy permalink navigation where media loads
cause the scroll position to drift down to the bottom:

1. CSS overflow-anchor: Add 'overflow-anchor: none' to the timeline
   scroll container. The browser now uses each message element as a
   scroll anchor (via 'overflow-anchor: auto' set on [data-message-id]),
   preventing layout shifts when content above the viewport changes size.

2. Reserve image dimensions: Pass width/height from Matrix event metadata
   (info.w / info.h) to the <img> element so it reserves space before
   loading. Images no longer cause layout shifts when they arrive.

Additionally, reset wasAtBottomBeforePaginationRef when starting a
permalink jump. This prevents the pagination completion handler from
scrolling to bottom after loading history for the jump target.
1. Scroll position & CLS tracking: jump_landed_offset_px, jump_cls metrics with PerformanceObserver
2. Cold cache handoff: already present (commit 3496a40), cold_cache_duration_ms metric
3. Pagination breadcrumbs: start/complete with direction, counts, tokens
4. Thread drop counter: per-event + per-sync metrics with room_id/reason tags
5. Jump re-entrancy: jump_superseded metric, aborted jump breadcrumbs
6. Room list phases: initial_sync_ms, member_lists_ms, notifications_ms metrics
7. Media span attributes: type/size/dimensions/authenticated/cache_hit/server

Fixes: Sentry API (tags→attributes, increment→count), thread routing (check getThread), addToState parameter
1. Media decryption integrity check (SABLE-5A):
   - Verify SHA-256 digest after decryption against encInfo.hashes.sha256
   - Capture mismatch to Sentry with diagnostic context (event/room/hash prefix)
   - Throw MediaIntegrityError for calling code to handle gracefully
   - Added doc comment confirming each decryptFile call uses independent
     key material and IV/counter state (no shared mutable state across
     concurrent decryptions from main client + search backfill worker)

2. Emergency stale service worker unregister (SABLE-2B):
   - Check sw.js availability at app startup (HEAD request)
   - Unregister any active SW if sw.js returns 404
   - Self-heals for users with broken SW registration without full cache clear
   - Non-blocking: logs failures but continues app init regardless
   - Uses Promise.allSettled to parallelize checks (fixes no-await-in-loop lint)
…ents

- Add decryptFileSafe wrapper that catches MediaIntegrityError
- Update all 8 media component call sites to use decryptFileSafe
- Return null on integrity failure to allow broken media placeholder
- Skip thread reply events in search backfill to prevent phantom results

Addresses Sentry feedback: MediaIntegrityError now caught at call sites
rather than crashing the timeline. Thread events from pagination are
skipped during search indexing since they can't be jumped to from search
results (SDK rejects them when adding to room timeline).

Refs: SABLE-5A, Sentry production error analysis
The synthetic 401 Response returned when no session is available was being
passed to decryptAttachment() for encrypted media requests. The decryption
pipeline would try to decrypt the JSON body '{"errcode":"M_MISSING_TOKEN",...}'
as if it were encrypted media bytes, causing SHA-256 hash verification to fail.

Fix: detect actual media download URLs (download/thumbnail endpoints) and let
the real network error propagate for those requests. Only return synthetic 401
for non-media endpoints (preview_url, metadata, etc.) where the JSON body is
appropriate.

This resolves the MediaIntegrityError cascade that started appearing after the
SABLE-4Y session fallback fix was deployed.

Refs: SABLE-5A (hash verification issue root cause)
…ation

- Remove validateSession: let 401s fail naturally instead of pre-validating
- Remove requestSessionWithTimeout: no more 10s timeout waiting for client
- Remove complex fallback chains: just try live → widget → persisted → passthrough
- Remove synthetic 401 responses: let real errors surface
- Remove global 15s timeout with Promise.race

Keep:
- iOS PWA session persistence via preloadedSession
- Widget auth by baseUrl matching (Element Call, etc)
- Session persistence across SW restarts

Now ~40 lines instead of 120+ lines in fetch handler.
Real 401 errors propagate to downloadMedia() which logs to Sentry.
UI shows error with retry button - no token refresh yet.

Part of SW simplification (Option 3) from architecture review.
- Reduce SW appIsVisible cache window from 10s to 2s to fix push
  notifications not showing when phone is locked with PWA open
  (Issue: iOS kills SW, then restores stale visible state from cache,
  causing push to be incorrectly suppressed)

- Add isLoud (loudByRule) to in-app banner logic so rooms set to
  'All Messages' with sound enabled always show banners, not just
  mentions/keywords/DMs

Fixes notification visibility for loud-notification rooms and improves
push notification reliability on iOS PWA.
Add hybrid badge display mode that shows counts for rooms set to
'All Messages' while showing plain dots for rooms set to
'Mentions & Keywords'.

Badge behavior:
- Highlighted messages (mentions/keywords): green badge with count
- Loud rooms (All Messages): grey badge with count
- Non-loud rooms (Mentions & Keywords): grey dot

New setting: showLoudRoomCounts (default: false)
- Added to settings.ts
- Passed as 'loud' parameter to UnreadBadge and SidebarUnreadBadge
- Updated RoomNavItem to pass notification mode
- Added UI toggle in Settings → Notifications → Badges

This gives users more granular control over which rooms show count
badges vs. plain dots based on notification levels.
BREAKING FIX: SHA-256 verification was checking the hash of the
DECRYPTED data, but Matrix spec requires verifying the hash of the
ENCRYPTED bytes (ciphertext) as they were uploaded to the server.

Additionally, the hash comparison was using URL-safe base64 encoding
(used for keys/IVs), but Matrix spec requires standard base64 encoding
(with padding) for file.hashes.sha256.

Changes:
- Verify hash of encrypted dataBuffer, not decrypted output
- Use standard base64 encoding for hash (encodeBase64Standard)
- Add encodeBase64Standard helper function (with padding)
- Add temporary diagnostic logging to debug mismatches:
  * Downloaded byte count
  * Expected vs actual SHA-256 hashes
  * Match result

This should fix the genuine SHA-256 mismatch errors that were
occurring even with cleared caches. The diagnostic logging will
help identify any remaining issues with upload-time hash calculation
or server-side truncation.
fix(media): strip base64 padding in SHA-256 comparison

- Loud rooms now only show counts when showLoudRoomCounts is enabled
- Non-loud rooms controlled by showUnreadCounts setting
- Matrix spec stores hashes as unpadded base64, strip padding before comparison
- Add test cases for loud room badge behavior
- Rooms on Default notification mode (Unset) now considered loud for badge toggle
- Updated description to clarify it applies to both 'All Messages' and default level
- This allows the toggle to work when global defaults are set to 'Notify Loud'
Adds three focus modes (Off, Focus, DND) that filter notifications without modifying notification settings:
- Off: Show all notifications (no filtering)
- Focus: Show DMs and mentions/keywords only
- Do Not Disturb: Show critical messages (highlights) only

Focus mode UI is accessible via account switcher menu between Status and Settings sections.

Changes:
- Add focusMode setting ('off' | 'focus' | 'dnd') to settings.ts
- Create focusMode.ts with shouldShowNotificationInFocusMode() filter
- Apply filter in ClientNonUIFeatures for in-app and OS notifications
- Apply filter in BackgroundNotifications for background sessions
- Apply filter in UnreadBadge for badge visibility
- Add Focus Mode UI section to AccountSwitcherTab
- Update UnreadBadge tests with focusMode parameter
…nection banner

- Bump SW_MEDIA_CACHE from v1 to v2 to force clearing old cached media responses that may contain SHA-256 padding bugs
- Fix SyncStatus banner to only show "Connected!" when recovering from actual connection issues (Error/Reconnecting states), not on normal startup
- Add 400 error handling to blob cache for federated media failures (SABLE-4X)
- Remove diagnostic console.log statements from SHA-256 verification in matrix.ts

Closes SABLE-5A, SABLE-5F (media hanging)
Closes SABLE-4X (blob cache 400 errors)
Closes SABLE-1K (connection banner on startup)
…currency

- Add .catch() handlers to all SW FetchEvent.respondWith() calls to prevent unhandled network failures from surfacing as browser errors (SABLE-2B)
- Increase profile fetch concurrency from 3 to 6 to reduce N+1 pattern impact while maintaining headroom for timeline requests (SABLE-2)
- Network-level fetch failures now fall through gracefully instead of throwing FetchEvent.respondWith errors

Closes SABLE-2B (38 SW fetch errors)
Improves SABLE-2 (916 N+1 profile fetches)
…handling

Add explicit error filtering for NotAllowedError in incoming/outgoing call ringtone playback. This prevents autoplay permission errors from being logged when there's no recent user gesture, which is expected browser behavior.

Closes SABLE-2K (15 events)

Notes on other reported issues:
- SABLE-1J: Retry UI already exists in Message.tsx (lines 828-842). Matrix SDK manages event queue internally; events marked NOT_SENT will show retry button.
- SABLE-46: Error occurs in Matrix SDK's GroupCallEventHandler.evaluateEventBuffer when event.getType() returns undefined. SDK-level issue; Sable has no similar pattern to guard.
- SABLE-1B: Generic TypeError likely resolves with connection persistence fixes from commit 1311434. Monitor after those ship.
…om Counts

Filter child rooms by notification mode before aggregating unreads for space badges. Only rooms with Default (Unset) or All Messages notification modes are considered "loud". Rooms set to "Mentions and Keywords Only" or "Mute" are excluded from the count.

This fixes the issue where space badges were showing counts for all unread rooms regardless of notification mode, even when "Show Loud Room Counts" was enabled.

Changes:
- SpaceTab: Filter allChild by notification mode before passing to useRoomsUnread
- ClosedSpaceFolder: Filter folder.content by notification mode before passing to RoomsUnreadProvider
- Calculate hasLoudUnreads based on whether filtered rooms have unreads
- Pass hasLoudUnreads to SidebarUnreadBadge instead of hardcoded true
Fixed two issues causing frequent "Connecting.../Connected!" banners:

1. **Removed duplicate SW activate handler** (lines 1204-1212)
   - The duplicate called clients.claim() which contradicted the iOS bfcache fix in the first activate handler
   - This was causing iOS PWA to show splash screen instead of instant restore

2. **Made app visibility retry logic conditional** (useAppVisibility.ts)
   - Added getSyncState() check before calling retryImmediately()
   - Only retry if sync state is ERROR, STOPPED, or RECONNECTING
   - Prevents unnecessary reconnection attempts when app is already syncing normally
   - Fixes desktop reconnect banner when app regains focus

The aggressive retry-on-foreground behavior was added for mobile wake/restore but was too broad - it triggered even when sync was healthy. Now retry only happens when actually needed.
… initialized

feat(telemetry): add Sentry metrics for pusher and push notification tracking

- Add settingsInitializedAtom to track when settings are fully loaded (localStorage + account data)
- Defer theme application in AuthRouteThemeManager until settings initialized
- Add 100ms grace period after account data check to complete settings merge
- Track pusher enable/disable events with reason tags (app_visible, kept_active_when_visible, etc.)
- Track push notification arrival, suppression, and decryption relay outcomes in SW
- Add breadcrumbs for pusher state changes to aid debugging

The theme flash occurred because settings sync from account data could overwrite
localStorage after ThemeManager had already applied the theme. Now we wait until
both sources are checked before applying theme classes.

Pusher telemetry will help diagnose notification delivery and decryption timing
issues, especially when the PWA is backgrounded.
…rypted listener leak

fix(crypto): track missing room keys in backgrounded push decryption

- Add sable.push.registration metric (outcome: created/reused/failed)

- Add sable.notification.displayed/display_failed metrics

- Add sable.push.decrypt_missing_keys metric

- Track pendingDecryptListeners Map to clean up listeners on unmount

- Add Sentry breadcrumbs for push registration failures

The Event.decrypted listener leak occurred because MessageNotifications registers .once() listeners that weren't cleaned up if component unmounted before decryption.
… only

Previously we only showed space badges when there were loud (Default/All Messages) room unreads. Now we show badges for all unreads, but use the loud parameter to control whether to show counts (for loud rooms) or dots (for quiet rooms like mentions-only or muted).

This ensures spaces always show an unread indicator when there are unreads, while respecting the 'Show Loud Room Counts' setting to display numeric counts only for rooms with Default or All Messages notification modes.
…ions

The selector in useRoomsUnread was recreated on every rooms array reference change, even when contents were identical. This caused stale closures and race conditions in jotai's selectAtom memoization, leading to inconsistent space badge visibility.

Fixed by using a stable string key (roomsKey) derived from array contents instead of the array reference itself. This ensures the selector is only recreated when room IDs actually change, not just when the array reference changes.

Fixes issue where clearing cache made more space badges appear - the race condition was resolved differently depending on timing of atom updates.
Added console.log statements to track:

- Space children detection in SpaceTabs

- Room unread map entries in useRoomsUnread

- Cases where rooms exist but have no unread data

This is temporary debug code to investigate why space badges don't show for rooms set to 'Mentions and Keywords Only' notification mode.
Added detailed logging in getRoomsUnread to show:

- All room IDs being queried

- Full roomToUnread map keys

- Whether each queried room is in the map

- The actual unread values for each room

- The final computed result

Also added loudChildIds and willShowBadge to SpaceTabs debug output.
Added console.log statements to trace:

- When useBindRoomToUnreadAtom initializes the atom

- What transport mode is being used (classic vs sliding sync)

- How many rooms getUnreadInfos processes

- Detailed output for Draupnir rooms specifically

- What unread info values are calculated for those rooms

- Whether rooms pass the 'total > 0 || highlight > 0' filter

This will help determine if the rooms are being excluded from the unread map during initialization or if the map is populated but queries see an empty snapshot.
Comment thread src/app/utils/room.ts
Comment thread src/app/features/settings/notifications/NativePushNotifications.ts
@github-actions

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

Status Preview URL Commit Alias Updated (UTC)
✅ Deployment successful! https://pr-25-sable.justin-tech.workers.dev 0a47082 pr-25 Thu, 11 Jun 2026 16:23:53 GMT

Comment on lines 348 to 354
}
const shouldSkipFocusCheck = eventId && skipFocusCheckEvents.has(eventId);
if (!shouldSkipFocusCheck) {
if (document.hasFocus() && (selectedRoomId === room?.roomId || notificationSelected))
return;
if (document.hasFocus() && notificationSelected) return;
}

// Older sliding sync proxies (e.g. matrix-sliding-sync) omit num_live,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Notifications will now fire for new messages in a room that the user is actively viewing because the check for the currently selected room was removed.
Severity: MEDIUM

Suggested Fix

Restore the selectedRoomId === room?.roomId check to the if condition to correctly suppress notifications for messages arriving in the currently viewed room. The condition should be if (document.hasFocus() && (selectedRoomId === room?.roomId || notificationSelected)) return;.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/app/pages/client/ClientNonUIFeatures.tsx#L348-L354

Potential issue: The removal of the `selectedRoomId === room?.roomId` check from the
notification suppression logic causes an issue. Previously, notifications were
suppressed if the user was focused on the app and either viewing the room where the
message arrived or viewing the notifications inbox. The new logic only suppresses
notifications if the user is viewing the notifications inbox. As a result, when a user
is actively viewing a room and a new message arrives in that same room, a desktop/sound
notification will be triggered, which is redundant and disruptive.

Comment on lines 283 to 293
const countAfter = getTimelinesEventsCount(getLinkedTimelines(firstTimeline));
const fetched = countAfter - countBefore;

let willContinue = false;
if (fetched > 0 && fetched < 5) {
const checkTimeline = backwards
? freshLTimelines[0]
: freshLTimelines[freshLTimelines.length - 1];
if (!checkTimeline) {
(backwards ? setBackwardStatus : setForwardStatus)('idle');
return;
}
if (!checkTimeline) return;
const checkDirection = backwards ? Direction.Backward : Direction.Forward;
const stillHasToken =
typeof getLinkedTimelines(checkTimeline)[0]?.getPaginationToken(checkDirection) ===

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The pagination loading spinner flickers during auto-continuation because the status is prematurely set to 'idle' between consecutive fetch requests.
Severity: LOW

Suggested Fix

Do not set the pagination status to 'idle' if an auto-continuation is about to occur. Check if a recursive pagination call will be made before updating the status, similar to the logic in the previous version of the code which used a willContinue flag.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/app/hooks/timeline/useTimelineSync.ts#L276-L293

Potential issue: During timeline pagination, when fetching history requires multiple
consecutive requests (auto-continuation), the pagination status is prematurely set to
`'idle'` after the first request completes. Immediately after, a recursive call to
`paginate` sets the status back to `'loading'`. This state change from `'loading'` to
`'idle'` and back to `'loading'` between requests causes the loading spinner in the UI
to flicker, creating a disruptive user experience. The previous implementation correctly
avoided this by checking if continuation was needed before setting the status to
`'idle'`.

Comment thread src/app/utils/room.ts
latestEvent &&
!latestEvent.isSending() &&
latestEvent.getSender() === userId &&
isNotificationEvent(latestEvent)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The call to isNotificationEvent is missing the room and userId arguments, causing it to incorrectly handle reaction events and leading to phantom unread badges.
Severity: LOW

Suggested Fix

Update the call to isNotificationEvent at line 323 to include the room and userId arguments, which are available in the surrounding scope. The corrected call should be isNotificationEvent(latestEvent, room, userId). This will allow the function to correctly identify reactions and suppress unread counts as intended.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/app/utils/room.ts#L323

Potential issue: In `src/app/utils/room.ts`, the phantom-unread-suppression logic calls
`isNotificationEvent(latestEvent)` without the `room` and `userId` arguments. The
function's internal logic requires these arguments to correctly classify reaction events
(`m.annotation`). When they are missing, the function defaults to returning `false` for
reactions. This prevents the suppression logic from working correctly when a user's own
reaction is the latest event in a room, resulting in a phantom unread badge that should
have been suppressed.

Also affects:

  • src/app/features/notifications/BackgroundNotifications.tsx:436
  • src/app/features/client/ClientNonUIFeatures.tsx:397
  • src/app/organisms/SyncDiagnostics.tsx:85
  • src/app/organisms/SyncDiagnostics.tsx:92

@Just-Insane

Copy link
Copy Markdown
Collaborator Author

Closing as obsolete under the integration-trunk workflow. The code is already represented on integration and the narrower tracking PRs now target integration directly.

@Just-Insane
Just-Insane deleted the codex/combined-cleanup-no-launchpad branch June 12, 2026 01:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants