From c41bf472d44ced34e2986af5b06bc4d3ef581b15 Mon Sep 17 00:00:00 2001 From: Logan Nguyen Date: Wed, 8 Apr 2026 12:42:51 -0500 Subject: [PATCH 1/3] refactor: remove Input Monitoring permission requirement from onboarding Simplify the onboarding permission flow from 3 steps (Accessibility, Input Monitoring, Screen Recording) to 2 steps (Accessibility, Screen Recording). Input Monitoring checks and IOKit FFI bindings are removed from permissions.rs and PermissionsStep.tsx. The CGEventTap at HID level receives cross-app keyboard events without needing a TCC entry. Signed-off-by: Logan Nguyen --- CLAUDE.md | 2 +- src-tauri/src/activator.rs | 6 - src-tauri/src/lib.rs | 9 +- src-tauri/src/permissions.rs | 106 +----- src/__tests__/OnboardingView.test.tsx | 452 ++++-------------------- src/view/onboarding/PermissionsStep.tsx | 187 ++-------- 6 files changed, 103 insertions(+), 659 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 872f2d2..3b852ee 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -110,7 +110,7 @@ Never commit files generated by superpowers skills (design specs, implementation - **macOS only** — uses NSPanel, Core Graphics event taps, macOS Control key - **Privacy-first** — Ollama runs locally; Docker sandbox drops all capabilities and isolates network -- **Three permissions required** — Accessibility (CGEventTap creation), Input Monitoring (cross-app key delivery), Screen Recording (/screen command) +- **Two permissions required** — Accessibility (CGEventTap creation), Screen Recording (/screen command) ### CGEventTap configuration — DO NOT CHANGE these two settings diff --git a/src-tauri/src/activator.rs b/src-tauri/src/activator.rs index 51a76b3..b5f2908 100644 --- a/src-tauri/src/activator.rs +++ b/src-tauri/src/activator.rs @@ -247,12 +247,6 @@ fn try_initialize_tap(is_active: &Arc, on_activation: &Arc) -> where F: Fn() + Send + Sync + 'static, { - let im_granted = crate::permissions::is_input_monitoring_granted(); - eprintln!( - "thuki: [activator] Input Monitoring permission: granted={im_granted} \ - (cross-app hotkey requires this)" - ); - let state = Arc::new(Mutex::new(ActivationState { last_trigger: None, is_pressed: false, diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 94053fd..fe3a27c 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -402,9 +402,8 @@ fn notify_frontend_ready(app_handle: tauri::AppHandle, db: tauri::State bool { - !accessibility || !screen_recording || !input_monitoring +/// Both Accessibility (hotkey listener) and Screen Recording (/screen command) +/// must be granted for Thuki to function fully. If either is missing the +/// onboarding screen is shown instead of the normal overlay. +pub fn needs_onboarding(accessibility: bool, screen_recording: bool) -> bool { + !accessibility || !screen_recording } // ─── macOS Permission Checks ───────────────────────────────────────────────── @@ -36,25 +29,6 @@ extern "C" { fn AXIsProcessTrusted() -> bool; } -/// IOKit constants for Input Monitoring permission checks. -#[cfg(target_os = "macos")] -const IOHID_REQUEST_TYPE_LISTEN_EVENT: u32 = 1; -#[cfg(target_os = "macos")] -const IOHID_ACCESS_TYPE_GRANTED: u32 = 1; - -#[cfg(target_os = "macos")] -#[link(name = "IOKit", kind = "framework")] -extern "C" { - /// Checks whether the process has Input Monitoring access. - /// Returns kIOHIDAccessTypeGranted (1) if granted, 0 if not determined, - /// 2 if denied. - fn IOHIDCheckAccess(request_type: u32) -> u32; - - /// Requests Input Monitoring access, showing the system permission dialog - /// on first call. Returns true if access was granted. - fn IOHIDRequestAccess(request_type: u32) -> bool; -} - /// Returns whether the process currently has Accessibility permission. #[cfg(target_os = "macos")] #[cfg_attr(coverage_nightly, coverage(off))] @@ -62,21 +36,6 @@ pub fn is_accessibility_granted() -> bool { unsafe { AXIsProcessTrusted() } } -/// Returns whether the process currently has Input Monitoring permission. -/// -/// Input Monitoring (`kTCCServiceListenEvent`) is required for a CGEventTap at -/// HID level to receive keyboard events from other applications. Without it, -/// the tap only sees events generated within the Thuki process itself, making -/// the double-tap hotkey invisible when the user's focus is elsewhere. -/// -/// Unlike Screen Recording, Input Monitoring does not require a process restart -/// after being granted; the CGEventTap immediately begins receiving cross-app events. -#[cfg(target_os = "macos")] -#[cfg_attr(coverage_nightly, coverage(off))] -pub fn is_input_monitoring_granted() -> bool { - unsafe { IOHIDCheckAccess(IOHID_REQUEST_TYPE_LISTEN_EVENT) == IOHID_ACCESS_TYPE_GRANTED } -} - /// Returns whether the process currently has Screen Recording permission. /// /// Uses `CGPreflightScreenCaptureAccess`, which only returns `true` after @@ -102,44 +61,6 @@ pub fn check_accessibility_permission() -> bool { is_accessibility_granted() } -/// Returns whether Input Monitoring permission has been granted. -#[tauri::command] -#[cfg(target_os = "macos")] -#[cfg_attr(coverage_nightly, coverage(off))] -pub fn check_input_monitoring_permission() -> bool { - is_input_monitoring_granted() -} - -/// Triggers the macOS Input Monitoring permission dialog. -/// -/// `IOHIDRequestAccess` registers the app in TCC and shows the system-level -/// "X would like to monitor input events" prompt. If the user previously denied -/// the permission, the dialog is skipped and the call returns false; the -/// onboarding UI then directs the user to System Settings directly. -#[tauri::command] -#[cfg(target_os = "macos")] -#[cfg_attr(coverage_nightly, coverage(off))] -pub fn request_input_monitoring_access() { - unsafe { - IOHIDRequestAccess(IOHID_REQUEST_TYPE_LISTEN_EVENT); - } -} - -/// Opens System Settings to the Input Monitoring privacy pane. -#[tauri::command] -#[cfg(target_os = "macos")] -#[cfg_attr(coverage_nightly, coverage(off))] -pub fn open_input_monitoring_settings() -> Result<(), String> { - std::process::Command::new("open") - .arg( - "x-apple.systempreferences:com.apple.preference.security\ - ?Privacy_ListenEvent", - ) - .spawn() - .map(|_| ()) - .map_err(|e| e.to_string()) -} - /// Opens System Settings to the Accessibility privacy pane so the user can /// enable the permission without encountering the native system popup. /// @@ -244,27 +165,22 @@ mod tests { use super::*; #[test] - fn needs_onboarding_false_when_all_granted() { - assert!(!needs_onboarding(true, true, true)); + fn needs_onboarding_false_when_both_granted() { + assert!(!needs_onboarding(true, true)); } #[test] fn needs_onboarding_true_when_accessibility_missing() { - assert!(needs_onboarding(false, true, true)); + assert!(needs_onboarding(false, true)); } #[test] fn needs_onboarding_true_when_screen_recording_missing() { - assert!(needs_onboarding(true, false, true)); - } - - #[test] - fn needs_onboarding_true_when_input_monitoring_missing() { - assert!(needs_onboarding(true, true, false)); + assert!(needs_onboarding(true, false)); } #[test] - fn needs_onboarding_true_when_all_missing() { - assert!(needs_onboarding(false, false, false)); + fn needs_onboarding_true_when_both_missing() { + assert!(needs_onboarding(false, false)); } } diff --git a/src/__tests__/OnboardingView.test.tsx b/src/__tests__/OnboardingView.test.tsx index 020344a..8baefbd 100644 --- a/src/__tests__/OnboardingView.test.tsx +++ b/src/__tests__/OnboardingView.test.tsx @@ -13,31 +13,16 @@ describe('OnboardingView', () => { vi.useRealTimers(); }); - /** - * Set up invoke mock for the standard permission check commands. - * - accessibility: whether check_accessibility_permission returns true - * - inputMonitoring: whether check_input_monitoring_permission returns true - * - screenRecording: whether check_screen_recording_tcc_granted returns true - */ - function setupPermissions( - accessibility: boolean, - inputMonitoring = false, - screenRecording = false, - ) { + function setupPermissions(accessibility: boolean, screenRecording = false) { invoke.mockImplementation(async (cmd: string) => { if (cmd === 'check_accessibility_permission') return accessibility; - if (cmd === 'check_input_monitoring_permission') return inputMonitoring; if (cmd === 'check_screen_recording_permission') return screenRecording; if (cmd === 'check_screen_recording_tcc_granted') return false; if (cmd === 'request_screen_recording_access') return; if (cmd === 'open_screen_recording_settings') return; - if (cmd === 'request_input_monitoring_access') return; - if (cmd === 'open_input_monitoring_settings') return; }); } - // ─── Basic render ────────────────────────────────────────────────────────── - it('shows step 1 as active when accessibility is not granted', async () => { setupPermissions(false); render(); @@ -57,19 +42,20 @@ describe('OnboardingView', () => { expect(screen.getByText("Let's get Thuki set up")).toBeInTheDocument(); }); - it('shows all three steps regardless of current active step', async () => { - setupPermissions(false); + it('skips to step 2 when accessibility is already granted on mount', async () => { + setupPermissions(true); render(); await act(async () => {}); - expect(screen.getByText('Accessibility')).toBeInTheDocument(); - expect(screen.getByText('Input Monitoring')).toBeInTheDocument(); - expect(screen.getByText('Screen Recording')).toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: /grant accessibility/i }), + ).toBeNull(); + expect( + screen.getByRole('button', { name: /open screen recording settings/i }), + ).toBeInTheDocument(); }); - // ─── Step 1: Accessibility ───────────────────────────────────────────────── - - it('clicking grant accessibility invokes open settings command', async () => { + it('clicking grant accessibility invokes request command', async () => { setupPermissions(false); render(); await act(async () => {}); @@ -83,7 +69,7 @@ describe('OnboardingView', () => { expect(invoke).toHaveBeenCalledWith('open_accessibility_settings'); }); - it('shows spinner while polling after accessibility grant request', async () => { + it('shows spinner while polling after grant request', async () => { setupPermissions(false); render(); await act(async () => {}); @@ -94,6 +80,7 @@ describe('OnboardingView', () => { ); }); + // Button should be disabled/spinner state while checking const btn = screen.getByRole('button', { name: /checking|grant accessibility/i, }); @@ -104,7 +91,7 @@ describe('OnboardingView', () => { let accessibilityGranted = false; invoke.mockImplementation(async (cmd: string) => { if (cmd === 'check_accessibility_permission') return accessibilityGranted; - if (cmd === 'check_input_monitoring_permission') return false; + if (cmd === 'check_screen_recording_permission') return false; if (cmd === 'open_accessibility_settings') return; }); @@ -117,15 +104,17 @@ describe('OnboardingView', () => { ); }); + // First poll fires but permission still false await act(async () => { await vi.advanceTimersByTimeAsync(500); }); - // Still on step 1, input monitoring button not yet shown + // Still on step 1, open screen recording button not yet shown expect( - screen.queryByRole('button', { name: /grant input monitoring/i }), + screen.queryByRole('button', { name: /open screen recording settings/i }), ).toBeNull(); + // Now grant it and fire second poll accessibilityGranted = true; await act(async () => { await vi.advanceTimersByTimeAsync(500); @@ -133,35 +122,39 @@ describe('OnboardingView', () => { // Step 2 now active expect( - screen.getByRole('button', { name: /grant input monitoring/i }), + screen.getByRole('button', { name: /open screen recording settings/i }), ).toBeInTheDocument(); }); - it('advances to step 2 (input monitoring) when polling detects accessibility granted', async () => { + it('advances to step 2 when polling detects accessibility granted', async () => { let accessibilityGranted = false; invoke.mockImplementation(async (cmd: string) => { if (cmd === 'check_accessibility_permission') return accessibilityGranted; - if (cmd === 'check_input_monitoring_permission') return false; + if (cmd === 'check_screen_recording_permission') return false; if (cmd === 'open_accessibility_settings') return; }); render(); await act(async () => {}); + // Click grant await act(async () => { fireEvent.click( screen.getByRole('button', { name: /grant accessibility/i }), ); }); + // Grant becomes true before next poll accessibilityGranted = true; + // Advance one poll interval await act(async () => { await vi.advanceTimersByTimeAsync(500); }); + // Step 2 should now be active expect( - screen.getByRole('button', { name: /grant input monitoring/i }), + screen.getByRole('button', { name: /open screen recording settings/i }), ).toBeInTheDocument(); }); @@ -169,7 +162,7 @@ describe('OnboardingView', () => { let accessibilityGranted = false; invoke.mockImplementation(async (cmd: string) => { if (cmd === 'check_accessibility_permission') return accessibilityGranted; - if (cmd === 'check_input_monitoring_permission') return false; + if (cmd === 'check_screen_recording_permission') return false; if (cmd === 'open_accessibility_settings') return; }); @@ -190,168 +183,8 @@ describe('OnboardingView', () => { expect(screen.getByText('Granted')).toBeInTheDocument(); }); - // ─── Mount: skip completed steps ────────────────────────────────────────── - - it('skips to step 2 when accessibility is already granted on mount', async () => { - setupPermissions(true, false); - render(); - await act(async () => {}); - - expect( - screen.queryByRole('button', { name: /grant accessibility/i }), - ).toBeNull(); - expect( - screen.getByRole('button', { name: /grant input monitoring/i }), - ).toBeInTheDocument(); - }); - - it('skips to step 3 when accessibility and input monitoring are both granted on mount', async () => { - setupPermissions(true, true); - render(); - await act(async () => {}); - - expect( - screen.queryByRole('button', { name: /grant accessibility/i }), - ).toBeNull(); - expect( - screen.queryByRole('button', { name: /grant input monitoring/i }), - ).toBeNull(); - expect( - screen.getByRole('button', { name: /open screen recording settings/i }), - ).toBeInTheDocument(); - }); - - // ─── Step 2: Input Monitoring ────────────────────────────────────────────── - - it('clicking grant input monitoring invokes request and open-settings commands', async () => { - setupPermissions(true, false); - render(); - await act(async () => {}); - - await act(async () => { - fireEvent.click( - screen.getByRole('button', { name: /grant input monitoring/i }), - ); - }); - - expect(invoke).toHaveBeenCalledWith('request_input_monitoring_access'); - expect(invoke).toHaveBeenCalledWith('open_input_monitoring_settings'); - }); - - it('shows spinner while polling after input monitoring grant request', async () => { - setupPermissions(true, false); - render(); - await act(async () => {}); - - await act(async () => { - fireEvent.click( - screen.getByRole('button', { name: /grant input monitoring/i }), - ); - }); - - const btn = screen.getByRole('button', { - name: /checking|grant input monitoring/i, - }); - expect(btn).toBeDisabled(); - }); - - it('keeps polling when input monitoring not yet granted on first poll interval', async () => { - let imGranted = false; - invoke.mockImplementation(async (cmd: string) => { - if (cmd === 'check_accessibility_permission') return true; - if (cmd === 'check_input_monitoring_permission') return imGranted; - if (cmd === 'request_input_monitoring_access') return; - if (cmd === 'open_input_monitoring_settings') return; - }); - - render(); - await act(async () => {}); - - await act(async () => { - fireEvent.click( - screen.getByRole('button', { name: /grant input monitoring/i }), - ); - }); - - await act(async () => { - await vi.advanceTimersByTimeAsync(500); - }); - - // Still on step 2, screen recording button not yet shown - expect( - screen.queryByRole('button', { name: /open screen recording settings/i }), - ).toBeNull(); - - imGranted = true; - await act(async () => { - await vi.advanceTimersByTimeAsync(500); - }); - - expect( - screen.getByRole('button', { name: /open screen recording settings/i }), - ).toBeInTheDocument(); - }); - - it('advances to step 3 when polling detects input monitoring granted', async () => { - let imGranted = false; - invoke.mockImplementation(async (cmd: string) => { - if (cmd === 'check_accessibility_permission') return true; - if (cmd === 'check_input_monitoring_permission') return imGranted; - if (cmd === 'request_input_monitoring_access') return; - if (cmd === 'open_input_monitoring_settings') return; - }); - - render(); - await act(async () => {}); - - await act(async () => { - fireEvent.click( - screen.getByRole('button', { name: /grant input monitoring/i }), - ); - }); - - imGranted = true; - - await act(async () => { - await vi.advanceTimersByTimeAsync(500); - }); - - expect( - screen.getByRole('button', { name: /open screen recording settings/i }), - ).toBeInTheDocument(); - }); - - it('step 2 shows granted badge after input monitoring is detected', async () => { - let imGranted = false; - invoke.mockImplementation(async (cmd: string) => { - if (cmd === 'check_accessibility_permission') return true; - if (cmd === 'check_input_monitoring_permission') return imGranted; - if (cmd === 'request_input_monitoring_access') return; - if (cmd === 'open_input_monitoring_settings') return; - }); - - render(); - await act(async () => {}); - - await act(async () => { - fireEvent.click( - screen.getByRole('button', { name: /grant input monitoring/i }), - ); - }); - - imGranted = true; - await act(async () => { - await vi.advanceTimersByTimeAsync(500); - }); - - const badges = screen.getAllByText('Granted'); - expect(badges.length).toBeGreaterThanOrEqual(2); - }); - - // ─── Step 3: Screen Recording ────────────────────────────────────────────── - it('clicking open screen recording settings registers app and opens settings', async () => { - setupPermissions(true, true); + setupPermissions(true); render(); await act(async () => {}); @@ -361,12 +194,13 @@ describe('OnboardingView', () => { ); }); + // Registers Thuki in TCC (so it appears in the list) then opens Settings expect(invoke).toHaveBeenCalledWith('request_screen_recording_access'); expect(invoke).toHaveBeenCalledWith('open_screen_recording_settings'); }); it('shows spinner while polling after opening screen recording settings', async () => { - setupPermissions(true, true); + setupPermissions(true); render(); await act(async () => {}); @@ -376,6 +210,7 @@ describe('OnboardingView', () => { ); }); + // Button should be disabled/spinner state while polling for tcc grant const btn = screen.getByRole('button', { name: /checking|open screen recording settings/i, }); @@ -383,7 +218,7 @@ describe('OnboardingView', () => { }); it('does not show quit and reopen immediately after clicking screen recording button', async () => { - setupPermissions(true, true); + setupPermissions(true); render(); await act(async () => {}); @@ -393,6 +228,7 @@ describe('OnboardingView', () => { ); }); + // Should NOT show quit & reopen until tcc grant is detected expect(screen.queryByRole('button', { name: /quit.*reopen/i })).toBeNull(); }); @@ -400,7 +236,6 @@ describe('OnboardingView', () => { let tccGranted = false; invoke.mockImplementation(async (cmd: string) => { if (cmd === 'check_accessibility_permission') return true; - if (cmd === 'check_input_monitoring_permission') return true; if (cmd === 'check_screen_recording_permission') return false; if (cmd === 'request_screen_recording_access') return; if (cmd === 'open_screen_recording_settings') return; @@ -416,12 +251,14 @@ describe('OnboardingView', () => { ); }); + // First poll: still not granted await act(async () => { await vi.advanceTimersByTimeAsync(500); }); expect(screen.queryByRole('button', { name: /quit.*reopen/i })).toBeNull(); + // Grant it tccGranted = true; await act(async () => { await vi.advanceTimersByTimeAsync(500); @@ -435,7 +272,6 @@ describe('OnboardingView', () => { it('shows quit and reopen after screen recording tcc grant is detected', async () => { invoke.mockImplementation(async (cmd: string) => { if (cmd === 'check_accessibility_permission') return true; - if (cmd === 'check_input_monitoring_permission') return true; if (cmd === 'check_screen_recording_permission') return false; if (cmd === 'request_screen_recording_access') return; if (cmd === 'open_screen_recording_settings') return; @@ -463,7 +299,6 @@ describe('OnboardingView', () => { it('clicking quit and reopen invokes quit_and_relaunch', async () => { invoke.mockImplementation(async (cmd: string) => { if (cmd === 'check_accessibility_permission') return true; - if (cmd === 'check_input_monitoring_permission') return true; if (cmd === 'check_screen_recording_permission') return false; if (cmd === 'request_screen_recording_access') return; if (cmd === 'open_screen_recording_settings') return; @@ -490,52 +325,39 @@ describe('OnboardingView', () => { expect(invoke).toHaveBeenCalledWith('quit_and_relaunch'); }); - // ─── Unmount cleanup ────────────────────────────────────────────────────── + it('shows screen recording step info', async () => { + setupPermissions(true); + render(); + await act(async () => {}); - it('does not emit console.error when unmounted during accessibility polling', async () => { - const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + expect(screen.getByText('Screen Recording')).toBeInTheDocument(); + }); + it('shows both steps regardless of current active step', async () => { setupPermissions(false); - const { unmount } = render(); + render(); await act(async () => {}); - await act(async () => { - fireEvent.click( - screen.getByRole('button', { name: /grant accessibility/i }), - ); - }); - - act(() => unmount()); - - await act(async () => { - await vi.advanceTimersByTimeAsync(1000); - }); - - expect(errorSpy).not.toHaveBeenCalled(); - errorSpy.mockRestore(); + expect(screen.getByText('Accessibility')).toBeInTheDocument(); + expect(screen.getByText('Screen Recording')).toBeInTheDocument(); }); - it('does not emit console.error when unmounted during input monitoring polling', async () => { + it('does not emit console.error when unmounted during accessibility polling', async () => { const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - invoke.mockImplementation(async (cmd: string) => { - if (cmd === 'check_accessibility_permission') return true; - if (cmd === 'check_input_monitoring_permission') return false; - if (cmd === 'request_input_monitoring_access') return; - if (cmd === 'open_input_monitoring_settings') return; - }); - + setupPermissions(false); const { unmount } = render(); await act(async () => {}); await act(async () => { fireEvent.click( - screen.getByRole('button', { name: /grant input monitoring/i }), + screen.getByRole('button', { name: /grant accessibility/i }), ); }); act(() => unmount()); + // Timer ticks after unmount must not trigger React state-update warnings. await act(async () => { await vi.advanceTimersByTimeAsync(1000); }); @@ -549,7 +371,6 @@ describe('OnboardingView', () => { invoke.mockImplementation(async (cmd: string) => { if (cmd === 'check_accessibility_permission') return true; - if (cmd === 'check_input_monitoring_permission') return true; if (cmd === 'check_screen_recording_permission') return false; if (cmd === 'request_screen_recording_access') return; if (cmd === 'open_screen_recording_settings') return; @@ -575,8 +396,6 @@ describe('OnboardingView', () => { errorSpy.mockRestore(); }); - // ─── CTAButton hover ─────────────────────────────────────────────────────── - it('hovering the CTA button applies brightness filter when enabled', async () => { setupPermissions(false); render(); @@ -584,6 +403,8 @@ describe('OnboardingView', () => { const btn = screen.getByRole('button', { name: /grant accessibility/i }); fireEvent.mouseEnter(btn); + // The button is not disabled so hovered=true applies brightness(1.1). + // Verify the element is still present and interactive (no errors thrown). expect(btn).toBeInTheDocument(); fireEvent.mouseLeave(btn); expect(btn).toBeInTheDocument(); @@ -600,10 +421,12 @@ describe('OnboardingView', () => { ); }); + // Button is now disabled/polling const btn = screen.getByRole('button', { name: /checking|grant accessibility/i, }); expect(btn).toBeDisabled(); + // mouseEnter on a disabled button must not toggle hovered state fireEvent.mouseEnter(btn); expect(btn).toBeDisabled(); fireEvent.mouseLeave(btn); @@ -611,9 +434,11 @@ describe('OnboardingView', () => { }); // ─── Defensive guard coverage ───────────────────────────────────────────── - // Tests below exercise the early-return branches that protect against stale - // state updates and concurrent invocations. Deferred promises keep invocations - // in-flight long enough to trigger each guard before resolving them. + // The following tests exercise the early-return branches that protect against + // stale state updates and concurrent invocations. These branches cannot be + // reached through the happy-path tests because the invoke mock resolves + // synchronously; here we use deferred promises to keep invocations in-flight + // long enough to trigger each guard. it('ignores initial accessibility check result when component unmounts mid-flight', async () => { const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); @@ -639,33 +464,6 @@ describe('OnboardingView', () => { errorSpy.mockRestore(); }); - it('ignores initial input monitoring check result when component unmounts mid-flight', async () => { - const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - let resolveIm!: (v: boolean) => void; - invoke.mockImplementation((cmd: string) => { - if (cmd === 'check_accessibility_permission') - return Promise.resolve(true); - if (cmd === 'check_input_monitoring_permission') - return new Promise((r) => { - resolveIm = r; - }); // hangs - return Promise.resolve(); - }); - - const { unmount } = render(); - // ax check resolves true; im check is now in-flight. - await act(async () => {}); - - act(() => unmount()); // mountedRef → false - - await act(async () => { - resolveIm(true); // then-handler fires; guard returns early - }); - - expect(errorSpy).not.toHaveBeenCalled(); - errorSpy.mockRestore(); - }); - it('ax in-flight guard prevents concurrent permission checks', async () => { let pollCallCount = 0; let resolveFirstPoll!: (v: boolean) => void; @@ -698,7 +496,7 @@ describe('OnboardingView', () => { }); // Only one poll call (initial was count=1, first poll was count=2; second - // tick was blocked, no count=3). + // tick was blocked — no count=3). expect(pollCallCount).toBe(2); await act(async () => { @@ -706,45 +504,6 @@ describe('OnboardingView', () => { }); }); - it('im in-flight guard prevents concurrent permission checks', async () => { - let imCallCount = 0; - let resolveFirstPoll!: (v: boolean) => void; - invoke.mockImplementation((cmd: string) => { - if (cmd === 'check_accessibility_permission') - return Promise.resolve(true); - if (cmd === 'check_input_monitoring_permission') { - imCallCount++; - if (imCallCount === 1) return Promise.resolve(false); // initial mount check - return new Promise((r) => { - resolveFirstPoll = r; - }); // poll hangs - } - if (cmd === 'request_input_monitoring_access') return Promise.resolve(); - if (cmd === 'open_input_monitoring_settings') return Promise.resolve(); - return Promise.resolve(); - }); - - render(); - await act(async () => {}); // ax + initial im check done - - await act(async () => { - fireEvent.click( - screen.getByRole('button', { name: /grant input monitoring/i }), - ); - }); - - act(() => { - vi.advanceTimersByTime(500); // first tick: in-flight - vi.advanceTimersByTime(500); // second tick: guard blocks it - }); - - expect(imCallCount).toBe(2); // initial check + one poll; second tick blocked - - await act(async () => { - resolveFirstPoll(false); - }); - }); - it('ignores ax poll result when component unmounts during in-flight check', async () => { const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); let callCount = 0; @@ -770,10 +529,14 @@ describe('OnboardingView', () => { ); }); + // Fire one tick so the poll invoke is in-flight (hanging). act(() => vi.advanceTimersByTime(500)); + // Unmount while the invoke is still pending; this clears the interval but + // the in-flight promise is still alive. act(() => unmount()); + // Resolving the promise must not trigger a React state update. await act(async () => { resolvePoll(true); }); @@ -782,90 +545,12 @@ describe('OnboardingView', () => { errorSpy.mockRestore(); }); - it('ignores im poll result when component unmounts during in-flight check', async () => { - const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - let imCallCount = 0; - let resolvePoll!: (v: boolean) => void; - invoke.mockImplementation((cmd: string) => { - if (cmd === 'check_accessibility_permission') - return Promise.resolve(true); - if (cmd === 'check_input_monitoring_permission') { - imCallCount++; - if (imCallCount === 1) return Promise.resolve(false); // initial mount check - return new Promise((r) => { - resolvePoll = r; - }); - } - if (cmd === 'request_input_monitoring_access') return Promise.resolve(); - if (cmd === 'open_input_monitoring_settings') return Promise.resolve(); - return Promise.resolve(); - }); - - const { unmount } = render(); - await act(async () => {}); // ax + initial im check done - - await act(async () => { - fireEvent.click( - screen.getByRole('button', { name: /grant input monitoring/i }), - ); - }); - - act(() => vi.advanceTimersByTime(500)); // poll fires, invoke hangs - - act(() => unmount()); // clears interval; in-flight promise still alive - - await act(async () => { - resolvePoll(true); - }); - - expect(errorSpy).not.toHaveBeenCalled(); - errorSpy.mockRestore(); - }); - - it('ignores input monitoring handler when component unmounts during open-settings call', async () => { - const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - let resolveOpen!: (v?: unknown) => void; - invoke.mockImplementation((cmd: string) => { - if (cmd === 'check_accessibility_permission') - return Promise.resolve(true); - if (cmd === 'check_input_monitoring_permission') - return Promise.resolve(false); - if (cmd === 'request_input_monitoring_access') return Promise.resolve(); - if (cmd === 'open_input_monitoring_settings') - return new Promise((r) => { - resolveOpen = r; - }); // hangs - return Promise.resolve(); - }); - - const { unmount } = render(); - await act(async () => {}); // ax granted; im check done (false) - - await act(async () => { - fireEvent.click( - screen.getByRole('button', { name: /grant input monitoring/i }), - ); - }); - // handler is suspended on open_input_monitoring_settings (resolveOpen set) - - act(() => unmount()); // mountedRef → false - - await act(async () => { - resolveOpen(); // mountedRef guard fires; returns early - }); - - expect(errorSpy).not.toHaveBeenCalled(); - errorSpy.mockRestore(); - }); - it('ignores screen recording handler when component unmounts during open-settings call', async () => { const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); let resolveOpen!: (v?: unknown) => void; invoke.mockImplementation((cmd: string) => { if (cmd === 'check_accessibility_permission') return Promise.resolve(true); - if (cmd === 'check_input_monitoring_permission') - return Promise.resolve(true); if (cmd === 'request_screen_recording_access') return Promise.resolve(); if (cmd === 'open_screen_recording_settings') return new Promise((r) => { @@ -877,8 +562,11 @@ describe('OnboardingView', () => { }); const { unmount } = render(); - await act(async () => {}); // ax + im both granted + await act(async () => {}); // accessibility granted + // Flush microtasks so the handler advances past the first await + // (request_screen_recording_access resolves) and suspends on the second + // (open_screen_recording_settings hangs), setting resolveOpen. await act(async () => { fireEvent.click( screen.getByRole('button', { name: /open screen recording settings/i }), @@ -888,7 +576,7 @@ describe('OnboardingView', () => { act(() => unmount()); // mountedRef → false await act(async () => { - resolveOpen(); // mountedRef guard fires; returns early + resolveOpen(); // mountedRef guard at line 225 fires; returns early }); expect(errorSpy).not.toHaveBeenCalled(); @@ -901,8 +589,6 @@ describe('OnboardingView', () => { invoke.mockImplementation((cmd: string) => { if (cmd === 'check_accessibility_permission') return Promise.resolve(true); - if (cmd === 'check_input_monitoring_permission') - return Promise.resolve(true); if (cmd === 'request_screen_recording_access') return Promise.resolve(); if (cmd === 'open_screen_recording_settings') return Promise.resolve(); if (cmd === 'check_screen_recording_tcc_granted') { @@ -941,8 +627,6 @@ describe('OnboardingView', () => { invoke.mockImplementation((cmd: string) => { if (cmd === 'check_accessibility_permission') return Promise.resolve(true); - if (cmd === 'check_input_monitoring_permission') - return Promise.resolve(true); if (cmd === 'request_screen_recording_access') return Promise.resolve(); if (cmd === 'open_screen_recording_settings') return Promise.resolve(); if (cmd === 'check_screen_recording_tcc_granted') @@ -966,7 +650,7 @@ describe('OnboardingView', () => { act(() => unmount()); // clears interval; in-flight promise still alive await act(async () => { - resolvePoll(true); + resolvePoll(true); // mountedRef guard at line 234 fires; returns early }); expect(errorSpy).not.toHaveBeenCalled(); diff --git a/src/view/onboarding/PermissionsStep.tsx b/src/view/onboarding/PermissionsStep.tsx index a71cbcd..406c780 100644 --- a/src/view/onboarding/PermissionsStep.tsx +++ b/src/view/onboarding/PermissionsStep.tsx @@ -8,7 +8,6 @@ import thukiLogo from '../../../src-tauri/icons/128x128.png'; const POLL_INTERVAL_MS = 500; type AccessibilityStatus = 'pending' | 'requesting' | 'granted'; -type InputMonitoringStatus = 'idle' | 'requesting' | 'polling' | 'granted'; type ScreenRecordingStatus = 'idle' | 'polling' | 'granted'; /** Inline macOS-style keyboard key chip for showing hotkey symbols. */ @@ -81,29 +80,7 @@ const KeyboardIcon = () => ( ); -/** Eye/monitoring icon for the Input Monitoring step. */ -const MonitoringIcon = ({ active }: { active: boolean }) => ( - -); - -/** Screen/camera icon for step 3. */ +/** Screen/camera icon for step 2. */ const ScreenIcon = ({ active }: { active: boolean }) => ( ( /** * Onboarding screen shown at first launch when required macOS permissions - * have not yet been granted. + * (Accessibility and Screen Recording) have not yet been granted. + * + * Follows a sequential flow: Accessibility first (polls until granted, + * no restart needed), then Screen Recording (registers app via + * CGRequestScreenCaptureAccess, polls TCC until granted, then prompts + * quit+reopen since macOS requires a restart for the permission to take effect). * - * Follows a sequential flow: - * 1. Accessibility: polls until granted, no restart needed - * 2. Input Monitoring: requests via IOHIDRequestAccess, polls until granted, - * no restart needed (CGEventTap begins receiving cross-app events immediately) - * 3. Screen Recording: registers app via CGRequestScreenCaptureAccess, polls - * TCC until granted, then prompts quit+reopen (macOS requires restart) + * Visual direction: Warm Ambient — dark base with a warm orange radial glow. + * The outer container is transparent so the rounded panel corners are visible + * against the macOS desktop. */ export function PermissionsStep() { const [accessibilityStatus, setAccessibilityStatus] = useState('pending'); - const [inputMonitoringStatus, setInputMonitoringStatus] = - useState('idle'); const [screenRecordingStatus, setScreenRecordingStatus] = useState('idle'); - const axPollRef = useRef | null>(null); - const imPollRef = useRef | null>(null); const screenPollRef = useRef | null>(null); - // Guards that prevent a new poll tick from firing while a previous invoke // call is still in-flight. Without these, a slow IPC response (> POLL_INTERVAL_MS) // could queue multiple concurrent permission checks. const axInFlightRef = useRef(false); - const imInFlightRef = useRef(false); const screenInFlightRef = useRef(false); - // Prevents state updates from resolving in-flight invocations after unmount. const mountedRef = useRef(true); @@ -200,13 +172,6 @@ export function PermissionsStep() { } }, []); - const stopImPolling = useCallback(() => { - if (imPollRef.current !== null) { - clearInterval(imPollRef.current); - imPollRef.current = null; - } - }, []); - const stopScreenPolling = useCallback(() => { if (screenPollRef.current !== null) { clearInterval(screenPollRef.current); @@ -215,7 +180,7 @@ export function PermissionsStep() { }, []); // On mount: check whether Accessibility is already granted so we can skip - // step 1, and if so check Input Monitoring so we can skip step 2. + // step 1 and show step 2 immediately. useEffect(() => { // Reset on every mount so that a remount after unmount gets a fresh guard. mountedRef.current = true; @@ -223,23 +188,14 @@ export function PermissionsStep() { if (!mountedRef.current) return; if (granted) { setAccessibilityStatus('granted'); - void invoke('check_input_monitoring_permission').then( - (imGranted) => { - if (!mountedRef.current) return; - if (imGranted) { - setInputMonitoringStatus('granted'); - } - }, - ); } }); return () => { mountedRef.current = false; stopAxPolling(); - stopImPolling(); stopScreenPolling(); }; - }, [stopAxPolling, stopImPolling, stopScreenPolling]); + }, [stopAxPolling, stopScreenPolling]); const handleGrantAccessibility = useCallback(async () => { setAccessibilityStatus('requesting'); @@ -260,32 +216,6 @@ export function PermissionsStep() { }, POLL_INTERVAL_MS); }, [stopAxPolling]); - const handleGrantInputMonitoring = useCallback(async () => { - setInputMonitoringStatus('requesting'); - // Register Thuki in TCC and trigger the system permission dialog. - await invoke('request_input_monitoring_access'); - // Also open System Settings in case the user previously denied the dialog. - await invoke('open_input_monitoring_settings'); - if (!mountedRef.current) return; - setInputMonitoringStatus('polling'); - imPollRef.current = setInterval(async () => { - if (imInFlightRef.current) return; - imInFlightRef.current = true; - try { - const granted = await invoke( - 'check_input_monitoring_permission', - ); - if (!mountedRef.current) return; - if (granted) { - stopImPolling(); - setInputMonitoringStatus('granted'); - } - } finally { - imInFlightRef.current = false; - } - }, POLL_INTERVAL_MS); - }, [stopImPolling]); - const handleOpenScreenRecording = useCallback(async () => { // Register Thuki in TCC (adds it to the Screen Recording list) then open // System Settings directly so the user can toggle it on without hunting. @@ -318,10 +248,6 @@ export function PermissionsStep() { const accessibilityGranted = accessibilityStatus === 'granted'; const isAxRequesting = accessibilityStatus === 'requesting'; - const imGranted = inputMonitoringStatus === 'granted'; - const isImRequesting = - inputMonitoringStatus === 'requesting' || - inputMonitoringStatus === 'polling'; const isScreenPolling = screenRecordingStatus === 'polling'; const screenGranted = screenRecordingStatus === 'granted'; @@ -369,7 +295,7 @@ export function PermissionsStep() { }} /> - {/* Logo mark + title, drag region so the user can reposition the + {/* Logo mark + title — drag region so the user can reposition the onboarding window when it overlaps System Settings. */}
- {/* Step 2: Input Monitoring */} - + {/* Step 2: Screen Recording */} +
- {imGranted ? ( - - ) : ( - - )} +
- Input Monitoring -
-
- Required for hotkey to work when another app is focused -
-
- {imGranted && ( -
- Granted -
- )} - - - {/* Step 3: Screen Recording */} - -
- -
-
-
Screen Recording
@@ -561,22 +432,8 @@ export function PermissionsStep() { )} - {/* Step 2 CTA: Grant Input Monitoring */} - {accessibilityGranted && !imGranted && ( - - {isImRequesting ? 'Checking...' : 'Grant Input Monitoring Access'} - - )} - - {/* Step 3 CTAs: Open Settings (with polling) + Quit & Reopen */} - {accessibilityGranted && imGranted && ( + {/* Step 2 CTAs: Open Settings (with polling) + Quit & Reopen */} + {accessibilityGranted && ( <> {!screenGranted && ( Date: Wed, 8 Apr 2026 13:43:17 -0500 Subject: [PATCH 2/3] fix: defer CGEventTap creation until Accessibility is granted Gate activator.start() behind is_accessibility_granted() so the HID-level event tap is only created when permission already exists. This prevents macOS from showing the native Accessibility popup during onboarding; the activator starts cleanly on the post-Screen Recording quit+reopen when both permissions are guaranteed. Co-Authored-By: Claude Opus 4.6 Signed-off-by: Logan Nguyen --- src-tauri/src/lib.rs | 43 +++++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index fe3a27c..5e5f3c6 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -672,29 +672,36 @@ pub fn run() { .build(app)?; // ── Activation listener (macOS only) ───────────────────────── + // Only start the event tap when Accessibility is already granted. + // Creating a CGEventTap without permission triggers a native macOS + // popup; deferring until after onboarding (and the quit+reopen for + // Screen Recording) avoids that redundant dialog entirely. #[cfg(target_os = "macos")] { let app_handle = app.handle().clone(); let activator = activator::OverlayActivator::new(); - activator.start(move || { - // Skip AX + clipboard when hiding — no context needed and - // simulating Cmd+C against Thuki's own WebView would produce - // a macOS alert sound. - let is_visible = OVERLAY_INTENDED_VISIBLE.load(Ordering::SeqCst); - let handle = app_handle.clone(); - let handle2 = app_handle.clone(); - // Dispatch context capture to a dedicated thread so the event - // tap callback returns immediately. AX attribute lookups and - // clipboard simulation can block for seconds (macOS AX default - // timeout is ~6 s) when the focused app does not implement the - // accessibility protocol. Blocking the tap callback freezes the - // CFRunLoop and silently prevents all future key events from - // being delivered to the activator. - std::thread::spawn(move || { - let ctx = crate::context::capture_activation_context(is_visible); - let _ = handle.run_on_main_thread(move || toggle_overlay(&handle2, ctx)); + if permissions::is_accessibility_granted() { + activator.start(move || { + // Skip AX + clipboard when hiding — no context needed and + // simulating Cmd+C against Thuki's own WebView would produce + // a macOS alert sound. + let is_visible = OVERLAY_INTENDED_VISIBLE.load(Ordering::SeqCst); + let handle = app_handle.clone(); + let handle2 = app_handle.clone(); + // Dispatch context capture to a dedicated thread so the event + // tap callback returns immediately. AX attribute lookups and + // clipboard simulation can block for seconds (macOS AX default + // timeout is ~6 s) when the focused app does not implement the + // accessibility protocol. Blocking the tap callback freezes the + // CFRunLoop and silently prevents all future key events from + // being delivered to the activator. + std::thread::spawn(move || { + let ctx = crate::context::capture_activation_context(is_visible); + let _ = + handle.run_on_main_thread(move || toggle_overlay(&handle2, ctx)); + }); }); - }); + } app.manage(activator); } From e8d32e8a8d850527df741be496eb7a9ba3badfb4 Mon Sep 17 00:00:00 2001 From: Logan Nguyen Date: Wed, 8 Apr 2026 13:53:01 -0500 Subject: [PATCH 3/3] style: replace em dashes with appropriate punctuation Co-Authored-By: Claude Opus 4.6 Signed-off-by: Logan Nguyen --- src/view/onboarding/PermissionsStep.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/view/onboarding/PermissionsStep.tsx b/src/view/onboarding/PermissionsStep.tsx index 406c780..e8cd5f4 100644 --- a/src/view/onboarding/PermissionsStep.tsx +++ b/src/view/onboarding/PermissionsStep.tsx @@ -146,7 +146,7 @@ const Spinner = () => ( * CGRequestScreenCaptureAccess, polls TCC until granted, then prompts * quit+reopen since macOS requires a restart for the permission to take effect). * - * Visual direction: Warm Ambient — dark base with a warm orange radial glow. + * Visual direction: Warm Ambient: dark base with a warm orange radial glow. * The outer container is transparent so the rounded panel corners are visible * against the macOS desktop. */ @@ -295,7 +295,7 @@ export function PermissionsStep() { }} /> - {/* Logo mark + title — drag region so the user can reposition the + {/* Logo mark + title, drag region so the user can reposition the onboarding window when it overlaps System Settings. */}