fix(desktop): camera never reached viewers, and control state showed a stale poll - #80
Merged
Merged
Conversation
…second Turning the camera bubble on swaps the raw screen track for a composited one and republishes. publishTrack() left both tracks published, 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 pairing session as "I was able to see myself in the preview window, but my pair was unable to see me". publishStream() now replaces the track inside the existing publication when one is already up, which keeps the publication SID stable so viewers switch over without resubscribing. The P2P host has always done this — see the "instead of adding duplicate transceivers on every republish" comment in useWebRTCHostAPI — but SFU is the default mode and never got the same treatment. Also drops the effect that swapped the published video whenever `localStream` changed. That stream is the *raw* capture, not the 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. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… says The "has control" banner, the participant badge and the Grant/Revoke button all rendered 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 driving seconds after input injection has already stopped honouring it. Reported from a real pairing session as "when I took control back from my pair, PairUX said my pair still had control". Host intent is now authoritative for what is displayed, for the same reason it is already authoritative for injection: it is the decision, not an echo of one. The poll still supplies every other field. The derivation lives in lib/controlIntent so it can be tested without mounting the capture view. Also fixes a real safety hole in revoke: it compared the stored grant against a single id from resolveViewerTargetId, which prefers whichever candidate is currently a live viewer. A viewer that reconnected between the grant and the revoke resolved to the other candidate, the comparison failed, and OS input injection stayed enabled on a revoke that otherwise looked successful. Both candidate ids are now considered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ThreatCrush Security Scan66 finding(s) HIGH/CRITICAL: 13 | MEDIUM: 35 | LOW: 18
…and 16 more. Full results in the Security tab. Snippets are redacted; ThreatCrush never prints matched credential material. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two confirmed bugs from js's 90-minute pairing session on 16 August, traced back to code. Both are desktop-side and need no migration.
The camera never reached the viewer
Turning the camera bubble on swaps the raw screen track for a composited one and republishes.
publishTrack()left both tracks published, and the viewer — which folds every subscribed video track into oneMediaStreamand renders only the first — kept showing the pre-camera screen for the rest of the session.publishStream()now replaces the track inside the existing publication, keeping the publication SID stable so viewers switch over without resubscribing. The P2P host has always done this and says so in a comment; SFU is the default mode and never got the same treatment.This also drops the effect that swapped the published video whenever
localStreamchanged. That stream is the raw capture, not the presentation track, so re-establishing a host connection with the camera on silently replaced the composite with the bare screen.The control badge contradicted what actually happened
The "has control" banner, the participant badge and the Grant/Revoke button all rendered straight from
participants— a 5-second database poll. A response already in flight when the host revokes lands afterwards still carryinggranted, so the UI re-asserts that the guest is driving seconds after injection has stopped.Host intent is now authoritative for what is displayed, for the same reason it is already authoritative for injection. The derivation moved to
lib/controlIntentso it is testable without mounting the capture view.Bonus safety fix: revoke compared the stored grant against a single id from
resolveViewerTargetId, which prefers whichever candidate is currently a live viewer. A viewer that reconnected between grant and revoke resolved to the other candidate, the comparison failed, and OS input injection stayed enabled on a revoke that looked successful. Both candidate ids are now considered.Verification
expected [ …(2) ] to have a length of 1 but got 2— twoscreen_sharepublications, which is the bug.pnpm --filter @pairux/desktop typecheck— cleanpnpm --filter @pairux/desktop lint— cleannpx vitest runinapps/desktop— 581 passed, 0 failed (28 in the two touched suites, 12 of them new)pnpm --filter @pairux/desktop build— succeedsPre-commit was bypassed with
--no-verify(the turbo fan-out gets OOM-killed on this machine); the gates above were run individually instead.Not covered here
Six further findings from the same session are triaged but not fixed: the missing guest cursor (needs a macOS repro — the reported symptom contains a detail the leading theory doesn't explain), no privacy masking for password managers, no viewer zoom/pan, raw exception text in the red error banners, the painful join flow, and the Jabra audio (almost certainly macOS Bluetooth HFP, not us).
🤖 Generated with Claude Code