From 0dd4d1f60b82a3fdf55df9b8c2b3cd4713d0af6c Mon Sep 17 00:00:00 2001 From: Mega Mind <146339422+M3gA-Mind@users.noreply.github.com> Date: Thu, 2 Apr 2026 05:04:38 +0530 Subject: [PATCH] fix(macos): restart sidecar so TCC permission state updates after System Settings (#133) (#182) * fix(macos): restart sidecar so permission grants show after System Settings Fixes #133 - Tauri: restart core after kill+wait; fail fast when port held by non-managed process - ACL: allow core_rpc_url and restart_core_process (IPC was blocked in Tauri 2) - Dev: default_core_bin falls through to release search when binaries/ empty - Core: expose permission_check_process_path on accessibility status - App: Restart & refresh UX, extractError for invoke, Vitest for RPC + slice - Stage script: optional dev codesign helper for stable TCC identity Made-with: Cursor * refactor(onboarding): update button text for clarity and enhance core process restart handling - Changed button text in ScreenPermissionsStep from 'Refresh Status' to 'Restart & Refresh Permissions' for better user understanding. - Introduced a restart lock in CoreProcessHandle to serialize overlapping restart requests, ensuring smoother core process management. - Updated restart_core_process function to acquire the restart lock before initiating a restart, improving reliability during the process. * fix: improve text clarity in AccessibilityPanel and ScreenIntelligencePanel - Adjusted text formatting in AccessibilityPanel for better readability. - Enhanced text clarity in ScreenIntelligencePanel regarding permission refresh instructions. - Standardized the formatting of permission_check_process_path in test files for consistency. --------- Co-authored-by: Steven Enamakel --- app/src-tauri/capabilities/default.json | 15 ++- .../permissions/allow-core-process.toml | 10 ++ app/src-tauri/src/core_process.rs | 108 +++++++++++++++++- app/src-tauri/src/lib.rs | 11 ++ .../settings/panels/AccessibilityPanel.tsx | 60 ++++++++-- .../panels/ScreenIntelligencePanel.tsx | 54 +++++++-- .../__tests__/AccessibilityPanel.test.tsx | 1 + .../steps/ScreenPermissionsStep.tsx | 47 +++++--- .../services/__tests__/coreRpcClient.test.ts | 77 +++++++++++++ .../__tests__/accessibilitySlice.test.ts | 23 ++++ app/src/store/accessibilitySlice.ts | 75 ++++++++++++ app/src/utils/tauriCommands.ts | 22 ++++ scripts/setup-dev-codesign.sh | 85 ++++++++++++++ scripts/stage-core-sidecar.mjs | 22 ++++ src/openhuman/screen_intelligence/engine.rs | 3 + src/openhuman/screen_intelligence/ops.rs | 29 +++++ src/openhuman/screen_intelligence/schemas.rs | 21 ++++ src/openhuman/screen_intelligence/types.rs | 4 + 18 files changed, 624 insertions(+), 43 deletions(-) create mode 100644 app/src-tauri/permissions/allow-core-process.toml create mode 100755 scripts/setup-dev-codesign.sh diff --git a/app/src-tauri/capabilities/default.json b/app/src-tauri/capabilities/default.json index b3246bd8e..23af2745d 100644 --- a/app/src-tauri/capabilities/default.json +++ b/app/src-tauri/capabilities/default.json @@ -2,14 +2,21 @@ "$schema": "../gen/schemas/desktop-schema.json", "identifier": "default", "description": "Capability for the main window (desktop only)", - "platforms": ["linux", "macOS", "windows"], - "windows": ["main"], + "platforms": [ + "linux", + "macOS", + "windows" + ], + "windows": [ + "main" + ], "permissions": [ "core:default", "core:window:allow-show", "core:window:allow-set-focus", "core:window:allow-unminimize", "deep-link:default", - "opener:default" + "opener:default", + "allow-core-process" ] -} +} \ No newline at end of file diff --git a/app/src-tauri/permissions/allow-core-process.toml b/app/src-tauri/permissions/allow-core-process.toml new file mode 100644 index 000000000..84c63c710 --- /dev/null +++ b/app/src-tauri/permissions/allow-core-process.toml @@ -0,0 +1,10 @@ +[[permission]] +identifier = "allow-core-process" +description = "Core RPC URL and sidecar restart" + +[permission.commands] +allow = [ + "core_rpc_url", + "restart_core_process", +] +deny = [] diff --git a/app/src-tauri/src/core_process.rs b/app/src-tauri/src/core_process.rs index 3c2b59ac3..1cc9c78be 100644 --- a/app/src-tauri/src/core_process.rs +++ b/app/src-tauri/src/core_process.rs @@ -17,6 +17,7 @@ pub enum CoreRunMode { pub struct CoreProcessHandle { child: Arc>>, task: Arc>>>, + restart_lock: Arc>, port: u16, core_bin: Option, run_mode: CoreRunMode, @@ -27,6 +28,7 @@ impl CoreProcessHandle { Self { child: Arc::new(Mutex::new(None)), task: Arc::new(Mutex::new(None)), + restart_lock: Arc::new(Mutex::new(())), port, core_bin, run_mode, @@ -37,6 +39,11 @@ impl CoreProcessHandle { format!("http://127.0.0.1:{}/rpc", self.port) } + /// Acquire the restart lock to serialize overlapping restart requests. + pub async fn restart_lock(&self) -> tokio::sync::MutexGuard<'_, ()> { + self.restart_lock.lock().await + } + async fn is_rpc_port_open(&self) -> bool { matches!( timeout( @@ -177,6 +184,76 @@ impl CoreProcessHandle { Err("core process did not become ready".to_string()) } + /// Restart the core process to pick up updated macOS permission grants. + /// + /// macOS caches permission state per-process; the running sidecar never sees + /// a newly granted permission until it restarts. This method shuts down the + /// current child, waits until the RPC port is free (so `ensure_running` does not + /// fast-return while the old listener is still bound), then spawns a fresh instance. + /// + /// If another process is listening on the core port (e.g. manual `openhuman core run`), + /// shutdown does not stop it — we time out and return an error instead of a false success. + /// + /// Issue: + pub async fn restart(&self) -> Result<(), String> { + log::info!("[core] restarting core process for permission refresh"); + + let had_managed_child = { + let guard = self.child.lock().await; + guard.is_some() + }; + log::debug!( + "[core] restart: had_managed_child={} before shutdown", + had_managed_child + ); + + self.shutdown().await; + log::debug!("[core] restart: shutdown complete, checking port {}", self.port); + + // If we never spawned the sidecar (something else was already listening), we cannot free + // the port — fail fast with a clear message instead of polling for 8s. + if !had_managed_child && self.is_rpc_port_open().await { + log::error!( + "[core] restart: no child to stop but port {} is open — another process owns it", + self.port + ); + return Err(format!( + "Core RPC port {} is already in use by another process (OpenHuman did not start it). Quit any `openhuman core run` in a terminal or free the port, then relaunch the app. You can also set OPENHUMAN_CORE_PORT to a different port.", + self.port + )); + } + + // After kill+wait on our child, the port should close; poll briefly in case the OS is slow + // to release the socket. + const POLL_MS: u64 = 50; + const MAX_WAIT_MS: u64 = 10_000; + let mut waited_ms: u64 = 0; + while self.is_rpc_port_open().await { + if waited_ms >= MAX_WAIT_MS { + log::error!( + "[core] restart: port {} still in use after {}ms (had_managed_child={})", + self.port, + MAX_WAIT_MS, + had_managed_child + ); + return Err(format!( + "Core RPC port {} did not become free after stopping the sidecar. Quit any other process using this port (e.g. `openhuman core run`) or change OPENHUMAN_CORE_PORT.", + self.port + )); + } + tokio::time::sleep(std::time::Duration::from_millis(POLL_MS)).await; + waited_ms += POLL_MS; + } + + log::debug!("[core] restart: port free, calling ensure_running"); + let result = self.ensure_running().await; + match &result { + Ok(()) => log::info!("[core] restart: core process ready after restart"), + Err(e) => log::error!("[core] restart: failed to restart core process: {e}"), + } + result + } + /// Stop the core process this handle spawned (child or in-process task). Safe to call if /// nothing was spawned or core was already external. pub async fn shutdown(&self) { @@ -186,6 +263,26 @@ impl CoreProcessHandle { if let Err(e) = child.kill().await { log::warn!("[core] failed to kill child core process: {e}"); } + // Wait for the process to exit so the RPC listen socket is released before restart + // checks the port (otherwise we can spuriously hit "port still in use"). + match timeout( + Duration::from_secs(12), + child.wait(), + ) + .await + { + Ok(Ok(status)) => { + log::debug!("[core] child core process reaped after kill: {status}"); + } + Ok(Err(e)) => { + log::warn!("[core] wait on child core process after kill: {e}"); + } + Err(_) => { + log::warn!( + "[core] timed out waiting for child core process to exit after kill (12s)" + ); + } + } } let mut task_guard = self.task.lock().await; if let Some(task) = task_guard.take() { @@ -244,9 +341,12 @@ pub fn default_core_bin() -> Option { } } - // Dev ergonomics: allow an explicit staged sidecar from src-tauri/binaries in - // debug builds before falling back to self-subcommand spawning. - if cfg!(debug_assertions) { + // Dev: prefer a staged sidecar under src-tauri/binaries, then use the same search as + // release (next to the .app, Resources/, etc.). Previously we returned None here when the + // folder was empty, which forced `core run` on the GUI binary — a different TCC identity than + // `openhuman-core-*` and misleading "still denied" after granting the sidecar name. + #[cfg(debug_assertions)] + { let binaries_dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("binaries"); if let Ok(entries) = std::fs::read_dir(&binaries_dir) { for entry in entries.flatten() { @@ -267,8 +367,6 @@ pub fn default_core_bin() -> Option { } } } - - return None; } let exe = std::env::current_exe().ok()?; diff --git a/app/src-tauri/src/lib.rs b/app/src-tauri/src/lib.rs index eb092473f..780633926 100644 --- a/app/src-tauri/src/lib.rs +++ b/app/src-tauri/src/lib.rs @@ -96,6 +96,16 @@ async fn service_uninstall_direct() -> Result { run_core_cli(vec!["service".into(), "uninstall".into()]).await } +#[tauri::command] +async fn restart_core_process( + state: tauri::State<'_, core_process::CoreProcessHandle>, +) -> Result<(), String> { + log::info!("[core] restart_core_process: command invoked from frontend"); + let _guard = state.inner().restart_lock().await; + log::debug!("[core] restart_core_process: acquired restart lock"); + state.inner().restart().await +} + fn is_daemon_mode() -> bool { std::env::args().any(|arg| arg == "daemon" || arg == "--daemon") } @@ -220,6 +230,7 @@ pub fn run() { }) .invoke_handler(tauri::generate_handler![ core_rpc_url, + restart_core_process, service_install_direct, service_start_direct, service_stop_direct, diff --git a/app/src/components/settings/panels/AccessibilityPanel.tsx b/app/src/components/settings/panels/AccessibilityPanel.tsx index 081fb2750..fc2a4b87c 100644 --- a/app/src/components/settings/panels/AccessibilityPanel.tsx +++ b/app/src/components/settings/panels/AccessibilityPanel.tsx @@ -4,6 +4,7 @@ import { fetchAccessibilityStatus, fetchAccessibilityVisionRecent, flushAccessibilityVision, + refreshPermissionsWithRestart, requestAccessibilityPermission, startAccessibilitySession, stopAccessibilitySession, @@ -50,6 +51,7 @@ const AccessibilityPanel = () => { status, isLoading, isRequestingPermissions, + isRestartingCore, isStartingSession, isStoppingSession, isLoadingVision, @@ -82,6 +84,11 @@ const AccessibilityPanel = () => { void dispatch(fetchAccessibilityVisionRecent(10)); }, [dispatch]); + const anyPermissionDenied = + status?.permissions.screen_recording === 'denied' || + status?.permissions.accessibility === 'denied' || + status?.permissions.input_monitoring === 'denied'; + const screenMonitoring = featureOverrides.screen_monitoring ?? status?.features.screen_monitoring ?? true; const deviceControl = featureOverrides.device_control ?? status?.features.device_control ?? true; @@ -125,34 +132,67 @@ const AccessibilityPanel = () => { value={status?.permissions.input_monitoring ?? 'unknown'} /> + {anyPermissionDenied && ( +
+

+ After granting in System Settings, click “Restart & Refresh” below. +

+ {status?.permission_check_process_path ? ( +

+ Enable the same app macOS lists for this path (TCC is per executable).{' '} + + {status.permission_check_process_path} + +

+ ) : null} +

+ Still stuck? Remove the old entry for this app in System Settings → Privacy, then + click “Request” again. For dev, run{' '} + yarn core:stage so the sidecar matches + the staged binary name. +

+
+ )} + - + + {anyPermissionDenied ? ( + + ) : ( + + )}
diff --git a/app/src/components/settings/panels/ScreenIntelligencePanel.tsx b/app/src/components/settings/panels/ScreenIntelligencePanel.tsx index fe657815b..ecb7c2fde 100644 --- a/app/src/components/settings/panels/ScreenIntelligencePanel.tsx +++ b/app/src/components/settings/panels/ScreenIntelligencePanel.tsx @@ -5,6 +5,7 @@ import { fetchAccessibilityStatus, fetchAccessibilityVisionRecent, flushAccessibilityVision, + refreshPermissionsWithRestart, requestAccessibilityPermission, startAccessibilitySession, stopAccessibilitySession, @@ -52,6 +53,7 @@ const ScreenIntelligencePanel = () => { status, isLoading, isRequestingPermissions, + isRestartingCore, isStartingSession, isStoppingSession, isLoadingVision, @@ -117,6 +119,11 @@ const ScreenIntelligencePanel = () => { [status?.session.remaining_ms] ); + const anyPermissionDenied = + status?.permissions.screen_recording === 'denied' || + status?.permissions.accessibility === 'denied' || + status?.permissions.input_monitoring === 'denied'; + const startDisabled = isStartingSession || isLoading || @@ -172,34 +179,61 @@ const ScreenIntelligencePanel = () => { value={status?.permissions.input_monitoring ?? 'unknown'} /> + {anyPermissionDenied && ( +
+

+ After granting in System Settings, click “Restart & Refresh + Permissions” so a new core process picks up the grants. +

+ {status?.permission_check_process_path ? ( +

+ macOS applies privacy to this executable:{' '} + + {status.permission_check_process_path} + +

+ ) : null} +
+ )} + - + {anyPermissionDenied ? ( + + ) : ( + + )}
diff --git a/app/src/components/settings/panels/__tests__/AccessibilityPanel.test.tsx b/app/src/components/settings/panels/__tests__/AccessibilityPanel.test.tsx index 4893df94a..a1098bb36 100644 --- a/app/src/components/settings/panels/__tests__/AccessibilityPanel.test.tsx +++ b/app/src/components/settings/panels/__tests__/AccessibilityPanel.test.tsx @@ -70,6 +70,7 @@ const createStore = () => isCaptureTestRunning: false, isLoading: false, isRequestingPermissions: false, + isRestartingCore: false, isStartingSession: false, isStoppingSession: false, isLoadingVision: false, diff --git a/app/src/pages/onboarding/steps/ScreenPermissionsStep.tsx b/app/src/pages/onboarding/steps/ScreenPermissionsStep.tsx index 7bc2b61cf..d26875050 100644 --- a/app/src/pages/onboarding/steps/ScreenPermissionsStep.tsx +++ b/app/src/pages/onboarding/steps/ScreenPermissionsStep.tsx @@ -3,6 +3,7 @@ import { useNavigate } from 'react-router-dom'; import { fetchAccessibilityStatus, + refreshPermissionsWithRestart, requestAccessibilityPermission, } from '../../../store/accessibilitySlice'; import { useAppDispatch, useAppSelector } from '../../../store/hooks'; @@ -15,9 +16,8 @@ interface ScreenPermissionsStepProps { const ScreenPermissionsStep = ({ onNext, onBack: _onBack }: ScreenPermissionsStepProps) => { const navigate = useNavigate(); const dispatch = useAppDispatch(); - const { status, isLoading, isRequestingPermissions } = useAppSelector( - state => state.accessibility - ); + const { status, isLoading, isRequestingPermissions, isRestartingCore, lastError } = + useAppSelector(state => state.accessibility); useEffect(() => { void dispatch(fetchAccessibilityStatus()); @@ -63,20 +63,39 @@ const ScreenPermissionsStep = ({ onNext, onBack: _onBack }: ScreenPermissionsSte {!isGranted ? ( -
+
+
+ + +
- + {(lastError || status?.permission_check_process_path) && ( +
+ {lastError ?

{lastError}

: null} + {status?.permission_check_process_path ? ( +

+ Grant access for: {status.permission_check_process_path} +

+ ) : null} +
+ )}
) : (