fix(desktop): camera never reached viewers, and control state showed a stale poll

apps/desktop/src/renderer/components/capture/CapturePreview.tsx

@@ -50,6 +50,7 @@ import { useLiveStreamEnabled } from '@/lib/liveStream';
 import { playJoinSound, playLeaveSound, useSessionSoundsEnabled } from '@/lib/sessionSounds';
 import { isPreferTailnetEnabled } from '@/lib/iceConfig';
 import { getDefaultAllowGuestControl, getDefaultSessionMode } from '@/lib/sessionDefaults';
+import { applyHostControlIntent } from '@/lib/controlIntent';
 import { useWebRTCHostAPI } from '@/hooks/useWebRTCHostAPI';
 import { useWebRTCHostSFUAPI } from '@/hooks/useWebRTCHostSFUAPI';
 import { useAutoStopServerStream } from '@/hooks/useAutoStopServerStream';
@@ -280,16 +281,6 @@ export function CapturePreview({
   // The remote participant the host has handed control to, which is what
   // enables input injection.
   //
-  // The host's own row is created with control_state 'granted' (they always
-  // control their own machine), so it must be excluded — otherwise injection
-  // switches on the moment a session is created, before anyone has joined.
-  const participantWithControl = useMemo(() => {
-    return (
-      participants.find((p) => p.control_state === 'granted' && !p.left_at && p.role !== 'host') ??
-      null
-    );
-  }, [participants]);
-
   // Viewers waiting on a control decision. Guests are anonymous and cannot
   // write control_state themselves, so requests arrive over the data channel
   // and live here until the host approves or denies them.
@@ -301,7 +292,7 @@ export function CapturePreview({
   // already in flight when control is granted lands afterwards carrying the
   // pre-grant state — which silently switched injection back off a few seconds
   // into a session. Host intent is authoritative; the poll only corroborates.
-  const [, setGrantedViewerId] = useState<string | null>(null);
+  const [grantedViewerId, setGrantedViewerId] = useState<string | null>(null);
   // This ref is the only authorization source for OS injection. The database
   // poll is intentionally not used here: it is eventually consistent and can
   // contain a stale grant from a previous connection.
@@ -314,6 +305,38 @@ export function CapturePreview({
     setGrantedViewerId(viewerId);
   }, []);
 
+  /**
+   * The participant list with `control_state` corrected to what the host
+   * actually decided.
+   *
+   * Everything the host sees about control — the "has control" banner, the
+   * per-participant badge, and whether the row offers Grant or Revoke — used to
+   * render straight from `participants`, which is a 5-second poll of the
+   * database. That poll is eventually consistent, so a response already in
+   * flight when the host revokes lands afterwards still carrying `granted`, and
+   * the UI re-asserts that the guest is in control seconds after input
+   * injection has already been switched off. Reported as "when I took control
+   * back from my pair, PairUX said my pair still had control".
+   *
+   * Host intent is authoritative here for exactly the same reason it is
+   * authoritative for injection: it is the decision, not an echo of it. The
+   * poll still supplies everything else about each participant.
+   */
+  const controlAdjustedParticipants = useMemo(
+    () => applyHostControlIntent(participants, grantedViewerId),
+    [participants, grantedViewerId]
+  );
+
+  // The remote participant the host has handed control to, which is what
+  // drives the on-screen "has control" banner.
+  const participantWithControl = useMemo(() => {
+    return (
+      controlAdjustedParticipants.find(
+        (p) => p.control_state === 'granted' && !p.left_at && p.role !== 'host'
+      ) ?? null
+    );
+  }, [controlAdjustedParticipants]);
+
   // Read through refs so cursor updates (up to 60/s) never re-create the host
   // hook options and tear down the connection.
   const sourceDimensionsRef = useRef({ width: 1920, height: 1080 });
@@ -981,12 +1004,25 @@ export function CapturePreview({
       if (!session) return;
       try {
         const viewerId = resolveViewerTargetId(participantId);
+        // Match on *either* identifier the grant could have been stored under.
+        // resolveViewerTargetId returns a single id and prefers whichever one
+        // is currently in hostedViewers, so a viewer that reconnected between
+        // the grant and the revoke resolves to the other candidate — and
+        // comparing only that one left OS injection enabled on a revoke that
+        // otherwise looked successful.
+        const participant = participants.find((p) => p.id === participantId);
+        const candidates = [participant?.user_id, participant?.id, viewerId].filter(
+          (value): value is string => typeof value === 'string' && value.length > 0
+        );
+        const holdsControl =
+          grantedViewerIdRef.current !== null && candidates.includes(grantedViewerIdRef.current);
+
         // Local safety wins over network/database round trips. Stop OS input
         // first, then notify the viewer and persist the revocation.
-        if (viewerId && grantedViewerIdRef.current === viewerId) {
+        if (holdsControl) {
           setGrantedController(null);
           await deactivateInput();
-          revokeControl(viewerId);
+          revokeControl(viewerId ?? candidates[0]);
         }
         const response = await fetch(
           `${API_BASE_URL}/api/sessions/${session.id}/participants/${participantId}/control`,
@@ -1015,6 +1051,7 @@ export function CapturePreview({
     },
     [
       session,
+      participants,
       getAuthHeaders,
       resolveViewerTargetId,
       deactivateInput,
@@ -1790,7 +1827,7 @@ export function CapturePreview({
       {session && showParticipants && (
         <div className="w-72 shrink-0 border-l border-border bg-background p-4">
           <ParticipantList
-            participants={participants}
+            participants={controlAdjustedParticipants}
             currentUserId={currentUserId}
             sessionId={session.id}
             isHost={canModerateSession}
@@ -1809,7 +1846,7 @@ export function CapturePreview({
         <ChatPanel
           sessionId={session.id}
           currentUserId={currentUserId}
-          participants={participants.filter((p) => !p.left_at)}
+          participants={controlAdjustedParticipants.filter((p) => !p.left_at)}
           isHost={canModerateSession}
           mutedParticipants={mutedParticipants}
           onGrantControl={

apps/desktop/src/renderer/hooks/useWebRTCHostSFUAPI.test.ts

@@ -590,4 +590,108 @@ describe('useWebRTCHostSFUAPI', () => {
     expect(mockConnect).toHaveBeenCalledTimes(2);
     expect(result.current.isHosting).toBe(true);
   });
+
+  /**
+   * Regression: turning the camera bubble on swaps the raw screen track for a
+   * composited one and republishes. Publishing again left *both* tracks up, and
+   * the viewer — which folds every subscribed video track into one MediaStream
+   * and renders only the first — kept showing the pre-camera screen. The camera
+   * never reached anyone. Reported from a real session as "I was able to see
+   * myself in the preview window, but my pair was unable to see me".
+   */
+  describe('republishing the presentation track', () => {
+    const mockReplaceTrack = vi.fn().mockResolvedValue(undefined);
+
+    /** Mirror LiveKit: a successful publish shows up in trackPublications. */
+    function trackPublishingRoom() {
+      mockReplaceTrack.mockClear();
+      mockPublishTrack.mockImplementation((track: { id: string }, options: { source: string }) => {
+        const publication = {
+          source: options.source,
+          track: { mediaStreamTrack: track, replaceTrack: mockReplaceTrack },
+        };
+        mockTrackPublications.set(options.source, publication);
+        return Promise.resolve(publication);
+      });
+    }
+
+    async function hostWithScreenTrack() {
+      trackPublishingRoom();
+      const { result } = renderHook(() =>
+        useWebRTCHostSFUAPI({ sessionId: 'session-1', hostId: 'host-1', localStream: null })
+      );
+
+      await act(async () => {
+        await result.current.startHosting();
+        await Promise.resolve();
+      });
+
+      const screenTrack = { kind: 'video', id: 'screen-track' };
+      await act(async () => {
+        await result.current.publishStream(
+          new MockMediaStream([screenTrack]) as unknown as MediaStream
+        );
+      });
+
+      return { result, screenTrack };
+    }
+
+    it('replaces the published video instead of publishing a second track', async () => {
+      const { result } = await hostWithScreenTrack();
+
+      const videoPublishes = () =>
+        mockPublishTrack.mock.calls.filter(
+          (call) => (call[1] as { source: string }).source === 'screen_share'
+        );
+
+      expect(videoPublishes()).toHaveLength(1);
+
+      // Camera on: the composited canvas track replaces the screen track.
+      const compositeTrack = { kind: 'video', id: 'composite-track' };
+      await act(async () => {
+        await result.current.publishStream(
+          new MockMediaStream([compositeTrack]) as unknown as MediaStream
+        );
+      });
+
+      expect(videoPublishes()).toHaveLength(1);
+      expect(mockReplaceTrack).toHaveBeenCalledTimes(1);
+      expect(mockReplaceTrack).toHaveBeenCalledWith(compositeTrack);
+    });
+
+    it('does not replace when handed the track it is already publishing', async () => {
+      const { result, screenTrack } = await hostWithScreenTrack();
+
+      // A re-render republishing the identical track must be a no-op, not a
+      // needless renegotiation on every state change in the capture view.
+      await act(async () => {
+        await result.current.publishStream(
+          new MockMediaStream([screenTrack]) as unknown as MediaStream
+        );
+      });
+
+      expect(mockReplaceTrack).not.toHaveBeenCalled();
+      expect(
+        mockPublishTrack.mock.calls.filter(
+          (call) => (call[1] as { source: string }).source === 'screen_share'
+        )
+      ).toHaveLength(1);
+    });
+
+    it('marks the replacement track as detail content for the encoder', async () => {
+      const { result } = await hostWithScreenTrack();
+
+      const compositeTrack: { kind: string; id: string; contentHint?: string } = {
+        kind: 'video',
+        id: 'composite-track',
+      };
+      await act(async () => {
+        await result.current.publishStream(
+          new MockMediaStream([compositeTrack]) as unknown as MediaStream
+        );
+      });
+
+      expect(compositeTrack.contentHint).toBe('detail');
+    });
+  });
 });

apps/desktop/src/renderer/hooks/useWebRTCHostSFUAPI.ts

@@ -56,6 +56,11 @@ export interface ViewerConnection {
 interface UseWebRTCHostSFUAPIOptions {
   sessionId: string;
   hostId: string;
+  /**
+   * The raw capture stream. Retained so callers keep a single options shape
+   * across the P2P and SFU hooks; publishing is driven entirely by
+   * `publishStream()`, which is handed the *presentation* track instead.
+   */
   localStream: MediaStream | null;
   allowControl?: boolean;
   onViewerJoined?: (viewerId: string) => void;
@@ -94,7 +99,6 @@ interface UseWebRTCHostSFUAPIReturn {
 export function useWebRTCHostSFUAPI({
   sessionId,
   hostId,
-  localStream,
   allowControl = false,
   onViewerJoined,
   onViewerLeft,
@@ -553,18 +557,55 @@ export function useWebRTCHostSFUAPI({
     }
   }, [sessionId, hostId, addViewer, attachViewerAudio, removeViewer, handleDataReceived]);
 
-  // Publish a screen share stream to the LiveKit room
+  /**
+   * Publish the presentation stream, replacing whatever is already published.
+   *
+   * Republishing is not the same as publishing. Turning the camera bubble on
+   * swaps the raw screen track for a composited one and calls this again —
+   * and `publishTrack` would then leave *both* tracks published. Viewers fold
+   * every subscribed video track into one MediaStream and a <video> element
+   * renders only the first, so the second publication is invisible: the camera
+   * never arrives and the viewer watches the pre-camera screen forever.
+   *
+   * Replacing the track inside the existing publication keeps the same
+   * publication SID, so viewers switch over without resubscribing. The P2P
+   * host does exactly this, for exactly this reason — see useWebRTCHostAPI.
+   */
   const publishStream = useCallback(async (stream: MediaStream) => {
     const room = roomRef.current;
     if (room?.state !== LKConnectionState.Connected) {
       console.warn('[WebRTCHostSFUAPI] Cannot publish stream: room not connected');
       return;
     }
 
+    const publicationFor = (source: Track.Source): LocalTrackPublication | undefined =>
+      Array.from(room.localParticipant.trackPublications.values()).find(
+        (pub: LocalTrackPublication) => pub.source === source
+      );
+
     for (const track of stream.getTracks()) {
       try {
+        const source =
+          track.kind === 'video' ? Track.Source.ScreenShare : Track.Source.ScreenShareAudio;
+
         if (track.kind === 'video') {
           track.contentHint = 'detail';
+        }
+
+        const existing = publicationFor(source);
+        if (existing?.track) {
+          // Already publishing this exact track — a re-render, not a new source.
+          if (existing.track.mediaStreamTrack.id === track.id) continue;
+
+          await existing.track.replaceTrack(track);
+          console.log('[WebRTCHostSFUAPI] Replaced published track', {
+            source,
+            trackId: track.id,
+          });
+          continue;
+        }
+
+        if (track.kind === 'video') {
           await room.localParticipant.publishTrack(track, {
             source: Track.Source.ScreenShare,
             simulcast: false,
@@ -573,7 +614,7 @@ export function useWebRTCHostSFUAPI({
               maxFramerate: 60,
             },
           });
-        } else if (track.kind === 'audio') {
+        } else {
           await room.localParticipant.publishTrack(track, {
             source: Track.Source.ScreenShareAudio,
           });
@@ -651,22 +692,15 @@ export function useWebRTCHostSFUAPI({
     };
   }, [stopHosting]);
 
-  // Update published tracks when stream changes
-  useEffect(() => {
-    if (!localStream || !isHosting) return;
-
-    const room = roomRef.current;
-    if (room?.state !== LKConnectionState.Connected) return;
-
-    const videoTrack = localStream.getVideoTracks()[0];
-    const existingPub = Array.from(room.localParticipant.trackPublications.values()).find(
-      (pub: LocalTrackPublication) => pub.source === Track.Source.ScreenShare
-    );
-
-    if (existingPub?.track) {
-      void existingPub.track.replaceTrack(videoTrack);
-    }
-  }, [localStream, isHosting]);
+  // Republishing is handled entirely by publishStream(), which replaces the
+  // track inside the existing publication.
+  //
+  // There used to be an effect here that swapped the published video for
+  // `localStream`'s track whenever that stream or `isHosting` changed. It was
+  // wrong once the camera bubble existed: `localStream` is the *raw* screen
+  // capture, not the composited presentation track, so re-establishing a host
+  // connection with the camera on silently replaced the composite with the
+  // bare screen and dropped the bubble for every viewer.
 
   /** Tell a peer our tailnet addresses so it can test a direct path. */
   const sendTailnetHello = useCallback(