Skip to content

Conversation

@Brendonovich
Copy link
Contributor

@Brendonovich Brendonovich commented Nov 3, 2025

Summary by CodeRabbit

  • Removed Features

    • Screenshot capture removed from the app: tray menu item, keyboard shortcut label, and the user-facing “take screenshot” command are no longer available.
  • Backend Updates

    • macOS screen-capture permission handling improved and consolidated for more reliable permission requests and checks.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 3, 2025

Walkthrough

The PR removes the scap dependency and the screenshot-taking feature from the Tauri desktop app, migrates macOS screen-recording permission checks to a new scap-screencapturekit crate, and adds a permission module exposing has_permission and request_permission that call CoreGraphics FFI functions.

Changes

Cohort / File(s) Summary
Dependency Removal
\Cargo.toml`, `apps/desktop/src-tauri/Cargo.toml``
Removed scap from workspace dependencies and from the desktop Tauri crate's dependency list.
Screenshot Feature Removal
\apps/desktop/src-tauri/src/audio.rs`, `apps/desktop/src-tauri/src/lib.rs`, `apps/desktop/src-tauri/src/tray.rs``
Deleted Screenshot variant from AppSounds; removed RequestNewScreenshot struct and take_screenshot() command; removed TakeScreenshot tray variant and related menu mappings and event handling.
Frontend/API Types Update
\apps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx`, `apps/desktop/src/utils/tauri.ts``
Removed "Take Screenshot" action label; deleted takeScreenshot() command; updated GifQuality (renamed property to quality, added optional fast), and made start_time optional in AudioMeta and VideoMeta.
Permission Handling Migration
\apps/desktop/src-tauri/src/permissions.rs``
Replaced calls to scap::has_permission() / scap::request_permission() with scap_screencapturekit::has_permission() / scap_screencapturekit::request_permission() under macOS config.
New Permission Module
\crates/scap-screencapturekit/src/lib.rs`, `crates/scap-screencapturekit/src/permission.rs``
Added permission module and re-exported has_permission and request_permission; implemented FFI wrappers for CGPreflightScreenCaptureAccess and CGRequestScreenCaptureAccess with #[link(name = "CoreGraphics", kind = "framework")].

Sequence Diagram(s)

sequenceDiagram
    participant App as Desktop App
    participant Perms as permissions.rs
    participant SCK as scap_screencapturekit
    participant CG as CoreGraphics (FFI)

    rect rgb(240, 249, 255)
    Note over App,CG: macOS Screen Recording Permission Flow (New)
    end

    App->>Perms: check_permissions()
    Perms->>SCK: has_permission()
    SCK->>CG: CGPreflightScreenCaptureAccess()
    CG-->>SCK: i32 (LSB = permission bit)
    SCK-->>Perms: bool
    Perms-->>App: permission status

    alt Permission Required
        App->>Perms: request_permission()
        Perms->>SCK: request_permission()
        SCK->>CG: CGRequestScreenCaptureAccess()
        CG-->>SCK: i32 (1 = success)
        SCK-->>Perms: bool
        Perms-->>App: request result
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Changes span backend (Tauri Rust), a new FFI module, frontend types, and UI labels.
  • New FFI functions and linking need verification; ensure correct signatures and behavior.
  • Verify no remaining references to removed screenshot APIs.

Areas that may require extra attention:

  • crates/scap-screencapturekit/src/permission.rs — FFI declarations, #[link] usage, and bit interpretation.
  • apps/desktop/src-tauri/src/lib.rs and tray/menu handling — ensure no orphaned command/event registrations remain.
  • apps/desktop/src/utils/tauri.ts — type changes (GifQuality, optional start_time) and removed takeScreenshot usage across frontend.

Possibly related PRs

Suggested labels

codex

Poem

🐰 I hopped through crates and trimmed the tree,
Scap hopped out, screencapture hopped in with glee,
CoreGraphics whispers permission true,
Menu lights dim where screenshots flew,
A tidy burrow — ready for new code to be. ✨

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "remove old scap and add permission check to scap-avfoundation" is substantially related to the actual changes in the PR. The title correctly identifies the two main objectives: removing the old scap dependency (evidenced by deletion of scap from Cargo.toml files, removal of screenshot functionality in lib.rs, audio.rs, tray.rs, and UI files) and adding permission check functionality (evidenced by new permission module and functions in crates/scap-screencapturekit). However, the title references "scap-avfoundation" while the actual permission functions are being added to "scap-screencapturekit," which represents a discrepancy in the crate name that could potentially cause confusion during review, though the core functionality and intent of the PR are accurately captured.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch remove-scap

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4569cd8 and 295b190.

📒 Files selected for processing (1)
  • crates/scap-screencapturekit/src/permission.rs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/scap-screencapturekit/src/permission.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Clippy
  • GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
  • GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
  • GitHub Check: Analyze (rust)

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 and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (2)
apps/desktop/src-tauri/src/tray.rs (1)

64-70: Remove commented-out code.

The commented-out TakeScreenshot menu item should be deleted entirely rather than left as a comment, as the functionality has been intentionally removed per the PR objectives.

Apply this diff to remove the dead code:

