Agent: [Agent] WebRTC: fix ICE gathering/trickle and remove dead signaling code

web/js/components/preact/WebRTCVideoCell.jsx

@@ -23,6 +23,11 @@ import { createPlayerTelemetry } from '../../utils/player-telemetry.js';
 import { useAutoRetry } from './useAutoRetry.js';
 import { useVideoZoom } from './useVideoZoom.js';
 import { streamConnectionGate, priorityForStreamStatus, isGateTimeout, isGateAbort } from '../../utils/stream-connection-gate.js';
+import {
+  DEFAULT_ICE_GATHERING_TIMEOUT_MS,
+  ICE_GATHER_OUTCOME,
+  waitForIceGathering,
+} from '../../utils/webrtc-ice-gathering.js';
 import { shouldFallbackFullscreenToSubStream } from './liveStreamPolicy.js';
 import { LiveTileStatus } from './LiveTileStatus.jsx';
 import { PictureInPictureButton } from './PictureInPictureButton.jsx';
@@ -716,32 +721,23 @@ export function WebRTCVideoCell({
 
     pc.oniceconnectionstatechange = handleIceConnectionStateChange;
 
-    // Handle ICE gathering state changes
+    // Handle ICE gathering state changes (debug level: this fires once per
+    // state transition per cell, which is noisy on multi-camera grids).
     pc.onicegatheringstatechange = () => {
-      console.log(`ICE gathering state for stream ${stream.name}: ${pc.iceGatheringState}`);
+      console.debug(`ICE gathering state for stream ${stream.name}: ${pc.iceGatheringState}`);
     };
 
-    // Handle ICE candidates - critical for NAT traversal
+    // Log ICE candidates at debug level. go2rtc's HTTP WebRTC endpoint
+    // (/api/webrtc) has no trickle-ICE channel — there is no server-side
+    // route that accepts candidates, so candidates travel to go2rtc inside
+    // the SDP offer itself (see the bounded gathering wait below). Trickle
+    // would require go2rtc's WebSocket API (/api/ws), which the HTTP-only
+    // /go2rtc/ reverse proxy does not forward.
     pc.onicecandidate = (event) => {
       if (event.candidate) {
-        console.log(`ICE candidate for stream ${stream.name}:`, event.candidate.candidate);
-
-        // Send the ICE candidate to the server
-        // Note: go2rtc typically handles ICE candidates in the SDP exchange,
-        // but we log them here for debugging purposes
-        // If trickle ICE is needed, uncomment the code below:
-        /*
-        fetch(`/api/webrtc/ice?src=${encodeURIComponent(stream.name)}`, {
-          method: 'POST',
-          headers: {
-            'Content-Type': 'application/json',
-            ...(auth ? { 'Authorization': 'Basic ' + auth } : {})
-          },
-          body: JSON.stringify(event.candidate)
-        }).catch(err => console.warn('Failed to send ICE candidate:', err));
-        */
+        console.debug(`ICE candidate for stream ${stream.name}:`, event.candidate.candidate);
       } else {
-        console.log(`ICE gathering complete for stream ${stream.name}`);
+        console.debug(`ICE gathering complete for stream ${stream.name}`);
       }
     };
 
@@ -790,9 +786,27 @@ export function WebRTCVideoCell({
         return pc.setLocalDescription(offer);
       })
       .then(() => {
-        console.log(`Set local description for stream ${stream.name}, waiting for ICE gathering...`);
+        // ICE gathering runs in the background once the local description is
+        // set. go2rtc's HTTP endpoint has no trickle channel, so any candidate
+        // that is going to reach it must be baked into this offer. Wait for
+        // gathering to complete, bounded so an unreachable STUN/TURN server
+        // can only stall the connect by the timeout, not by the browser's
+        // multi-second gathering timeout.
+        console.log(
+          `Set local description for stream ${stream.name}, waiting for ICE gathering ` +
+          `(up to ${DEFAULT_ICE_GATHERING_TIMEOUT_MS}ms)...`
+        );
+        return waitForIceGathering(pc, DEFAULT_ICE_GATHERING_TIMEOUT_MS, cellAbortSignal);
+      })
+      .then((gatherOutcome) => {
+        if (gatherOutcome === ICE_GATHER_OUTCOME.TIMEOUT) {
+          console.warn(
+            `ICE gathering timed out after ${DEFAULT_ICE_GATHERING_TIMEOUT_MS}ms for stream ` +
+            `${stream.name}, sending offer with partial candidates`
+          );
+        }
 
-        console.log(`Sending offer directly to go2rtc for stream ${stream.name}`);
+        console.log(`Sending offer directly to go2rtc for stream ${stream.name} (ICE gathering: ${gatherOutcome})`);
 
         // Send the offer to go2rtc through the shared connection gate. The
         // gate bounds concurrent offers page-wide and enforces a per-attempt

web/js/utils/webrtc-ice-gathering.js

@@ -0,0 +1,78 @@
+/**
+ * Bounded ICE gathering helper for the go2rtc HTTP signaling flow.
+ *
+ * go2rtc's HTTP endpoint (`POST /api/webrtc?src=...`) has no trickle-ICE
+ * channel: the SDP offer is the only place client candidates can travel.
+ * Waiting indefinitely for `icegatheringstate === 'complete'` lets an
+ * unreachable STUN/TURN server stall the connect for several seconds, so the
+ * wait is bounded — the offer goes out with whatever candidates exist when
+ * gathering completes or the timeout expires, whichever comes first.
+ */
+
+export const DEFAULT_ICE_GATHERING_TIMEOUT_MS = 1000;
+
+export const ICE_GATHER_OUTCOME = {
+  COMPLETE: 'complete',
+  TIMEOUT: 'timeout',
+  ABORTED: 'aborted',
+};
+
+/**
+ * Wait for a peer connection's ICE gathering to complete, bounded by a
+ * timeout.
+ *
+ * @param {RTCPeerConnection} pc - The peer connection to watch.
+ * @param {number} [timeoutMs] - Maximum time to wait before resolving.
+ * @param {AbortSignal} [abortSignal] - Optional signal; aborting ends the
+ *   wait early (e.g. the cell unmounted or a retry superseded this attempt).
+ * @returns {Promise<string>} Resolves with one of ICE_GATHER_OUTCOME values.
+ */
+export function waitForIceGathering(
+  pc,
+  timeoutMs = DEFAULT_ICE_GATHERING_TIMEOUT_MS,
+  abortSignal = null
+) {
+  return new Promise((resolve) => {
+    if (!pc || pc.iceGatheringState === 'complete') {
+      resolve(ICE_GATHER_OUTCOME.COMPLETE);
+      return;
+    }
+    if (timeoutMs <= 0) {
+      resolve(ICE_GATHER_OUTCOME.TIMEOUT);
+      return;
+    }
+    if (abortSignal && abortSignal.aborted) {
+      resolve(ICE_GATHER_OUTCOME.ABORTED);
+      return;
+    }
+
+    let settled = false;
+    const finish = (outcome) => {
+      if (settled) return;
+      settled = true;
+      clearTimeout(timer);
+      pc.removeEventListener('icegatheringstatechange', onStateChange);
+      if (abortSignal) {
+        abortSignal.removeEventListener('abort', onAbort);
+      }
+      resolve(outcome);
+    };
+    const onStateChange = () => {
+      if (pc.iceGatheringState === 'complete') {
+        finish(ICE_GATHER_OUTCOME.COMPLETE);
+      }
+    };
+    const onAbort = () => finish(ICE_GATHER_OUTCOME.ABORTED);
+
+    const timer = setTimeout(() => finish(ICE_GATHER_OUTCOME.TIMEOUT), timeoutMs);
+    // Under Node (unit tests) setTimeout returns a Timeout object; unref it so
+    // a pending gather bound never keeps the test process alive.
+    if (typeof timer === 'object' && typeof timer.unref === 'function') {
+      timer.unref();
+    }
+    pc.addEventListener('icegatheringstatechange', onStateChange);
+    if (abortSignal) {
+      abortSignal.addEventListener('abort', onAbort);
+    }
+  });
+}

web/tests/webrtcIceGathering.spec.js

@@ -0,0 +1,111 @@
+import {
+  DEFAULT_ICE_GATHERING_TIMEOUT_MS,
+  ICE_GATHER_OUTCOME,
+  waitForIceGathering,
+} from '../js/utils/webrtc-ice-gathering.js';
+
+function fakePeerConnection(initialState = 'gathering') {
+  const listeners = {};
+  return {
+    iceGatheringState: initialState,
+    listenerCount(type) {
+      return (listeners[type] || []).length;
+    },
+    addEventListener(type, fn) {
+      listeners[type] = listeners[type] || [];
+      listeners[type].push(fn);
+    },
+    removeEventListener(type, fn) {
+      if (!listeners[type]) return;
+      listeners[type] = listeners[type].filter((l) => l !== fn);
+    },
+    setGatheringState(state) {
+      this.iceGatheringState = state;
+      (listeners.icegatheringstatechange || []).slice().forEach((fn) => fn());
+    },
+  };
+}
+
+describe('waitForIceGathering', () => {
+  test('exposes a 1s default bound', () => {
+    expect(DEFAULT_ICE_GATHERING_TIMEOUT_MS).toBe(1000);
+  });
+
+  test('resolves immediately when gathering is already complete', async () => {
+    const pc = fakePeerConnection('complete');
+    await expect(waitForIceGathering(pc, 50)).resolves.toBe(ICE_GATHER_OUTCOME.COMPLETE);
+    expect(pc.listenerCount('icegatheringstatechange')).toBe(0);
+  });
+
+  test('resolves immediately for a missing peer connection', async () => {
+    await expect(waitForIceGathering(null, 50)).resolves.toBe(ICE_GATHER_OUTCOME.COMPLETE);
+  });
+
+  test('resolves complete when gathering finishes before the timeout', async () => {
+    const pc = fakePeerConnection('gathering');
+    const promise = waitForIceGathering(pc, 1000);
+    expect(pc.listenerCount('icegatheringstatechange')).toBe(1);
+    pc.setGatheringState('complete');
+    await expect(promise).resolves.toBe(ICE_GATHER_OUTCOME.COMPLETE);
+    expect(pc.listenerCount('icegatheringstatechange')).toBe(0);
+  });
+
+  test('resolves timeout when gathering stalls past the bound', async () => {
+    const pc = fakePeerConnection('gathering');
+    await expect(waitForIceGathering(pc, 25)).resolves.toBe(ICE_GATHER_OUTCOME.TIMEOUT);
+    expect(pc.listenerCount('icegatheringstatechange')).toBe(0);
+  });
+
+  test('resolves timeout immediately for a non-positive bound', async () => {
+    const pc = fakePeerConnection('gathering');
+    await expect(waitForIceGathering(pc, 0)).resolves.toBe(ICE_GATHER_OUTCOME.TIMEOUT);
+    expect(pc.listenerCount('icegatheringstatechange')).toBe(0);
+  });
+
+  test('ignores intermediate states that are not complete', async () => {
+    const pc = fakePeerConnection('new');
+    const promise = waitForIceGathering(pc, 250);
+    let settled = false;
+    promise.then(() => {
+      settled = true;
+    });
+    pc.setGatheringState('gathering');
+    await new Promise((r) => setTimeout(r, 25));
+    expect(settled).toBe(false);
+    pc.setGatheringState('complete');
+    await expect(promise).resolves.toBe(ICE_GATHER_OUTCOME.COMPLETE);
+  });
+
+  test('resolves aborted when the abort signal fires', async () => {
+    const pc = fakePeerConnection('gathering');
+    const controller = new AbortController();
+    const promise = waitForIceGathering(pc, 1000, controller.signal);
+    controller.abort();
+    await expect(promise).resolves.toBe(ICE_GATHER_OUTCOME.ABORTED);
+    expect(pc.listenerCount('icegatheringstatechange')).toBe(0);
+  });
+
+  test('resolves aborted immediately when the signal is already aborted', async () => {
+    const pc = fakePeerConnection('gathering');
+    const controller = new AbortController();
+    controller.abort();
+    await expect(waitForIceGathering(pc, 1000, controller.signal))
+      .resolves.toBe(ICE_GATHER_OUTCOME.ABORTED);
+    expect(pc.listenerCount('icegatheringstatechange')).toBe(0);
+  });
+
+  test('resolves only once when events race the timeout', async () => {
+    const pc = fakePeerConnection('gathering');
+    const outcomes = [];
+    const promise = waitForIceGathering(pc, 25).then((outcome) => {
+      outcomes.push(outcome);
+      return outcome;
+    });
+    pc.setGatheringState('complete');
+    await expect(promise).resolves.toBe(ICE_GATHER_OUTCOME.COMPLETE);
+    // Fire more transitions after settling; must not throw or re-resolve.
+    pc.setGatheringState('complete');
+    await new Promise((r) => setTimeout(r, 50));
+    expect(outcomes).toEqual([ICE_GATHER_OUTCOME.COMPLETE]);
+  });
+});