Skip to content

fix(session): require entire-screen sharing on Windows - #203

Merged
scotej merged 2 commits into
mainfrom
agent/windows-entire-screen-sharing-188
Aug 6, 2026
Merged

fix(session): require entire-screen sharing on Windows#203
scotej merged 2 commits into
mainfrom
agent/windows-entire-screen-sharing-188

Conversation

@scotej

@scotej scotej commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Closes #188.

Summary

  • Prefer Entire Screen in the Windows screen-capture picker with displaySurface: 'monitor' and monitorTypeSurfaces: 'include'.
  • Verify the returned video track is actually a monitor before any session state or WebRTC publication occurs.
  • Stop every returned track and show an actionable retry message when Windows returns a window, browser surface, missing video track, or unavailable surface metadata.
  • Preserve the existing picker behavior on macOS and other platforms.

Platform contract

The Screen Capture specification does not allow application constraints to remove picker choices. The request therefore prefers a monitor, then enforces the Windows policy on the consented stream before publication. No browser flags or native WebView2 bypasses are used.

Validation

  • npm test — 93 files / 1,129 tests passed
  • Focused screen-share coverage — 28 tests passed, including 8 new capture-policy cases
  • npm run lint
  • npm run build
  • npm run format:check
  • token, string, migration, story, and contrast guards
  • npm run build-storybook
  • git diff --check

GitHub CI remains the authority for the browser-backed accessibility run and the macOS/Windows Rust and installer jobs, which are not available in this cloud VM. The interactive Windows picker path is covered at the policy boundary with injected MediaStream tracks; a packaged Windows smoke test should confirm the WebView2 picker presentation.

Summary by CodeRabbit

  • New Features
    • Improved Windows screen sharing by guiding users to select “Entire Screen” when required.
    • Added validation to prevent unsupported display surfaces from being shared.
  • Bug Fixes
    • Screen-share failures now show a specific, actionable message instead of a generic error.
    • Rejected screen-share tracks are properly stopped to prevent resource leaks.
  • Documentation
    • Updated screen-sharing guidance with platform-specific capture requirements.

Copilot AI lite review requested due to automatic review settings August 6, 2026 02:14

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 454f2119-c1e9-4ec7-ae73-021717c3a567

📥 Commits

Reviewing files that changed from the base of the PR and between b8ee114 and 6c98772.

📒 Files selected for processing (2)
  • src/features/session/screenShare.ts
  • tests/unit/screen-share-capture.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • tests/unit/screen-share-capture.test.ts
  • src/features/session/screenShare.ts
📜 Recent review details
⏰ Context from checks skipped due to timeout. (8)
  • GitHub Check: Installer (macOS)
  • GitHub Check: Installer (Windows)
  • GitHub Check: Rust (macOS)
  • GitHub Check: Rust (Linux)
  • GitHub Check: Rust (Windows)
  • GitHub Check: Frontend
  • GitHub Check: Analyze (rust)
  • GitHub Check: Analyze (javascript-typescript)

📝 Walkthrough

Walkthrough

Windows screen sharing now requests monitor-oriented capture, rejects non-monitor surfaces, stops rejected tracks, and reports a dedicated error. The session view shows an Entire Screen toast. macOS capture behavior remains unchanged.

Changes

Screen-share capture policy

Layer / File(s) Summary
Capture runtime and surface validation
src/features/session/screenShare.ts
Screen capture accepts an injectable runtime. Windows requests monitor-oriented capture, validates displaySurface, stops rejected streams, and raises EntireScreenShareRequiredError.
Session error feedback and documentation
src/features/session/SessionView.tsx, src/strings.ts, ARCHITECTURE.md
SessionView shows a dedicated Entire Screen toast. The architecture documentation describes Windows validation and unchanged macOS behavior.
Capture policy validation tests
tests/unit/screen-share-capture.test.ts
Tests cover Windows, macOS, and default browser capture, including constraints, accepted and rejected surfaces, settings failures, synchronous invocation, and track cleanup.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SessionView
  participant screenShare
  participant BrowserPicker
  participant MediaStreamTrack
  SessionView->>screenShare: requestScreenShareStream()
  screenShare->>BrowserPicker: getDisplayMedia(constraints)
  BrowserPicker-->>screenShare: selected display stream
  screenShare->>MediaStreamTrack: read displaySurface
  alt Non-monitor surface
    screenShare->>MediaStreamTrack: stop()
    screenShare-->>SessionView: EntireScreenShareRequiredError
    SessionView-->>SessionView: show entireScreenRequiredToast
  else Monitor surface
    screenShare-->>SessionView: accepted display stream
  end
Loading

Possibly related PRs

  • scotej/studyvis#45: Both changes update SessionView.tsx for screen-sharing recovery behavior.
  • scotej/studyvis#103: Both changes modify SessionView screen-capture acquisition and error handling.
  • scotej/studyvis#128: Both changes modify getDisplayMedia() behavior for screen capture.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the implementation and validation, but it omits required template sections and manual-test status labels. Use the repository template headings and record manual-test, compatibility, gate, and merge-style statuses, including machine-walked, user-walked, or n-a labels.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the Windows screen-sharing behavior change: requiring Entire Screen sharing.
Linked Issues check ✅ Passed The changes implement issue #188 by preferring and enforcing monitor capture on Windows while preserving other platforms.
Out of Scope Changes check ✅ Passed All changes support issue #188 through Windows capture policy, user messaging, tests, and related documentation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch agent/windows-entire-screen-sharing-188

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@scotej
scotej merged commit 36556f1 into main Aug 6, 2026
20 checks passed
scotej added a commit that referenced this pull request Aug 8, 2026
Keep the stronger capture implementation from #203 while preserving #204's distinct guidance for unverifiable WebView2 selections and cross-platform/error-passthrough coverage.
scotej added a commit that referenced this pull request Aug 8, 2026
Preserve the stronger entire-screen capture implementation from #203 while distinguishing unverifiable WebView2 selections from explicit window or tab choices.

Add Linux behavior, display-surface propagation, and native denial passthrough coverage.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows force full screen sharing

2 participants