Skip to content

fix(mcp): record the window an in-app take names, even where Apple's picker owns sources - #1237

Merged
EtienneLescot merged 1 commit into
mainfrom
fix/1232-inapp-mcp-take-names-its-source
Oct 14, 2026
Merged

EtienneLescot merged 1 commit into
mainfrom
fix/1232-inapp-mcp-take-names-its-source

Conversation

@EtienneLescot

@EtienneLescot EtienneLescot commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

Fixes #1232.

On macOS 15.2 or later, with a default launch, pointHudAtTake cleared the selected source for an in-app MCP take so that Apple's picker would ask which window or display to record. An MCP client is not there to answer: startRecording with window waited 120 s, failed with "capture had not started", and left the picker on screen, even after OpenScreen quit (measured on 2026-10-14, build run 38615516865).

A named take now records what it names in that case too. It goes through the same enumeration and selectSource path as everywhere else, the path the CLI's record already uses in picker mode. A non-picker source makes the capture spawn its own helper (isMacPickerSourceId is false), so no picker opens. That path needs the Screen Recording grant, which the picker otherwise makes unnecessary. It is read fresh with readMacScreenCaptureAccess(). A Mac that refused it gets an error naming the System Settings pane instead of a picker. A probe that could not answer (no helper, a crash, a timeout) is not taken for a refusal (electron/mcp/macNamedTake.ts, unit-tested). The in-app startRecording description now says what happens on macOS and on Linux, where the portal still asks.

Because the picker is never opened for an MCP take, it can no longer be left on screen after a timeout either.

Checks: npx vitest --run electron/mcp (151 tests), npx tsc --noEmit, npx tsc -p tsconfig.test.json --noEmit and Biome pass. Not yet run in a packaged build: the end-to-end check is an in-app startRecording on a window from a default (non-legacy) launch on macOS 15.2+.

Summary by CodeRabbit

  • Bug Fixes
    • On macOS, named recordings now check Screen & System Audio Recording access before selecting a window or display. If access is denied, recording is refused with a message explaining the required permission.
  • Documentation
    • Clarified that macOS named recordings capture the selected window or display directly, while Linux continues to use its system picker.

…picker owns sources

On macOS 15.2+ the in-app startRecording cleared the source and left the choice to Apple's picker, which waits for a person: the call timed out after 120 s and left the picker on screen. A named take now records its window or display directly, as the stdio server and the legacy picker do, and is refused with a clear message when Screen Recording is denied. Fixes #1232.
@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 139f4ca6-4d42-44e6-b69f-a69cbc98e71c
📥 Commits

Reviewing files that changed from the base of the PR and between 99d46a3 and d9897d2.

📒 Files selected for processing: 4
  • electron/ipc/handlers.ts
  • electron/mcp/macNamedTake.test.ts
  • electron/mcp/macNamedTake.ts
  • electron/mcp/media-tools.ts

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

📝 Walkthrough

Named macOS takes now check Screen Recording access before source enumeration. Denied access produces NAMED_TAKE_NEEDS_SCREEN_ACCESS; other access statuses continue through named-source selection. Tests and the recording note describe these rules.

Changes: macOS Named Take Source Selection

Layer / File(s) Summary
Named-take access policy
electron/mcp/macNamedTake.ts, electron/mcp/macNamedTake.test.ts
namedTakeMayEnumerate returns false for denied access and true for other statuses. Tests cover granted, denied, and probe-unavailable statuses, and check the permission message.
Handler access check and source selection
electron/ipc/handlers.ts, electron/mcp/media-tools.ts
pointHudAtTake checks access before continuing to source enumeration and named-source selection. The recording note says macOS records the named window or display and refuses the call without OpenScreen Screen Recording permission; Linux continues to use the system portal picker.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant MCP as MCP startRecording
  participant Take as pointHudAtTake
  participant Access as readMacScreenCaptureAccess
  participant Policy as namedTakeMayEnumerate
  participant Sources as Source enumeration
  MCP->>Take: Request a named window or display
  Take->>Access: Read macOS screen-capture access
  Access-->>Take: Return access status
  Take->>Policy: Check whether enumeration is allowed
  Policy-->>Take: Return eligibility
  alt Enumeration allowed
    Take->>Sources: Enumerate and select the named source
  else Access denied
    Take-->>MCP: Throw NAMED_TAKE_NEEDS_SCREEN_ACCESS
  end
Loading
Merge Risk: ⚪ Minimal · up to d9897

The named macOS take path is ready for normal validation, including the planned packaged-app check.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly identifies the main change: named MCP takes record the requested window instead of relying on Apple’s picker.
Description check Passed The description explains the change, references issue #1232, and reports testing. It omits the template’s type of change, release impact, and desktop impact selections, but the main information is com…
Linked Issues check Passed Issue #1232 asks in-app MCP takes that name a window or display to select and record that source, and to report a Screen Recording permission denial instead of opening Apple’s picker. The handler now …
Out of Scope Changes check Passed The changes in electron/ipc/handlers.ts, electron/mcp/macNamedTake.ts, its tests, and electron/mcp/media-tools.ts all support issue #1232. They implement named-source selection, permission handl…
  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@EtienneLescot
EtienneLescot merged commit 6bfbcb3 into main Oct 14, 2026
19 checks passed
@EtienneLescot
EtienneLescot deleted the fix/1232-inapp-mcp-take-names-its-source branch October 14, 2026 19:55
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.

[Bug]: macOS 15.2+: in-app MCP startRecording opens Apple's picker instead of recording the window, then times out

1 participant