fix(si_server): idempotent start_session, suppress benign 'session already active' (#5J, #5H) (#1635)

Signed-off-by: oxoxDev <nikhil@tinyhumans.ai>
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

src/openhuman/screen_intelligence/engine.rs

@@ -77,12 +77,13 @@ impl AccessibilityEngine {
             }
         }
 
-        if !spawned_new_session {
-            return Ok(self.status().await.session);
+        let status = self.status().await;
+        if spawned_new_session {
+            self.spawn_workers().await;
+            Ok(self.status().await.session)
+        } else {
+            Ok(status.session)
         }
-
-        self.spawn_workers().await;
-        Ok(self.status().await.session)
     }
 
     pub async fn start_session(
@@ -101,40 +102,49 @@ impl AccessibilityEngine {
             .unwrap_or(ScreenIntelligenceConfig::default().session_ttl_secs)
             .clamp(30, 3600);
 
+        let mut spawned_new_session = false;
         {
             let mut state = self.inner.lock().await;
             if state.session.is_some() {
-                return Err("session already active".to_string());
+                tracing::debug!(
+                    "[screen_intelligence] start_session requested while session already active"
+                );
+            } else {
+                state.permissions = detect_permissions();
+                if state.permissions.accessibility != PermissionState::Granted {
+                    return Err("accessibility permission is not granted".to_string());
+                }
+
+                let screen_monitoring_requested = params.screen_monitoring.unwrap_or(true);
+                if screen_monitoring_requested
+                    && state.permissions.screen_recording != PermissionState::Granted
+                {
+                    return Err("screen recording permission is not granted".to_string());
+                }
+
+                let now = now_ms();
+                let expires_at_ms = now + (ttl_secs as i64 * 1000);
+                state.features.screen_monitoring = screen_monitoring_requested;
+
+                state.session = Some(new_session_runtime(
+                    &state.config,
+                    now,
+                    expires_at_ms,
+                    ttl_secs,
+                ));
+                state.last_event = Some("session_started".to_string());
+                state.last_error = None;
+                spawned_new_session = true;
             }
-
-            state.permissions = detect_permissions();
-            if state.permissions.accessibility != PermissionState::Granted {
-                return Err("accessibility permission is not granted".to_string());
-            }
-
-            let screen_monitoring_requested = params.screen_monitoring.unwrap_or(true);
-            if screen_monitoring_requested
-                && state.permissions.screen_recording != PermissionState::Granted
-            {
-                return Err("screen recording permission is not granted".to_string());
-            }
-
-            let now = now_ms();
-            let expires_at_ms = now + (ttl_secs as i64 * 1000);
-            state.features.screen_monitoring = screen_monitoring_requested;
-
-            state.session = Some(new_session_runtime(
-                &state.config,
-                now,
-                expires_at_ms,
-                ttl_secs,
-            ));
-            state.last_event = Some("session_started".to_string());
-            state.last_error = None;
         }
 
-        self.spawn_workers().await;
-        Ok(self.status().await.session)
+        let status = self.status().await;
+        if spawned_new_session {
+            self.spawn_workers().await;
+            Ok(self.status().await.session)
+        } else {
+            Ok(status.session)
+        }
     }
 
     pub async fn disable(&self, reason: Option<String>) -> SessionStatus {

src/openhuman/screen_intelligence/server.rs

@@ -365,6 +365,8 @@ pub async fn start_if_enabled(app_config: &Config) {
         if let Err(e) = server.run(&config_for_run).await {
             if is_expected_benign_start_session_failure(&e) {
                 info!("{LOG_PREFIX} embedded server autostart skipped: {e}");
+            } else if e.contains("session already active") {
+                info!("{LOG_PREFIX} embedded server session already active (benign)");
             } else {
                 error!("{LOG_PREFIX} embedded server exited with error: {e}");
             }

src/openhuman/screen_intelligence/tests.rs

@@ -844,3 +844,48 @@ async fn analyze_and_persist_frame_rejects_disabled_local_ai() {
         "unexpected error when local ai is disabled: {err}"
     );
 }
+
+#[tokio::test]
+async fn start_session_is_idempotent() {
+    if !cfg!(target_os = "macos") {
+        return;
+    }
+
+    let engine = Arc::new(AccessibilityEngine {
+        inner: Mutex::new(EngineState::new(ScreenIntelligenceConfig {
+            baseline_fps: 6.0,
+            session_ttl_secs: 60,
+            ..Default::default()
+        })),
+    });
+
+    // First start
+    let started = engine
+        .start_session(StartSessionParams {
+            consent: true,
+            ttl_secs: Some(60),
+            screen_monitoring: Some(true),
+        })
+        .await;
+
+    if started.is_err() {
+        // If we can't start the first session (e.g. no permissions in test env),
+        // we can't test idempotency properly here, but it shouldn't fail the test.
+        return;
+    }
+
+    // Second start - should succeed and return the same session status (active: true)
+    let second_start = engine
+        .start_session(StartSessionParams {
+            consent: true,
+            ttl_secs: Some(60),
+            screen_monitoring: Some(true),
+        })
+        .await;
+
+    assert!(second_start.is_ok(), "Second start_session should be Ok");
+    assert!(
+        second_start.unwrap().active,
+        "Second start_session should return active session status"
+    );
+}