diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index cb924e7..1854347 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -1122,6 +1122,16 @@ pub fn run() { ); if did_upgrade { updater::tcc_reset::tccutil_reset(&app.config().identifier); + // Persist that the running version's csreq is what + // owns any TCC entries on disk now (or there are no + // entries, which is also fine). The click-time grant + // flow consults this so the user's first grant click + // after an upgrade does not trigger a second + // reset+relaunch on top of the one we are about to + // schedule below. Held in the sidecar (not memory) + // because the relaunch wipes any in-process state + // before the user could ever click. + sidecar.last_reset_for_version = Some(running_version.clone()); } // Restore persisted snooze flags into the live state. @@ -1135,6 +1145,9 @@ pub fn run() { // snooze. updater_state .set_last_seen_update_version(sidecar.last_seen_update_version.clone()); + // Mirror the on-disk reset marker so click-time decisions + // don't have to re-read the sidecar. + updater_state.set_last_reset_for_version(sidecar.last_reset_for_version.clone()); // Record the running version BEFORE any potential restart // so the post-restart launch reads a sidecar where the @@ -1339,7 +1352,11 @@ pub fn run() { #[cfg(not(coverage))] updater::commands::snooze_update_chat, #[cfg(not(coverage))] - updater::commands::snooze_update_settings + updater::commands::snooze_update_settings, + #[cfg(not(coverage))] + updater::commands::reset_and_relaunch_for_grant, + #[cfg(not(coverage))] + updater::commands::consume_pending_grant_resume ]) .build(tauri::generate_context!()) .expect("error while building tauri application") diff --git a/src-tauri/src/updater/commands.rs b/src-tauri/src/updater/commands.rs index edc1063..5c26e37 100644 --- a/src-tauri/src/updater/commands.rs +++ b/src-tauri/src/updater/commands.rs @@ -1,6 +1,7 @@ use crate::config::defaults::{DEFAULT_UPDATER_STATE_FILENAME, MAX_UPDATER_SNOOZE_HOURS}; use crate::updater::poller; -use crate::updater::state::{UpdaterSnapshot, UpdaterState}; +use crate::updater::state::{SnoozeSidecar, UpdaterSnapshot, UpdaterState}; +use crate::updater::tcc_reset; use std::path::PathBuf; use std::time::{SystemTime, UNIX_EPOCH}; use tauri::{AppHandle, Manager, State}; @@ -107,6 +108,121 @@ fn sidecar_path(app: &AppHandle) -> Result { Ok(dir.join(DEFAULT_UPDATER_STATE_FILENAME)) } +/// Stores `service` on `sidecar` so the post-restart launch can resume the +/// grant flow. Returns `Err` when the service string is not one of the +/// values Thuki resets at click time, so callers cannot smuggle arbitrary +/// strings into a later `tccutil reset` invocation. +pub fn prepare_pending_reregister( + sidecar: &mut SnoozeSidecar, + service: &str, +) -> Result<&'static str, String> { + let canonical = tcc_reset::validate_click_time_service(service) + .ok_or_else(|| format!("unsupported tcc service: {service}"))?; + sidecar.pending_reregister = Some(canonical.to_string()); + Ok(canonical) +} + +/// Removes `pending_reregister` from `sidecar` and returns its previous +/// value. The caller is responsible for persisting the cleared sidecar so +/// the resume flow does not loop on the next restart. +pub fn take_pending_reregister(sidecar: &mut SnoozeSidecar) -> Option { + sidecar.pending_reregister.take() +} + +/// Pure decision helper. Returns `true` when the click-time reset can be +/// skipped because the startup path already cleared TCC for the running +/// version. The marker survives A's reset+restart by being persisted to +/// the sidecar, which is why the comparison is meaningful even though +/// `was_reset_at_startup` would always be `false` on a freshly relaunched +/// process. +pub fn click_time_reset_can_skip( + last_reset_for_version: Option<&str>, + running_version: &str, +) -> bool { + last_reset_for_version == Some(running_version) +} + +/// Click-time grant flow: persist a "resume after restart" marker, clear +/// the stale TCC entry for the requested service, and relaunch. The +/// frontend hands the service string straight in, so the validator inside +/// `prepare_pending_reregister` is the trust boundary. +/// +/// Returns `true` when a relaunch has been scheduled and `false` when the +/// running process already has a clean TCC slate (the startup path's most +/// recent reset matches the running version). In the `false` case the +/// frontend should run the in-line open-Settings + polling flow without +/// expecting a relaunch. +/// +/// Sequencing matters when a relaunch is scheduled. Sidecar must be saved +/// BEFORE `tccutil reset` runs so a crash between the two does not leave +/// the user with a cleared grant and no resume marker. The restart is +/// deferred so Tauri can finish dispatching the IPC reply (otherwise the +/// frontend sees a disconnect error rather than a clean relaunch). +#[cfg_attr(coverage_nightly, coverage(off))] +#[tauri::command] +pub fn reset_and_relaunch_for_grant( + app: AppHandle, + state: State<'_, UpdaterState>, + service: String, +) -> Result { + // Validate first so a hostile string never reaches `tccutil` even when + // the startup-clean path skips the reset. + let canonical = tcc_reset::validate_click_time_service(&service) + .ok_or_else(|| format!("unsupported tcc service: {service}"))?; + + let running = app.package_info().version.to_string(); + let snooze = state.snooze_clone(); + if click_time_reset_can_skip(snooze.last_reset_for_version.as_deref(), &running) { + // Startup path already reset TCC for this exact version, so the + // running binary's csreq already owns whatever TCC entries (if + // any) System Settings will display. A second reset+relaunch + // would only add a jarring quit on every grant click. + return Ok(false); + } + + let mut snooze = snooze; + prepare_pending_reregister(&mut snooze, canonical)?; + + let path = sidecar_path(&app)?; + snooze.save(&path).map_err(|e| e.to_string())?; + state.set_pending_reregister(Some(canonical.to_string())); + + let bundle_id = app.config().identifier.clone(); + tcc_reset::tccutil_reset_service(&bundle_id, canonical); + + let app_handle = app.clone(); + tauri::async_runtime::spawn(async move { + tokio::time::sleep(std::time::Duration::from_millis(150)).await; + eprintln!( + "thuki: [updater] relaunching after click-time TCC reset \ + to refresh tccd PID tracking" + ); + app_handle.restart(); + }); + + Ok(true) +} + +/// Frontend-facing companion to `reset_and_relaunch_for_grant`. Reads the +/// `pending_reregister` flag, clears it (in memory and on disk), and +/// returns the value so PermissionsStep can resume the right step on a +/// fresh launch without forcing the user to click a second time. +#[cfg_attr(coverage_nightly, coverage(off))] +#[tauri::command] +pub fn consume_pending_grant_resume( + app: AppHandle, + state: State<'_, UpdaterState>, +) -> Result, String> { + let mut snooze = state.snooze_clone(); + let value = take_pending_reregister(&mut snooze); + if value.is_some() { + let path = sidecar_path(&app)?; + snooze.save(&path).map_err(|e| e.to_string())?; + state.set_pending_reregister(None); + } + Ok(value) +} + #[cfg(test)] mod tests { use super::*; @@ -144,4 +260,66 @@ mod tests { fn snooze_deadline_zero_hours_is_now() { assert_eq!(snooze_deadline(1_700_000_000, 0), 1_700_000_000); } + + #[test] + fn prepare_pending_reregister_accepts_accessibility() { + let mut sidecar = SnoozeSidecar::default(); + let canonical = prepare_pending_reregister(&mut sidecar, "Accessibility").unwrap(); + assert_eq!(canonical, "Accessibility"); + assert_eq!(sidecar.pending_reregister.as_deref(), Some("Accessibility")); + } + + #[test] + fn prepare_pending_reregister_accepts_screen_capture() { + let mut sidecar = SnoozeSidecar::default(); + let canonical = prepare_pending_reregister(&mut sidecar, "ScreenCapture").unwrap(); + assert_eq!(canonical, "ScreenCapture"); + assert_eq!(sidecar.pending_reregister.as_deref(), Some("ScreenCapture")); + } + + #[test] + fn prepare_pending_reregister_rejects_unsupported_service() { + let mut sidecar = SnoozeSidecar::default(); + let err = prepare_pending_reregister(&mut sidecar, "Camera").unwrap_err(); + assert!(err.contains("Camera"), "error must surface offending value"); + // Sidecar must remain untouched on rejection so a hostile call + // cannot pollute the persisted resume marker. + assert!(sidecar.pending_reregister.is_none()); + } + + #[test] + fn take_pending_reregister_returns_and_clears_value() { + let mut sidecar = SnoozeSidecar { + pending_reregister: Some("Accessibility".to_string()), + ..SnoozeSidecar::default() + }; + assert_eq!( + take_pending_reregister(&mut sidecar), + Some("Accessibility".to_string()), + ); + assert!(sidecar.pending_reregister.is_none()); + } + + #[test] + fn take_pending_reregister_returns_none_when_unset() { + let mut sidecar = SnoozeSidecar::default(); + assert!(take_pending_reregister(&mut sidecar).is_none()); + } + + #[test] + fn click_time_reset_can_skip_when_versions_match() { + assert!(click_time_reset_can_skip(Some("0.8.5"), "0.8.5")); + } + + #[test] + fn click_time_reset_does_not_skip_when_versions_differ() { + assert!(!click_time_reset_can_skip(Some("0.8.4"), "0.8.5")); + } + + #[test] + fn click_time_reset_does_not_skip_when_marker_is_absent() { + // No prior startup reset for this binary recorded: a stale csreq + // grant could still be on disk, so the click MUST clean it up. + assert!(!click_time_reset_can_skip(None, "0.8.5")); + } } diff --git a/src-tauri/src/updater/state.rs b/src-tauri/src/updater/state.rs index fda3b3f..00fb713 100644 --- a/src-tauri/src/updater/state.rs +++ b/src-tauri/src/updater/state.rs @@ -32,6 +32,22 @@ pub struct SnoozeSidecar { /// just in memory) so the comparison survives an app restart. #[serde(default)] pub last_seen_update_version: Option, + /// TCC service the user just clicked "Grant" for, persisted across the + /// reset+relaunch cycle so the post-restart launch knows to resume the + /// grant flow (open System Settings, trigger a new csreq registration) + /// instead of waiting for another click. Cleared on consumption. + #[serde(default)] + pub pending_reregister: Option, + /// SemVer string of the binary the startup path most recently ran a + /// `tccutil reset` for. The click-time grant flow consults this to + /// decide whether ANOTHER reset+relaunch is necessary: if it matches + /// the running version, the running binary's csreq already owns the + /// only TCC entries on disk (or there are none), so a click does not + /// need a redundant restart. Persisted (rather than held in memory) + /// because A's reset+restart drops any in-process flag before the + /// user can ever click. + #[serde(default)] + pub last_reset_for_version: Option, } impl SnoozeSidecar { @@ -135,6 +151,14 @@ impl UpdaterState { inner.snooze.last_seen_update_version = version; } + /// Mirror the on-disk `pending_reregister` field into the live state + /// so a crash before the next sidecar save still leaves the in-memory + /// view consistent with what was persisted. + pub fn set_pending_reregister(&self, value: Option) { + let mut inner = self.inner.lock().expect("updater state mutex"); + inner.snooze.pending_reregister = value; + } + pub fn snooze_clone(&self) -> SnoozeSidecar { self.inner .lock() @@ -142,6 +166,16 @@ impl UpdaterState { .snooze .clone() } + + /// Mirrors the persisted `last_reset_for_version` from the sidecar + /// into live state. Called both at startup (when seeding from disk) + /// and immediately after a successful startup-time `tccutil reset` + /// (so the click-time grant flow sees the new value without re-reading + /// the sidecar). + pub fn set_last_reset_for_version(&self, version: Option) { + let mut inner = self.inner.lock().expect("updater state mutex"); + inner.snooze.last_reset_for_version = version; + } } #[derive(Debug, Clone, Serialize)] @@ -179,6 +213,8 @@ mod tests { chat_snoozed_until: Some(1_700_001_000), last_launched_version: None, last_seen_update_version: None, + pending_reregister: None, + last_reset_for_version: Some("0.8.5".to_string()), }; original.save(&path).unwrap(); @@ -205,6 +241,27 @@ mod tests { chat_snoozed_until: None, last_launched_version: Some("0.8.1".to_string()), last_seen_update_version: None, + pending_reregister: None, + last_reset_for_version: None, + }; + original.save(&path).unwrap(); + + let loaded = SnoozeSidecar::load(&path).unwrap(); + assert_eq!(loaded, original); + } + + #[test] + fn snooze_sidecar_round_trips_pending_reregister() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("updater_state.json"); + + let original = SnoozeSidecar { + settings_snoozed_until: None, + chat_snoozed_until: None, + last_launched_version: Some("0.8.5".to_string()), + last_seen_update_version: None, + pending_reregister: Some("Accessibility".to_string()), + last_reset_for_version: Some("0.8.5".to_string()), }; original.save(&path).unwrap(); @@ -212,6 +269,25 @@ mod tests { assert_eq!(loaded, original); } + #[test] + fn snooze_sidecar_back_compat_old_file_without_pending_reregister() { + // Sidecars written before the click-time grant flow shipped lack + // the field. Loading must default it to None so existing snooze / + // version state is preserved across the upgrade. + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("updater_state.json"); + std::fs::write( + &path, + r#"{"settings_snoozed_until":null,"chat_snoozed_until":null, + "last_launched_version":"0.8.4","last_seen_update_version":null}"#, + ) + .unwrap(); + + let loaded = SnoozeSidecar::load(&path).unwrap(); + assert_eq!(loaded.last_launched_version.as_deref(), Some("0.8.4")); + assert!(loaded.pending_reregister.is_none()); + } + #[test] fn snooze_sidecar_back_compat_old_file_without_version_field() { // Old (pre-0.8.2) sidecar files were written without the @@ -406,6 +482,30 @@ mod tests { assert_eq!(snap.settings_snoozed_until, Some(2)); } + #[test] + fn set_last_reset_for_version_round_trips_through_snooze_clone() { + let state = UpdaterState::default(); + state.set_last_reset_for_version(Some("0.8.5".to_string())); + assert_eq!( + state.snooze_clone().last_reset_for_version.as_deref(), + Some("0.8.5"), + ); + state.set_last_reset_for_version(None); + assert!(state.snooze_clone().last_reset_for_version.is_none()); + } + + #[test] + fn set_pending_reregister_round_trips_through_snooze_clone() { + let state = UpdaterState::default(); + state.set_pending_reregister(Some("Accessibility".to_string())); + assert_eq!( + state.snooze_clone().pending_reregister.as_deref(), + Some("Accessibility"), + ); + state.set_pending_reregister(None); + assert!(state.snooze_clone().pending_reregister.is_none()); + } + #[test] fn system_time_to_unix_returns_some_for_now() { let now = SystemTime::now(); diff --git a/src-tauri/src/updater/tcc_reset.rs b/src-tauri/src/updater/tcc_reset.rs index 1f3fe25..735b37e 100644 --- a/src-tauri/src/updater/tcc_reset.rs +++ b/src-tauri/src/updater/tcc_reset.rs @@ -30,48 +30,86 @@ use std::process::Command; /// `ScreenCapture` powers the `/screen` command. const SERVICES: &[&str] = &["Accessibility", "ScreenCapture"]; -/// Pure decision function. Returns `true` when the recorded version -/// differs from the running version, indicating an upgrade just happened. -/// Returns `false` when: -/// - The sidecar has no recorded version (first ever launch; nothing to -/// reset because no prior binary ever held grants). -/// - The recorded version equals the running version (normal launch). +/// Subset of `SERVICES` accepted by the click-time reset command. Held as +/// a separate const so any addition to the runtime services list is an +/// explicit, reviewed change to the click-time API surface. +const CLICK_TIME_SERVICES: &[&str] = &["Accessibility", "ScreenCapture"]; + +/// Returns the canonical TCC service string when `service` is one Thuki +/// is willing to reset on demand, or `None` otherwise. The frontend hands +/// the string straight to a Tauri command so this validator is the trust +/// boundary that prevents `tccutil reset ` from +/// shelling out with caller-controlled args. +pub fn validate_click_time_service(service: &str) -> Option<&'static str> { + CLICK_TIME_SERVICES + .iter() + .copied() + .find(|allowed| *allowed == service) +} + +/// Pure decision function. Returns `true` when the running binary may not +/// match the code requirement of any TCC entry currently on disk. +/// +/// - Recorded version differs from running version: upgrade just happened. +/// - No recorded version: either a truly first-ever launch (no prior grant +/// to clear, so the reset is a harmless no-op) OR an upgrade from a +/// pre-0.8.2 build that never wrote the sidecar (and almost certainly +/// has stale grants whose csreq no longer matches). Treating both the +/// same costs one extra restart on first install but is the only way to +/// migrate users coming from pre-sidecar builds without leaving them +/// stuck with a stale "granted" toggle. +/// - Recorded equals running: normal subsequent launch, nothing to do. pub fn should_reset_for_upgrade(recorded: Option<&str>, running: &str) -> bool { match recorded { Some(prev) => prev != running, - None => false, + None => true, + } +} + +/// Shells out to `/usr/bin/tccutil reset `. Logs +/// failures but never propagates them: TCC reset is a UX nicety, not a +/// correctness requirement. +#[cfg_attr(coverage_nightly, coverage(off))] +fn tccutil_reset_one(bundle_id: &str, service: &str) { + let result = Command::new("/usr/bin/tccutil") + .args(["reset", service, bundle_id]) + .status(); + match result { + Ok(status) if status.success() => { + eprintln!("thuki: [updater] cleared stale TCC grant for {service} ({bundle_id})"); + } + Ok(status) => { + eprintln!( + "thuki: [updater] tccutil reset {service} exited with {status}; \ + leaving any existing grant in place" + ); + } + Err(e) => { + eprintln!( + "thuki: [updater] tccutil invocation failed: {e}; \ + leaving any existing grant in place" + ); + } } } -/// Shells out to `/usr/bin/tccutil reset ` for each -/// TCC service Thuki uses. Logs failures but never propagates them: TCC -/// reset is a UX nicety, not a correctness requirement. +/// Resets every TCC service Thuki uses for `bundle_id`. Called from the +/// startup upgrade path so a relaunched binary starts with a clean slate. #[cfg_attr(coverage_nightly, coverage(off))] pub fn tccutil_reset(bundle_id: &str) { for service in SERVICES { - let result = Command::new("/usr/bin/tccutil") - .args(["reset", service, bundle_id]) - .status(); - match result { - Ok(status) if status.success() => { - eprintln!("thuki: [updater] cleared stale TCC grant for {service} ({bundle_id})"); - } - Ok(status) => { - eprintln!( - "thuki: [updater] tccutil reset {service} exited with {status}; \ - leaving any existing grant in place" - ); - } - Err(e) => { - eprintln!( - "thuki: [updater] tccutil invocation failed: {e}; \ - leaving any existing grant in place" - ); - } - } + tccutil_reset_one(bundle_id, service); } } +/// Resets a single TCC service for `bundle_id`. Called from the click-time +/// reset flow so the user only blows away the grant for the permission +/// they are actively re-requesting. +#[cfg_attr(coverage_nightly, coverage(off))] +pub fn tccutil_reset_service(bundle_id: &str, service: &str) { + tccutil_reset_one(bundle_id, service); +} + #[cfg(test)] mod tests { use super::*; @@ -87,15 +125,45 @@ mod tests { } #[test] - fn no_reset_on_first_ever_launch_when_recorded_is_absent() { - // First launch: nothing recorded, nothing to invalidate. - assert!(!should_reset_for_upgrade(None, "0.8.1")); + fn reset_when_recorded_is_absent() { + // No sidecar version: either first-ever launch (reset is a no-op) + // or an upgrade from a pre-0.8.2 build whose stale csreq must be + // cleared. Both cases must reset so pre-sidecar users are not + // stranded with a stale "granted" toggle. + assert!(should_reset_for_upgrade(None, "0.8.1")); } #[test] fn reset_when_recorded_version_is_higher_than_running() { - // Downgrade still counts as a binary swap — the csreq differs in + // Downgrade still counts as a binary swap. The csreq differs in // either direction, so the stale grant must be cleared. assert!(should_reset_for_upgrade(Some("0.9.0"), "0.8.1")); } + + #[test] + fn validate_click_time_service_accepts_accessibility() { + assert_eq!( + validate_click_time_service("Accessibility"), + Some("Accessibility"), + ); + } + + #[test] + fn validate_click_time_service_accepts_screen_capture() { + assert_eq!( + validate_click_time_service("ScreenCapture"), + Some("ScreenCapture"), + ); + } + + #[test] + fn validate_click_time_service_rejects_arbitrary_strings() { + // Trust boundary check: anything not in the allow-list must be + // rejected so the Tauri command does not shell out with + // caller-controlled service args. + assert!(validate_click_time_service("").is_none()); + assert!(validate_click_time_service("Camera").is_none()); + assert!(validate_click_time_service("accessibility").is_none()); + assert!(validate_click_time_service("Accessibility; rm -rf /").is_none()); + } } diff --git a/src/__tests__/OnboardingView.test.tsx b/src/__tests__/OnboardingView.test.tsx index 6d5dd42..e54d28d 100644 --- a/src/__tests__/OnboardingView.test.tsx +++ b/src/__tests__/OnboardingView.test.tsx @@ -15,11 +15,14 @@ describe('OnboardingView', () => { function setupPermissions(accessibility: boolean, screenRecording = false) { invoke.mockImplementation(async (cmd: string) => { + if (cmd === 'consume_pending_grant_resume') return null; + if (cmd === 'reset_and_relaunch_for_grant') return false; if (cmd === 'check_accessibility_permission') return accessibility; 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 === 'open_accessibility_settings') return; }); } @@ -66,7 +69,95 @@ describe('OnboardingView', () => { ); }); + expect(invoke).toHaveBeenCalledWith('reset_and_relaunch_for_grant', { + service: 'Accessibility', + }); + expect(invoke).toHaveBeenCalledWith('open_accessibility_settings'); + }); + + it('clicking grant accessibility skips inline flow when backend signals relaunch', async () => { + invoke.mockImplementation(async (cmd: string) => { + if (cmd === 'consume_pending_grant_resume') return null; + if (cmd === 'reset_and_relaunch_for_grant') return true; + if (cmd === 'check_accessibility_permission') return false; + if (cmd === 'check_screen_recording_permission') return false; + if (cmd === 'open_accessibility_settings') return; + }); + + render(); + await act(async () => {}); + + await act(async () => { + fireEvent.click( + screen.getByRole('button', { name: /grant accessibility/i }), + ); + }); + + // Backend reports a relaunch is in flight, so the frontend must not + // open System Settings or start polling: the relaunched process owns + // both responsibilities via the consume_pending_grant_resume marker. + expect(invoke).not.toHaveBeenCalledWith('open_accessibility_settings'); + }); + + it('auto-resumes the accessibility flow when consume returns Accessibility', async () => { + invoke.mockImplementation(async (cmd: string) => { + if (cmd === 'consume_pending_grant_resume') return 'Accessibility'; + if (cmd === 'reset_and_relaunch_for_grant') return false; + if (cmd === 'check_accessibility_permission') return false; + if (cmd === 'check_screen_recording_permission') return false; + if (cmd === 'open_accessibility_settings') return; + }); + + render(); + // Drain the two sequential awaits inside the mount IIFE. + await act(async () => {}); + await act(async () => {}); + + expect(invoke).toHaveBeenCalledWith('consume_pending_grant_resume'); expect(invoke).toHaveBeenCalledWith('open_accessibility_settings'); + // Click button shows "Checking..." because the resume kicked the flow + // into the requesting state without a click. + expect( + screen.getByRole('button', { name: /checking/i }), + ).toBeInTheDocument(); + }); + + it('auto-resumes the screen recording flow when consume returns ScreenCapture', async () => { + invoke.mockImplementation(async (cmd: string) => { + if (cmd === 'consume_pending_grant_resume') return 'ScreenCapture'; + if (cmd === 'reset_and_relaunch_for_grant') return false; + if (cmd === 'check_accessibility_permission') return true; + if (cmd === 'check_screen_recording_permission') return false; + if (cmd === 'check_screen_recording_tcc_granted') return false; + if (cmd === 'request_screen_recording_access') return; + if (cmd === 'open_screen_recording_settings') return; + }); + + render(); + await act(async () => {}); + await act(async () => {}); + + expect(invoke).toHaveBeenCalledWith('request_screen_recording_access'); + expect(invoke).toHaveBeenCalledWith('open_screen_recording_settings'); + }); + + it('does not auto-resume screen recording when accessibility is not yet granted', async () => { + invoke.mockImplementation(async (cmd: string) => { + if (cmd === 'consume_pending_grant_resume') return 'ScreenCapture'; + if (cmd === 'reset_and_relaunch_for_grant') return false; + if (cmd === 'check_accessibility_permission') return false; + if (cmd === 'check_screen_recording_permission') return false; + if (cmd === 'request_screen_recording_access') return; + if (cmd === 'open_screen_recording_settings') return; + }); + + render(); + await act(async () => {}); + await act(async () => {}); + + // ScreenCapture resume only kicks in when AX is already granted; here it + // must NOT have triggered the request_screen_recording_access path. + expect(invoke).not.toHaveBeenCalledWith('request_screen_recording_access'); }); it('shows spinner while polling after grant request', async () => { @@ -194,11 +285,39 @@ describe('OnboardingView', () => { ); }); - // Registers Thuki in TCC (so it appears in the list) then opens Settings + // First clears any stale ScreenCapture grant left from a previous + // binary, then registers Thuki in TCC + opens Settings. + expect(invoke).toHaveBeenCalledWith('reset_and_relaunch_for_grant', { + service: 'ScreenCapture', + }); expect(invoke).toHaveBeenCalledWith('request_screen_recording_access'); expect(invoke).toHaveBeenCalledWith('open_screen_recording_settings'); }); + it('clicking screen recording skips inline flow when backend signals relaunch', async () => { + invoke.mockImplementation(async (cmd: string) => { + if (cmd === 'consume_pending_grant_resume') return null; + if (cmd === 'reset_and_relaunch_for_grant') return true; + if (cmd === 'check_accessibility_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; + }); + + render(); + await act(async () => {}); + + await act(async () => { + fireEvent.click( + screen.getByRole('button', { name: /open screen recording settings/i }), + ); + }); + + // Relaunch is in flight; inline flow must not register/open settings. + expect(invoke).not.toHaveBeenCalledWith('request_screen_recording_access'); + expect(invoke).not.toHaveBeenCalledWith('open_screen_recording_settings'); + }); + it('shows spinner while polling after opening screen recording settings', async () => { setupPermissions(true); render(); @@ -440,10 +559,36 @@ describe('OnboardingView', () => { // synchronously; here we use deferred promises to keep invocations in-flight // long enough to trigger each guard. + it('ignores resume marker when component unmounts before mount-effect resolves', async () => { + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + let resolveResume!: (v: string | null) => void; + invoke.mockImplementation((cmd: string) => { + if (cmd === 'consume_pending_grant_resume') + return new Promise((r) => { + resolveResume = r; + }); + return Promise.resolve(); + }); + + const { unmount } = render(); + // The mount IIFE awaits consume_pending_grant_resume first; it is + // suspended waiting for `resolveResume`. + + act(() => unmount()); // mountedRef → false + + await act(async () => { + resolveResume(null); // first guard fires; IIFE returns early + }); + + expect(errorSpy).not.toHaveBeenCalled(); + errorSpy.mockRestore(); + }); + it('ignores initial accessibility check result when component unmounts mid-flight', async () => { const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); let resolveInitial!: (v: boolean) => void; invoke.mockImplementation((cmd: string) => { + if (cmd === 'consume_pending_grant_resume') return Promise.resolve(null); if (cmd === 'check_accessibility_permission') return new Promise((r) => { resolveInitial = r; @@ -452,12 +597,14 @@ describe('OnboardingView', () => { }); const { unmount } = render(); - // useEffect has fired; initial invoke is in-flight (resolveInitial is set). + // Drain the consume await so the IIFE advances to the + // check_accessibility_permission await and exposes resolveInitial. + await act(async () => {}); act(() => unmount()); // mountedRef → false await act(async () => { - resolveInitial(true); // then-handler fires; guard returns early + resolveInitial(true); // post-AX guard fires; IIFE returns early }); expect(errorSpy).not.toHaveBeenCalled(); @@ -545,6 +692,44 @@ describe('OnboardingView', () => { errorSpy.mockRestore(); }); + it('ignores accessibility 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 === 'consume_pending_grant_resume') return Promise.resolve(null); + if (cmd === 'reset_and_relaunch_for_grant') return Promise.resolve(false); + if (cmd === 'check_accessibility_permission') + return Promise.resolve(false); + if (cmd === 'check_screen_recording_permission') + return Promise.resolve(false); + if (cmd === 'open_accessibility_settings') + return new Promise((r) => { + resolveOpen = r; + }); + return Promise.resolve(); + }); + + const { unmount } = render(); + await act(async () => {}); + + await act(async () => { + fireEvent.click( + screen.getByRole('button', { name: /grant accessibility/i }), + ); + }); + + // The click handler is now suspended inside startAccessibilityFlow on + // open_accessibility_settings; resolveOpen is set. + act(() => unmount()); + + await act(async () => { + resolveOpen(); // post-open mountedRef guard at PermissionsStep.tsx:192 fires + }); + + 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; diff --git a/src/view/onboarding/PermissionsStep.tsx b/src/view/onboarding/PermissionsStep.tsx index b49c440..4ea137a 100644 --- a/src/view/onboarding/PermissionsStep.tsx +++ b/src/view/onboarding/PermissionsStep.tsx @@ -180,27 +180,16 @@ export function PermissionsStep() { } }, []); - // On mount: check whether Accessibility is already granted so we can skip - // 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; - void invoke('check_accessibility_permission').then((granted) => { - if (!mountedRef.current) return; - if (granted) { - setAccessibilityStatus('granted'); - } - }); - return () => { - mountedRef.current = false; - stopAxPolling(); - stopScreenPolling(); - }; - }, [stopAxPolling, stopScreenPolling]); - - const handleGrantAccessibility = useCallback(async () => { + // Inline grant flow used both by a fresh click (when the backend reports + // no relaunch needed) and by the post-restart resume path. Opens System + // Settings directly and polls AXIsProcessTrusted until the user toggles + // the permission on. The first AXIsProcessTrusted call from a fresh PID + // is what registers Thuki in the System Settings list, so polling does + // double duty here. + const startAccessibilityFlow = useCallback(async () => { setAccessibilityStatus('requesting'); await invoke('open_accessibility_settings'); + if (!mountedRef.current) return; axPollRef.current = setInterval(async () => { if (axInFlightRef.current) return; axInFlightRef.current = true; @@ -217,10 +206,10 @@ export function PermissionsStep() { }, POLL_INTERVAL_MS); }, [stopAxPolling]); - 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. - // The registration call may briefly show a macOS system prompt on first use. + const startScreenRecordingFlow = useCallback(async () => { + // CGRequestScreenCaptureAccess is the call that adds Thuki to the + // Screen Recording list AND surfaces the macOS allow dialog. Without + // it the entry never appears, so the user has nothing to toggle. await invoke('request_screen_recording_access'); await invoke('open_screen_recording_settings'); if (!mountedRef.current) return; @@ -243,6 +232,71 @@ export function PermissionsStep() { }, POLL_INTERVAL_MS); }, [stopScreenPolling]); + // On mount: drain any pending click-time resume marker the previous + // process wrote before relaunching, then check whether Accessibility is + // already granted so we can skip step 1 and show step 2 immediately. + // Order matters: the resume handler may auto-start a flow whose state + // would otherwise be clobbered by the granted-check setter. + useEffect(() => { + // Reset on every mount so that a remount after unmount gets a fresh guard. + mountedRef.current = true; + + void (async () => { + const resume = await invoke( + 'consume_pending_grant_resume', + ); + if (!mountedRef.current) return; + + const accessibilityGranted = await invoke( + 'check_accessibility_permission', + ); + if (!mountedRef.current) return; + if (accessibilityGranted) setAccessibilityStatus('granted'); + + if (resume === 'Accessibility') { + // Previous process did the TCC reset+restart for Accessibility. + // Resume the open-Settings + polling step so the user does not + // have to click Grant a second time. + void startAccessibilityFlow(); + } else if (resume === 'ScreenCapture' && accessibilityGranted) { + void startScreenRecordingFlow(); + } + })(); + + return () => { + mountedRef.current = false; + stopAxPolling(); + stopScreenPolling(); + }; + }, [ + startAccessibilityFlow, + startScreenRecordingFlow, + stopAxPolling, + stopScreenPolling, + ]); + + const handleGrantAccessibility = useCallback(async () => { + setAccessibilityStatus('requesting'); + // Backend clears any stale TCC.Accessibility entry left over from a + // previous binary's code requirement and relaunches so tccd registers + // the new csreq cleanly. When the startup path already did the reset + // (true on a fresh install or a detected upgrade) the backend returns + // false and we run the open-Settings flow inline without a relaunch. + const restarting = await invoke('reset_and_relaunch_for_grant', { + service: 'Accessibility', + }); + if (!mountedRef.current || restarting) return; + await startAccessibilityFlow(); + }, [startAccessibilityFlow]); + + const handleOpenScreenRecording = useCallback(async () => { + const restarting = await invoke('reset_and_relaunch_for_grant', { + service: 'ScreenCapture', + }); + if (!mountedRef.current || restarting) return; + await startScreenRecordingFlow(); + }, [startScreenRecordingFlow]); + const handleQuitAndRelaunch = useCallback(async () => { await invoke('quit_and_relaunch'); }, []);