-            // &MenuItem::with_id(
-            //     app,
-            //     TrayItem::TakeScreenshot,
-            //     "Take Screenshot",
-            //     true,
-            //     None::<&str>,
-            // )?,
             &MenuItem::with_id(
apps/desktop/src-tauri/src/permissions.rs (1)

74-75: Remove redundant cfg attribute.

Line 74's #[cfg(target_os = "macos")] is redundant since it's already inside a macOS-only block that starts at line 67.

Apply this diff:

             OSPermission::ScreenRecording => {
-                #[cfg(target_os = "macos")]
                 scap_screencapturekit::request_permission();
             }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 65deca5 and 4569cd8.

⛔ Files ignored due to path filters (2)
  • Cargo.lock is excluded by !**/*.lock
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (10)
  • Cargo.toml (0 hunks)
  • apps/desktop/src-tauri/Cargo.toml (0 hunks)
  • apps/desktop/src-tauri/src/audio.rs (0 hunks)
  • apps/desktop/src-tauri/src/lib.rs (0 hunks)
  • apps/desktop/src-tauri/src/permissions.rs (2 hunks)
  • apps/desktop/src-tauri/src/tray.rs (1 hunks)
  • apps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx (0 hunks)
  • apps/desktop/src/utils/tauri.ts (3 hunks)
  • crates/scap-screencapturekit/src/lib.rs (1 hunks)
  • crates/scap-screencapturekit/src/permission.rs (1 hunks)
💤 Files with no reviewable changes (5)
  • apps/desktop/src/routes/(window-chrome)/settings/hotkeys.tsx
  • Cargo.toml
  • apps/desktop/src-tauri/Cargo.toml
  • apps/desktop/src-tauri/src/audio.rs
  • apps/desktop/src-tauri/src/lib.rs
🧰 Additional context used
📓 Path-based instructions (5)
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Format Rust code using rustfmt and ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.

Files:

  • apps/desktop/src-tauri/src/permissions.rs
  • crates/scap-screencapturekit/src/lib.rs
  • crates/scap-screencapturekit/src/permission.rs
  • apps/desktop/src-tauri/src/tray.rs
crates/*/src/**/*

📄 CodeRabbit inference engine (AGENTS.md)

Rust crates should place tests within the src/ and/or a sibling tests/ directory for each crate inside crates/*.

Files:

  • crates/scap-screencapturekit/src/lib.rs
  • crates/scap-screencapturekit/src/permission.rs
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by running pnpm format.

Files:

  • apps/desktop/src/utils/tauri.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g., user-menu.tsx).
Use PascalCase for React/Solid components.

Files:

  • apps/desktop/src/utils/tauri.ts
**/tauri.ts

📄 CodeRabbit inference engine (AGENTS.md)

Do not edit auto-generated files named tauri.ts.

Files:

  • apps/desktop/src/utils/tauri.ts
🧠 Learnings (2)
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: On macOS, desktop app permissions (screen/mic) apply to the terminal running `pnpm dev:desktop`.

Applied to files:

  • apps/desktop/src-tauri/src/permissions.rs
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to **/tauri.ts : Do not edit auto-generated files named `tauri.ts`.

Applied to files:

  • apps/desktop/src/utils/tauri.ts
🧬 Code graph analysis (4)
apps/desktop/src-tauri/src/permissions.rs (1)
crates/scap-screencapturekit/src/permission.rs (2)
  • request_permission (1-3)
  • has_permission (5-7)
crates/scap-screencapturekit/src/lib.rs (2)
crates/scap-screencapturekit/src/permission.rs (2)
  • has_permission (5-7)
  • request_permission (1-3)
apps/desktop/src-tauri/src/permissions.rs (1)
  • request_permission (66-112)
crates/scap-screencapturekit/src/permission.rs (1)
apps/desktop/src-tauri/src/permissions.rs (1)
  • request_permission (66-112)
apps/desktop/src-tauri/src/tray.rs (1)
apps/desktop/src/utils/tauri.ts (3)
  • RecordingStarted (448-448)
  • RecordingStopped (449-449)
  • RequestOpenSettings (454-454)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Clippy
  • GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
  • GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
  • GitHub Check: Analyze (rust)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
apps/desktop/src/utils/tauri.ts (1)

1-493: Auto-generated file - no manual edits required.

This file is auto-generated by tauri-specta (as noted in line 2). The type updates and command removal reflect changes in the Rust backend and will be regenerated automatically when the backend changes.

Based on learnings.

apps/desktop/src-tauri/src/tray.rs (3)

2-2: LGTM: Import cleanup aligns with screenshot removal.

The removal of RequestNewScreenshot from imports is consistent with the elimination of the screenshot functionality from the tray menu.


19-56: LGTM: TakeScreenshot completely removed from tray items.

The TrayItem enum and its conversions have been properly updated to remove all TakeScreenshot references. The implementation is clean and consistent.


58-202: LGTM: Tray menu creation and event handling properly updated.

The create_tray function correctly excludes any screenshot-related menu items and event handlers. The remaining tray functionality (recordings, settings, logs, quit) is intact and properly implemented.

apps/desktop/src-tauri/src/permissions.rs (1)

167-167: LGTM!

The migration from scap::has_permission() to scap_screencapturekit::has_permission() is correct.

crates/scap-screencapturekit/src/lib.rs (1)

5-5: LGTM!

The module declaration and re-exports follow standard Rust patterns and provide a clean public API for permission handling.

Also applies to: 9-9

@Brendonovich Brendonovich merged commit 48a7fe2 into main Nov 3, 2025
17 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Nov 4, 2025
1 task
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.

2 participants