Skip to content

fix(desktop): allow pointer lock in the built-in browser - #25275

Open
msfeldstein wants to merge 5 commits into
pingdotgg:mainfrom
msfeldstein:fix/preview-pointer-lock
Open

msfeldstein wants to merge 5 commits into
pingdotgg:mainfrom
msfeldstein:fix/preview-pointer-lock

Conversation

@msfeldstein

@msfeldstein msfeldstein commented Oct 26, 2026 •

Copy link
Copy Markdown

Problem

Pages opened in the desktop app's built-in browser can't capture the mouse. element.requestPointerLock() is rejected, so browser games and 3D viewers with mouse-look can't be tested in the preview.

The preview session grants only the permissions in ALLOWED_PREVIEW_PERMISSIONS and denies everything else. Electron gates pointer lock behind its pointerLock permission, and that permission isn't on the list. The comments there explain why local-fonts and notifications are denied on purpose, but nothing says pointer lock was meant to be blocked.

Fix

Add pointerLock to ALLOWED_PREVIEW_PERMISSIONS. It goes through the same request and check handlers as fullscreen, and Chromium always lets the user release it with Escape. This qualifies as a small, focused fix for an obvious bug: one permission is added and nothing else in the preview changes.

Verification

  • Extended the existing permission test in BrowserSession.test.ts so it asserts that both handlers grant pointerLock. vp test run apps/desktop/src/preview/BrowserSession.test.ts passes (7/7).
  • vp run check:changed passes: format, lint, knip, typecheck, and tests.
  • Manual check on macOS with vp run dev:desktop: a browser game opened in the built-in browser. Before the fix, clicking to capture the mouse did nothing. With the fix, the game captures the mouse and Escape releases it.

Written by Claude Opus 5.5 in Claude Code, running inside T3 Code.

🤖 Generated with Claude Code

Games and 3D viewers previewed in the built-in browser could not capture
the mouse because the preview session denied the pointerLock permission.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Oct 26, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — This PR broadens the built-in browser's default permission allowlist so preview pages can capture the pointer, with corresponding test coverage. Because it changes a product default, the permission-policy change requires human review.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 365ec13 and f40f98a.

Files selected for processing: 2
  • apps/desktop/src/preview/BrowserSession.test.ts
  • apps/desktop/src/preview/BrowserSession.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

Walkthrough

Preview sessions now allow the pointerLock permission. The test checks that both the request and check handlers allow it.

Changes: Preview Session Permissions

Layer / File(s) Summary
Allow Pointer Lock
apps/desktop/src/preview/BrowserSession.ts, apps/desktop/src/preview/BrowserSession.test.ts
The preview-session permission allowlist includes pointerLock. The test expects both permission handlers to allow it.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Bug fix

Suggested reviewers: maria-rcks, fixfon

Merge Risk: Minimal · up to f40f9

The pointer-lock allowlist change and both permission-handler tests present no merge-blocking risk.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title is concise, specific, and accurately describes the main change: enabling pointer lock in the desktop app's built-in browser.
Description check Passed The description clearly explains the problem, fix, verification steps, test results, manual check, and why the change is narrowly scoped. It uses a "Fix" heading instead of the template's "Change" hea…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description clearly explains the problem, fix, verification steps, test results, manual check, and why the change is narrowly scoped. It uses a "Fix" heading instead of the template's "Change" heading and does not include a separate "Scope and approval" heading, but it provides the required scope rationale for this obvious bug fix.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

kept @msfeldstein's implementation, regression coverage, and commit history. merged current main without rebasing and removed the redundant pointer-lock comment. the final diff is two added lines: the shared permission allowlist entry and its existing both-handler test case.

verification at addf4a92bbd71db8cb1b497c24167a2b8b1ebdd5:

  • all 24 reported checks are green, including ci build/typecheck, lint, and tests. the bot review has no actionable findings. two independent code reviews found no issues.
  • blacksmith: vp run check:changed passed format, lint, knip, affected typechecks, and all seven browser-session tests. the regression test first failed against main's production permission policy (one failed, six passed).
  • native electron 44.9.3 runtime, using the actual BrowserSession service and mdn's pointer-lock demo: main's policy returned SecurityError with no locked element; this head returned locked: CANVAS. document.exitPointerLock() released it.

unverified: the complete built-in browser interaction, escape-key release, and macos/windows. the desktop dev renderer stayed blank and cdp requests timed out on this vm, so no full-app before/after recording is available. the native runtime check above is narrower than full desktop ui verification. the permission-default change still needs maria's human approval; i have not merged it.

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

partial verification at addf4a92bbd71db8cb1b497c24167a2b8b1ebdd5 on this linux boat vm. this is not verification of the pointer-lock fix.

  • read the description, discussion, commits, and two-line diff. the pr is open, with no pr commits by @juliusmarminge or @t3dotgg.
  • created ../main at github/main (7c6446b81a), installed with vp i, and started the real dev environment. stopped main before starting the exact pr head, installed with vp i, and ran setsid nohup ... vp run dev in the background.
  • used repo Playwright with /usr/bin/google-chrome, paired the pr web client, added /home/user/t3code through add project → local folder, selected codex gpt-6.1-sol, and submitted a real provider turn. the completed response was provider path verified. no mocks, demo turns, or database writes were used to create this scenario.
  • all 24 reported CI checks are successful. no new local check run or source change was made.

real codex gpt-6.1-sol turn completed in the pr web client; this does not prove desktop pointer lock

short runtime transcript (pairing tokens omitted):

main: mode=dev serverPort=16733 webPort=8693
main: Listening on http://127.0.0.1:16733
main desktop: missing libsecret-1 development headers
installed libsecret-1-dev and retried
main desktop: web build reported built in 99m 21s
main desktop: server bundling began; electron debugging endpoint never appeared
pr head: addf4a92bbd71db8cb1b497c24167a2b8b1ebdd5
pr: Listening on http://127.0.0.1:13773; webPort=5733
web client: GPT-6.1-Sol → completed response: provider path verified.
pr desktop: bounded five-minute startup did not reach electron

Not verified

pointer capture itself, escape-key release, fullscreen/other neighboring preview permissions, and before/after interaction evidence remain unverified. the change is in electron's preview-session permission handlers; standalone chrome success cannot validate it. main's desktop startup exceeded the requested ten-minute stall limit and consumed most of the task budget. the bounded pr desktop attempt also did not expose the electron debugging endpoint. i could not reproduce the original bug or exercise the changed desktop flow, and did not infer success from the earlier narrower native-runtime check.

macos, windows, and native ios/android were not exercised. no pointer-lock recording exists. no source changes, commits, pushes, labels, merges, or rebases were made. task-owned dev processes and chrome were stopped. result: unverifiable for the changed behavior, with the real web/provider path verified separately.

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

merged current github/main (5ef56853ef18) into the existing contributor head without rebasing. resolved the permission-test conflict by preserving main's clipboard-read consent policy and this pr's pointer-lock case. the diff against main remains the pointer-lock allowlist entry and its existing both-handler test case.

at 365ec131271618013deeeb2b26f223cd2236faa6, blacksmith vp run check:changed passed format, lint, both knip checks, affected desktop/scripts typechecks, and all eight browser-session tests. two independent read-only reviews found no code issues. pushed to msfeldstein:fix/preview-pointer-lock without force.

full desktop pointer-lock capture, escape release, and interaction evidence remain unverified this turn. prior runtime evidence is narrower than full-app verification. macroscope's permission-policy human approval remains for maria. ci on the merge commit is pending.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants