Skip to content

fix(mcp): fail startRecording when the camera cannot be opened - #1371

Merged
EtienneLescot merged 1 commit into
mainfrom
pilot/fix-1303
Oct 23, 2026
Merged

EtienneLescot merged 1 commit into
mainfrom
pilot/fix-1303

Conversation

@EtienneLescot

@EtienneLescot EtienneLescot commented Oct 23, 2026 •

Copy link
Copy Markdown
Member

What was wrong

startRecording with camera: true (and openscreen record --camera) runs the hidden CLI record runner. The runner turned the camera on, which only checks permission and that a camera exists. The actual getUserMedia comes after: when it fails (camera held by another app, e.g. the running OpenScreen app on Linux, NotReadableError), useScreenRecorder shows a toast, turns the camera back off and the capture starts without it. Nothing in the hidden runner reported that, so the take was saved without webcam and the tool returned success.

Not Linux-only in the code: any platform where the camera cannot be opened twice (Windows too) takes the same path. Linux is just where the running app keeps it open.

What changed

  • CliRecordRunner: when the request asked for the camera and the recorder turns it back off before capture starts, the run fails with "The camera could not be opened: another app is using it (OpenScreen itself, when it is running), or it was disconnected". The MCP tool returns that as The recording did not start: ....
  • If the capture was already starting when the failure lands, the runner stops it instead of reporting it.
  • The HUD and the in-app MCP recorder are unchanged (the app reuses its own camera stream there).

How it was tested

  • New unit test in src/cli/CliRecordRunner.test.tsx (fails without the fix).
  • npx vitest --run src/cli electron/mcp src/hooks, npx tsc --noEmit, npx tsc -p tsconfig.test.json --noEmit, npm run lint: green. Full suite left to CI.

Pending before merge: a real Linux check, app running + MCP take with camera: true, before/after.

Refs #1303

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Camera-enabled recordings now fail with a clear error if the camera cannot be opened before recording begins, instead of continuing without camera footage.
    • If recording starts after this failure, it is stopped automatically. The failure is reported without showing a “Recording started” message.

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b4f494b9-eb61-43ef-86cc-2c729ed0aea7
📥 Commits

Reviewing files that changed from the base of the PR and between b664d49 and 444907f.

📒 Files selected for processing: 2
  • src/cli/CliRecordRunner.test.tsx
  • src/cli/CliRecordRunner.tsx

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

📝 Walkthrough

The CLI runner now reports failure if the requested camera turns off before recording starts. If recording starts after that failure, the runner turns recording off. A test covers this camera acquisition failure case.

Changes: CLI camera capture failure handling

Layer / File(s) Summary
Handle camera failure before recording
src/cli/CliRecordRunner.tsx, src/cli/CliRecordRunner.test.tsx
The runner reports a camera-open error if the requested camera turns off before recording starts. If recording starts after failure, it turns recording off. The test checks the failure and stop behavior.

Priority: ➖ Normal

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

Change: Bug fix

Suggested reviewers: mcmah309

Merge Risk: ⚪ Minimal · up to 44490

The camera-open failure handling is mergeable after normal checks. A live Linux camera check remains pending, but no merge-blocking failure is established.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 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: failing startRecording when the requested camera cannot be opened.
Description check Passed The description explains the problem, implementation, testing, related issue, and pending Linux validation. It does not use every template heading or include the template checkboxes, but it provides t…
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 explains the problem, implementation, testing, related issue, and pending Linux validation. It does not use every template heading or include the template checkboxes, but it provides the required information in equivalent sections.

✨ 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

Copy link
Copy Markdown
Member Author

Checked on a real Linux machine (Ubuntu 24.04, GNOME Wayland, built-in webcam), through openscreen mcp over stdio, startRecording {display: 0, camera: true}. Same app build for both heads, only app.asar swapped.

Camera held by another process (ffmpeg -f v4l2 -i /dev/video0):

  • main (b664d49): {"recording":true,...}, and the take is saved with no webcam file and no webcamVideoPath. Bug reproduced.
  • this branch: fails in 0.4 s, before the screen-share prompt even shows: The recording did not start: Error: The camera could not be opened: another app is using it (OpenScreen itself, when it is running), or it was disconnected

Camera free, this branch: success, take saved with its webcam file. No regression.

Side note, not this PR: the MCP message carries the renderer stack trace after that first line, as every CLI runner's fail() sends error.stack.

🤖 Generated with Claude Code

@EtienneLescot
EtienneLescot merged commit 8c19db8 into main Oct 23, 2026
20 checks passed
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.

1 participant