diff --git a/apps/desktop/src/renderer/components/capture/CapturePreview.tsx b/apps/desktop/src/renderer/components/capture/CapturePreview.tsx index ca600f2..128cf59 100644 --- a/apps/desktop/src/renderer/components/capture/CapturePreview.tsx +++ b/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(null); + const [grantedViewerId, setGrantedViewerId] = useState(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 && (
!p.left_at)} + participants={controlAdjustedParticipants.filter((p) => !p.left_at)} isHost={canModerateSession} mutedParticipants={mutedParticipants} onGrantControl={ diff --git a/apps/desktop/src/renderer/hooks/useWebRTCHostSFUAPI.test.ts b/apps/desktop/src/renderer/hooks/useWebRTCHostSFUAPI.test.ts index bb18dad..945e3f0 100644 --- a/apps/desktop/src/renderer/hooks/useWebRTCHostSFUAPI.test.ts +++ b/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'); + }); + }); }); diff --git a/apps/desktop/src/renderer/hooks/useWebRTCHostSFUAPI.ts b/apps/desktop/src/renderer/hooks/useWebRTCHostSFUAPI.ts index ca0d621..daf3775 100644 --- a/apps/desktop/src/renderer/hooks/useWebRTCHostSFUAPI.ts +++ b/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,7 +557,20 @@ 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