fix(calls,matrix): address review findings from agent code review

- CallEmbed watchdog now SELF-HEALS: a genuine ready/joined signal arriving
  after the 25s timeout clears the error and notifies subscribers with
  undefined, so a slow-but-successful EC load no longer strands the user on
  the recovery screen over a live call. Listener dispatch wrapped in try/catch.
- ringtones: synth notes route through a per-session master gain; stop() ramps
  it to 0 so the ring is silenced instantly on answer instead of letting the
  last scheduled phrase ring out over call audio.
- IncomingCallBanner: ping fires exactly once per incoming call (guarded by
  refEventId) instead of re-pinging when ringtone settings change mid-banner.
- focusCameraParticipant: try multiple tile selectors (EC labels vary by
  version), defer the tile click past EC's async spotlight layout switch
  (rAF x2), and dev-warn when no tile matches so testers get signal.
- uploadContent: a cancelled upload (mx.cancelUpload -> AbortError) is no
  longer treated as retryable — previously the retry loop could resurrect an
  upload the user just cancelled. Also retry on 408.
- addRoomIdToMDirect/removeRoomIdFromMDirect: guard against a corrupt m.direct
  whose values aren't arrays.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

src/app/components/CallEmbedProvider.tsx

@@ -289,11 +289,16 @@ function IncomingCallBanner({ dm, info, onIgnore, onAnswer, onReject }: Incoming
   );
 
   // Single soft ping (non-looping) on arrival, respecting the chosen ringtone
-  // + volume. We intentionally do NOT loop here — the user is mid-call.
+  // + volume. We intentionally do NOT loop here — the user is mid-call — and we
+  // ping exactly once per incoming call, not again if the user happens to tweak
+  // ringtone settings while the banner is showing.
+  const pingedRef = useRef<string | undefined>(undefined);
   useEffect(() => {
     if (info.notificationType !== 'ring') return;
+    if (pingedRef.current === info.refEventId) return;
+    pingedRef.current = info.refEventId;
     previewRingtone(ringtoneId, Math.max(0, Math.min(1, ringtoneVolume / 100)));
-  }, [info.notificationType, ringtoneId, ringtoneVolume]);
+  }, [info.notificationType, info.refEventId, ringtoneId, ringtoneVolume]);
 
   useEffect(() => {
     const remaining = info.senderTs + info.lifetime - Date.now();

src/app/plugins/call/CallControl.ts

@@ -356,20 +356,48 @@ export class CallControl extends EventEmitter implements CallControlState {
     const doc = this.document;
     if (!doc) return;
 
-    // Find the mute icon / aria-label element that identifies this participant
+    // EC labels participant tiles inconsistently across versions — the user's
-    const userEl = doc.querySelector<HTMLElement>(`[aria-label="${CSS.escape(userId)}"]`);
+    // matrix id may be the full aria-label, a substring of it, or carried on a
-    // Walk up to the nearest video tile container
+    // data attribute (and sometimes the visible label is the display name, not
-    const tile =
+    // the id at all). Try several strategies before giving up, then walk up to
-      userEl?.closest<HTMLElement>('[data-testid="videoTile"]') ??
+    // the enclosing video tile.
-      userEl?.closest<HTMLElement>('[data-video-fit]');
+    const findTile = (): HTMLElement | undefined => {
+      const escaped = CSS.escape(userId);
+      const el =
+        doc.querySelector<HTMLElement>(`[aria-label="${escaped}"]`) ??
+        doc.querySelector<HTMLElement>(`[data-testid="videoTile"][aria-label*="${escaped}"]`) ??
+        doc.querySelector<HTMLElement>(`[aria-label*="${escaped}"]`) ??
+        doc.querySelector<HTMLElement>(`[data-member-id="${escaped}"]`) ??
+        doc.querySelector<HTMLElement>(`[data-id="${escaped}"]`) ??
+        undefined;
+      return (
+        el?.closest<HTMLElement>('[data-testid="videoTile"]') ??
+        el?.closest<HTMLElement>('[data-video-fit]') ??
+        el ??
+        undefined
+      );
+    };
 
-    if (!this.spotlight) {
+    const applyFocus = () => {
-      this.spotlightButton?.click();
+      const tile = findTile();
+      if (tile) {
+        tile.click();
+      } else if (import.meta.env.DEV) {
+        console.warn(`[CallControl] focusCameraParticipant: no tile matched ${userId}`);
+      }
+    };
+
+    if (this.spotlight) {
+      // Already in spotlight — pin immediately.
+      applyFocus();
+      return;
     }
 
-    if (tile) {
+    // Switching to spotlight re-renders EC's layout asynchronously; clicking the
-      tile.click();
+    // tile in the same tick would land in the old (grid) DOM. Toggle spotlight,
-    }
+    // then click on a later frame once the spotlight tiles have mounted.
+    this.spotlightButton?.click();
+    requestAnimationFrame(() => requestAnimationFrame(applyFocus));
   }
 
   public dispose() {

src/app/plugins/call/CallEmbed.ts

@@ -70,7 +70,9 @@ export class CallEmbed {
 
   private loadError?: CallLoadErrorReason;
 
-  private readonly loadErrorListeners = new Set<(reason: CallLoadErrorReason) => void>();
+  private readonly loadErrorListeners = new Set<
+    (reason: CallLoadErrorReason | undefined) => void
+  >();
 
   // Arrow-function class fields so dispose() passes the exact same reference to mx.off()
   private readonly boundOnEvent = (ev: MatrixEvent) => this.onEvent(ev);
@@ -375,17 +377,44 @@ export class CallEmbed {
     }
   }
 
+  private notifyLoadListeners(reason: CallLoadErrorReason | undefined): void {
+    this.loadErrorListeners.forEach((cb) => {
+      try {
+        cb(reason);
+      } catch {
+        // a misbehaving subscriber must not block the others
+      }
+    });
+  }
+
   /**
-   * Marks the load lifecycle as settled. Called on success (no reason) or on
+   * Marks the load lifecycle as settled.
-   * failure (reason set). Idempotent so the first signal wins.
+   *
+   * - Failure (reason set): the FIRST failure wins; a later success can still
+   *   heal it (below). Once we've genuinely succeeded, later spurious failures
+   *   are ignored.
+   * - Success (no reason): always clears the watchdog. Crucially, if we had
+   *   previously settled as a failure (e.g. the 25s watchdog fired on a slow
+   *   network but EC then finished loading), we self-heal: clear the error and
+   *   notify subscribers with `undefined` so the recovery UI dismisses itself
+   *   instead of stranding the user on an error screen over a live call.
    */
   private settleLoad(reason?: CallLoadErrorReason): void {
-    if (this.loadSettled) return;
-    this.loadSettled = true;
-    this.clearLoadWatchdog();
     if (reason) {
+      if (this.loadSettled) return;
+      this.loadSettled = true;
+      this.clearLoadWatchdog();
       this.loadError = reason;
-      this.loadErrorListeners.forEach((cb) => cb(reason));
+      this.notifyLoadListeners(reason);
+      return;
+    }
+
+    this.clearLoadWatchdog();
+    const wasFailed = this.loadError !== undefined;
+    this.loadSettled = true;
+    this.loadError = undefined;
+    if (wasFailed) {
+      this.notifyLoadListeners(undefined);
     }
   }
 
@@ -402,7 +431,7 @@ export class CallEmbed {
    * immediately so late subscribers still see the error.
    * @returns an unsubscribe function.
    */
-  public onLoadError(callback: (reason: CallLoadErrorReason) => void): () => void {
+  public onLoadError(callback: (reason: CallLoadErrorReason | undefined) => void): () => void {
     this.loadErrorListeners.add(callback);
     if (this.loadError) callback(this.loadError);
     return () => {

src/app/utils/matrix.ts

@@ -169,12 +169,17 @@ const matrixErrorFromUnknown = (e: unknown): MatrixError => {
 // HTTP statuses that should not be retried — client errors are deterministic
 // (e.g. 413 payload too large, 400 bad request, 401/403 auth) and won't succeed on retry.
 const isRetryableUploadError = (e: unknown): boolean => {
+  // A user-cancelled / aborted upload must never be retried. matrix-js-sdk's
+  // mx.cancelUpload() rejects the upload with a DOMException named "AbortError";
+  // without this guard the retry loop would resurrect an upload the user just
+  // cancelled.
+  if ((e as { name?: unknown } | null | undefined)?.name === 'AbortError') return false;
   if (e instanceof MatrixError) {
     const status = e.httpStatus;
     // No status => network/transport failure (transient): retry.
     if (typeof status !== 'number') return true;
-    // Retry on rate-limiting and server-side (5xx) errors only.
+    // Retry on request-timeout, rate-limiting and server-side (5xx) errors only.
-    return status === 429 || status >= 500;
+    return status === 408 || status === 429 || status >= 500;
   }
   // Non-Matrix errors are typically network/transport failures: retry.
   return true;
@@ -307,6 +312,8 @@ export const addRoomIdToMDirect = async (
   // (it can only be a DM room for one person)
   Object.keys(userIdToRoomIds).forEach((targetUserId) => {
     const roomIds = userIdToRoomIds[targetUserId];
+    // Guard against a corrupt m.direct where a value isn't an array.
+    if (!Array.isArray(roomIds)) return;
 
     if (targetUserId !== userId) {
       const indexOfRoomId = roomIds.indexOf(roomId);
@@ -316,7 +323,7 @@ export const addRoomIdToMDirect = async (
     }
   });
 
-  const roomIds = userIdToRoomIds[userId] || [];
+  const roomIds = Array.isArray(userIdToRoomIds[userId]) ? userIdToRoomIds[userId] : [];
   if (roomIds.indexOf(roomId) === -1) {
     roomIds.push(roomId);
   }
@@ -334,6 +341,7 @@ export const removeRoomIdFromMDirect = async (mx: MatrixClient, roomId: string):
 
   Object.keys(userIdToRoomIds).forEach((targetUserId) => {
     const roomIds = userIdToRoomIds[targetUserId];
+    if (!Array.isArray(roomIds)) return;
     const indexOfRoomId = roomIds.indexOf(roomId);
     if (indexOfRoomId > -1) {
       roomIds.splice(indexOfRoomId, 1);

src/app/utils/ringtones.ts

@@ -78,7 +78,7 @@ const PHRASES: Record<
   },
 };
 
-const playPhrase = (style: SynthStyle, volume: number): void => {
+const playPhrase = (style: SynthStyle, volume: number, destination: AudioNode): void => {
   const ctx = getCtx();
   if (!ctx) return;
   const { type, gain: peak, notes } = PHRASES[style];
@@ -96,7 +96,7 @@ const playPhrase = (style: SynthStyle, volume: number): void => {
     gain.gain.linearRampToValueAtTime(scaledPeak, start + 0.015);
     gain.gain.exponentialRampToValueAtTime(0.0001, start + dur);
     osc.connect(gain);
-    gain.connect(ctx.destination);
+    gain.connect(destination);
     osc.start(start);
     osc.stop(start + dur + 0.02);
   });
@@ -121,11 +121,41 @@ const startClassic = (volume: number, loop: boolean): (() => void) => {
 };
 
 const startSynth = (style: SynthStyle, volume: number, loop: boolean): (() => void) => {
-  playPhrase(style, volume);
+  const ctx = getCtx();
-  if (!loop) return () => undefined;
+  if (!ctx) return () => undefined;
-  const period = PHRASES[style].period * 1000;
+  // All notes route through a per-session master gain so stop() can silence
-  const id = window.setInterval(() => playPhrase(style, volume), period);
+  // everything instantly — including notes already scheduled slightly in the
-  return () => window.clearInterval(id);
+  // future — instead of letting the last phrase ring out after the user answers.
+  const master = ctx.createGain();
+  master.gain.value = 1;
+  master.connect(ctx.destination);
+
+  playPhrase(style, volume, master);
+  const id = loop
+    ? window.setInterval(() => playPhrase(style, volume, master), PHRASES[style].period * 1000)
+    : 0;
+
+  let stopped = false;
+  return () => {
+    if (stopped) return;
+    stopped = true;
+    if (id) window.clearInterval(id);
+    try {
+      const now = ctx.currentTime;
+      master.gain.cancelScheduledValues(now);
+      master.gain.setValueAtTime(master.gain.value, now);
+      master.gain.linearRampToValueAtTime(0, now + 0.03);
+    } catch {
+      /* context may be closed */
+    }
+    window.setTimeout(() => {
+      try {
+        master.disconnect();
+      } catch {
+        /* already disconnected */
+      }
+    }, 100);
+  };
 };
 
 /**