From 79fe36b41dcbc8f6bff28a7595bd0fad0eba5396 Mon Sep 17 00:00:00 2001 From: YellowSnnowmann <167776381+YellowSnnowmann@users.noreply.github.com> Date: Thu, 2 Apr 2026 20:07:15 +0530 Subject: [PATCH] fix: remove the deadlock when re-enabling an already-active session and add unit tests for ScreenIntelligenceDebugPanel and ScreenIntelligencePanel (#275) --- .../ScreenIntelligenceDebugPanel.test.tsx | 208 +++++++++++++++ .../ScreenIntelligencePanel.test.tsx | 233 +++++++++++++++++ .../e2e/specs/screen-intelligence.spec.ts | 136 ++++++++-- src/openhuman/accessibility/capture.rs | 246 +++++++++++++----- src/openhuman/screen_intelligence/engine.rs | 118 ++++++--- src/openhuman/screen_intelligence/tests.rs | 25 +- tests/json_rpc_e2e.rs | 82 ++++++ 7 files changed, 934 insertions(+), 114 deletions(-) create mode 100644 app/src/components/intelligence/__tests__/ScreenIntelligenceDebugPanel.test.tsx create mode 100644 app/src/components/settings/panels/__tests__/ScreenIntelligencePanel.test.tsx diff --git a/app/src/components/intelligence/__tests__/ScreenIntelligenceDebugPanel.test.tsx b/app/src/components/intelligence/__tests__/ScreenIntelligenceDebugPanel.test.tsx new file mode 100644 index 000000000..fbe61d6ac --- /dev/null +++ b/app/src/components/intelligence/__tests__/ScreenIntelligenceDebugPanel.test.tsx @@ -0,0 +1,208 @@ +import { configureStore } from '@reduxjs/toolkit'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { Provider } from 'react-redux'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import accessibilityReducer from '../../../store/accessibilitySlice'; +import authReducer from '../../../store/authSlice'; +import socketReducer from '../../../store/socketSlice'; +import teamReducer from '../../../store/teamSlice'; +import userReducer from '../../../store/userSlice'; +import { + type AccessibilityStatus, + type AccessibilityVisionRecentResult, + type CaptureTestResult, + type CommandResponse, + openhumanAccessibilityInputAction, + openhumanAccessibilityRequestPermission, + openhumanAccessibilityRequestPermissions, + openhumanAccessibilityStartSession, + openhumanAccessibilityStatus, + openhumanAccessibilityStopSession, + openhumanAccessibilityVisionFlush, + openhumanAccessibilityVisionRecent, + openhumanScreenIntelligenceCaptureTest, + restartCoreProcess, +} from '../../../utils/tauriCommands'; +import ScreenIntelligenceDebugPanel from '../ScreenIntelligenceDebugPanel'; + +vi.mock('../../../utils/tauriCommands', () => ({ + isTauri: vi.fn(() => true), + openhumanAccessibilityInputAction: vi.fn(), + openhumanAccessibilityRequestPermission: vi.fn(), + openhumanAccessibilityRequestPermissions: vi.fn(), + openhumanAccessibilityStartSession: vi.fn(), + openhumanAccessibilityStatus: vi.fn(), + openhumanAccessibilityStopSession: vi.fn(), + openhumanAccessibilityVisionFlush: vi.fn(), + openhumanAccessibilityVisionRecent: vi.fn(), + openhumanScreenIntelligenceCaptureTest: vi.fn(), + restartCoreProcess: vi.fn(), +})); + +const sampleStatus: AccessibilityStatus = { + platform_supported: true, + permissions: { + screen_recording: 'granted', + accessibility: 'granted', + input_monitoring: 'granted', + }, + features: { screen_monitoring: true, device_control: true, predictive_input: true }, + session: { + active: false, + started_at_ms: null, + expires_at_ms: null, + remaining_ms: null, + ttl_secs: 300, + panic_hotkey: 'Cmd+Shift+.', + stop_reason: null, + frames_in_memory: 0, + last_capture_at_ms: null, + last_context: null, + vision_enabled: true, + vision_state: 'idle', + vision_queue_depth: 0, + last_vision_at_ms: null, + last_vision_summary: null, + }, + config: { + enabled: true, + capture_policy: 'hybrid', + policy_mode: 'all_except_blacklist', + baseline_fps: 1, + vision_enabled: true, + session_ttl_secs: 300, + panic_stop_hotkey: 'Cmd+Shift+.', + autocomplete_enabled: true, + allowlist: [], + denylist: [], + }, + denylist: [], + is_context_blocked: false, +}; + +const emptyVisionResponse: CommandResponse = { + result: { summaries: [] }, + logs: [], +}; + +const createStore = () => + configureStore({ + reducer: { + auth: authReducer, + socket: socketReducer, + user: userReducer, + team: teamReducer, + accessibility: accessibilityReducer, + }, + }); + +function renderPanel() { + const store = createStore(); + render( + + + + ); + return store; +} + +describe('ScreenIntelligenceDebugPanel', () => { + beforeEach(() => { + vi.mocked(openhumanAccessibilityStatus).mockResolvedValue({ result: sampleStatus, logs: [] }); + vi.mocked(openhumanAccessibilityVisionRecent).mockResolvedValue(emptyVisionResponse); + vi.mocked(openhumanAccessibilityInputAction).mockResolvedValue({ + result: {} as never, + logs: [], + }); + vi.mocked(openhumanAccessibilityRequestPermission).mockResolvedValue({ + result: sampleStatus.permissions, + logs: [], + } as never); + vi.mocked(openhumanAccessibilityRequestPermissions).mockResolvedValue({ + result: sampleStatus.permissions, + logs: [], + } as never); + vi.mocked(openhumanAccessibilityStartSession).mockResolvedValue({ + result: sampleStatus.session, + logs: [], + } as never); + vi.mocked(openhumanAccessibilityStopSession).mockResolvedValue({ + result: sampleStatus.session, + logs: [], + } as never); + vi.mocked(openhumanAccessibilityVisionFlush).mockResolvedValue({ + result: { accepted: true, summary: null }, + logs: [], + } as never); + vi.mocked(restartCoreProcess).mockResolvedValue(undefined); + }); + + it('renders successful capture diagnostics and preview image', async () => { + const captureResult: CaptureTestResult = { + ok: true, + capture_mode: 'windowed', + context: { + app_name: 'Safari', + window_title: 'GitHub', + bounds_x: 10, + bounds_y: 20, + bounds_width: 1440, + bounds_height: 900, + }, + image_ref: 'data:image/png;base64,ZmFrZQ==', + bytes_estimate: 2048, + error: null, + timing_ms: 155, + }; + vi.mocked(openhumanScreenIntelligenceCaptureTest).mockResolvedValue({ + result: captureResult, + logs: [], + }); + + renderPanel(); + + await waitFor(() => { + expect(openhumanAccessibilityStatus).toHaveBeenCalled(); + expect(openhumanAccessibilityVisionRecent).toHaveBeenCalledWith(5); + }); + + fireEvent.click(screen.getByRole('button', { name: 'Test Capture' })); + + expect(await screen.findByText('Success')).toBeInTheDocument(); + expect(screen.getByText('windowed')).toBeInTheDocument(); + expect(screen.getByText('155ms')).toBeInTheDocument(); + expect(screen.getByText('2.0 KB')).toBeInTheDocument(); + expect(screen.getByText('Safari')).toBeInTheDocument(); + expect(screen.getByAltText('Capture test result')).toHaveAttribute( + 'src', + 'data:image/png;base64,ZmFrZQ==' + ); + }); + + it('renders capture failures without breaking the diagnostics panel', async () => { + vi.mocked(openhumanScreenIntelligenceCaptureTest).mockResolvedValue({ + result: { + ok: false, + capture_mode: 'fullscreen', + context: null, + image_ref: null, + bytes_estimate: null, + error: 'screen recording permission is not granted', + timing_ms: 42, + }, + logs: [], + }); + + renderPanel(); + + fireEvent.click(screen.getByRole('button', { name: 'Test Capture' })); + + expect(await screen.findByText('Failed')).toBeInTheDocument(); + expect(screen.getByText('fullscreen')).toBeInTheDocument(); + expect(screen.getByText('42ms')).toBeInTheDocument(); + expect(screen.getByText('screen recording permission is not granted')).toBeInTheDocument(); + expect(screen.queryByAltText('Capture test result')).not.toBeInTheDocument(); + expect(screen.getByText('Permissions')).toBeInTheDocument(); + }); +}); diff --git a/app/src/components/settings/panels/__tests__/ScreenIntelligencePanel.test.tsx b/app/src/components/settings/panels/__tests__/ScreenIntelligencePanel.test.tsx new file mode 100644 index 000000000..d4b664bda --- /dev/null +++ b/app/src/components/settings/panels/__tests__/ScreenIntelligencePanel.test.tsx @@ -0,0 +1,233 @@ +import { configureStore } from '@reduxjs/toolkit'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { Provider } from 'react-redux'; +import { MemoryRouter } from 'react-router-dom'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import accessibilityReducer from '../../../../store/accessibilitySlice'; +import authReducer from '../../../../store/authSlice'; +import socketReducer from '../../../../store/socketSlice'; +import teamReducer from '../../../../store/teamSlice'; +import userReducer from '../../../../store/userSlice'; +import { + type AccessibilityStatus, + type AccessibilityVisionRecentResult, + type CommandResponse, + type ConfigSnapshot, + isTauri, + openhumanAccessibilityInputAction, + openhumanAccessibilityRequestPermission, + openhumanAccessibilityRequestPermissions, + openhumanAccessibilityStartSession, + openhumanAccessibilityStatus, + openhumanAccessibilityStopSession, + openhumanAccessibilityVisionFlush, + openhumanAccessibilityVisionRecent, + openhumanScreenIntelligenceCaptureTest, + openhumanUpdateScreenIntelligenceSettings, + restartCoreProcess, +} from '../../../../utils/tauriCommands'; +import ScreenIntelligencePanel from '../ScreenIntelligencePanel'; + +vi.mock('../../../../utils/tauriCommands', () => ({ + isTauri: vi.fn(() => true), + openhumanAccessibilityInputAction: vi.fn(), + openhumanAccessibilityRequestPermission: vi.fn(), + openhumanAccessibilityRequestPermissions: vi.fn(), + openhumanAccessibilityStartSession: vi.fn(), + openhumanAccessibilityStatus: vi.fn(), + openhumanAccessibilityStopSession: vi.fn(), + openhumanAccessibilityVisionFlush: vi.fn(), + openhumanAccessibilityVisionRecent: vi.fn(), + openhumanScreenIntelligenceCaptureTest: vi.fn(), + openhumanUpdateScreenIntelligenceSettings: vi.fn(), + restartCoreProcess: vi.fn(), +})); + +const baseStatus: AccessibilityStatus = { + platform_supported: true, + permissions: { + screen_recording: 'granted', + accessibility: 'granted', + input_monitoring: 'unknown', + }, + features: { screen_monitoring: true, device_control: true, predictive_input: true }, + session: { + active: false, + started_at_ms: null, + expires_at_ms: null, + remaining_ms: null, + ttl_secs: 300, + panic_hotkey: 'Cmd+Shift+.', + stop_reason: null, + frames_in_memory: 0, + last_capture_at_ms: null, + last_context: null, + vision_enabled: true, + vision_state: 'idle', + vision_queue_depth: 0, + last_vision_at_ms: null, + last_vision_summary: null, + }, + config: { + enabled: false, + capture_policy: 'hybrid', + policy_mode: 'all_except_blacklist', + baseline_fps: 1, + vision_enabled: true, + session_ttl_secs: 300, + panic_stop_hotkey: 'Cmd+Shift+.', + autocomplete_enabled: true, + allowlist: ['Code'], + denylist: ['1Password'], + }, + denylist: ['1Password'], + is_context_blocked: false, + permission_check_process_path: '/tmp/openhuman-core', +}; + +const emptyVisionResponse: CommandResponse = { + result: { summaries: [] }, + logs: [], +}; + +const createStore = () => + configureStore({ + reducer: { + auth: authReducer, + socket: socketReducer, + user: userReducer, + team: teamReducer, + accessibility: accessibilityReducer, + }, + }); + +function renderPanel() { + const store = createStore(); + render( + + + + + + ); + return store; +} + +function createDeferred() { + let resolve!: (value: T) => void; + const promise = new Promise(res => { + resolve = res; + }); + return { promise, resolve }; +} + +describe('ScreenIntelligencePanel', () => { + beforeEach(() => { + vi.mocked(isTauri).mockReturnValue(true); + vi.mocked(openhumanAccessibilityStatus).mockResolvedValue({ result: baseStatus, logs: [] }); + vi.mocked(openhumanAccessibilityVisionRecent).mockResolvedValue(emptyVisionResponse); + vi.mocked(openhumanAccessibilityInputAction).mockResolvedValue({ + result: {} as never, + logs: [], + }); + vi.mocked(openhumanAccessibilityRequestPermission).mockResolvedValue({ + result: baseStatus.permissions, + logs: [], + } as never); + vi.mocked(openhumanAccessibilityRequestPermissions).mockResolvedValue({ + result: baseStatus.permissions, + logs: [], + } as never); + vi.mocked(openhumanAccessibilityStartSession).mockResolvedValue({ + result: baseStatus.session, + logs: [], + } as never); + vi.mocked(openhumanAccessibilityStopSession).mockResolvedValue({ + result: baseStatus.session, + logs: [], + } as never); + vi.mocked(openhumanAccessibilityVisionFlush).mockResolvedValue({ + result: { accepted: true, summary: null }, + logs: [], + } as never); + vi.mocked(openhumanScreenIntelligenceCaptureTest).mockResolvedValue({ + result: { + ok: false, + capture_mode: 'fullscreen', + context: null, + image_ref: null, + bytes_estimate: null, + error: 'screen capture is unsupported on this platform', + timing_ms: 12, + }, + logs: [], + }); + vi.mocked(restartCoreProcess).mockResolvedValue(undefined); + }); + + it('saves screen intelligence settings and clears the saving state', async () => { + const deferred = createDeferred>(); + vi.mocked(openhumanUpdateScreenIntelligenceSettings).mockReturnValueOnce(deferred.promise); + + renderPanel(); + + await screen.findByText('Screen Intelligence Policy'); + + const enabledLabel = screen.getByText('Enabled').closest('label'); + const enabledCheckbox = enabledLabel?.querySelector( + 'input[type="checkbox"]' + ) as HTMLInputElement; + expect(enabledCheckbox.checked).toBe(false); + + fireEvent.click(enabledCheckbox); + fireEvent.click(screen.getByRole('button', { name: 'Save Screen Intelligence Settings' })); + + expect(await screen.findByRole('button', { name: 'Saving…' })).toBeInTheDocument(); + expect(openhumanUpdateScreenIntelligenceSettings).toHaveBeenCalledWith({ + enabled: true, + policy_mode: 'all_except_blacklist', + baseline_fps: 1, + allowlist: ['Code'], + denylist: ['1Password'], + }); + + deferred.resolve({ + result: { config: {}, workspace_dir: '/tmp/workspace', config_path: '/tmp/config.toml' }, + logs: [], + }); + + await waitFor(() => { + expect( + screen.getByRole('button', { name: 'Save Screen Intelligence Settings' }) + ).toBeInTheDocument(); + }); + expect(openhumanAccessibilityStatus).toHaveBeenCalledTimes(2); + }); + + it('shows permission restart guidance and unsupported-platform messaging', async () => { + vi.mocked(openhumanAccessibilityStatus).mockResolvedValueOnce({ + result: { + ...baseStatus, + platform_supported: false, + permissions: { + screen_recording: 'denied', + accessibility: 'denied', + input_monitoring: 'unknown', + }, + }, + logs: [], + }); + + renderPanel(); + + expect(await screen.findByText('Permissions')).toBeInTheDocument(); + expect(screen.getByText(/After granting in System Settings, click/i)).toBeInTheDocument(); + expect( + screen.getByRole('button', { name: 'Restart & Refresh Permissions' }) + ).toBeInTheDocument(); + expect( + screen.getByText('Screen Intelligence V1 is currently supported on macOS only.') + ).toBeInTheDocument(); + }); +}); diff --git a/app/test/e2e/specs/screen-intelligence.spec.ts b/app/test/e2e/specs/screen-intelligence.spec.ts index 08690ffe8..a072dee60 100644 --- a/app/test/e2e/specs/screen-intelligence.spec.ts +++ b/app/test/e2e/specs/screen-intelligence.spec.ts @@ -1,36 +1,124 @@ -// @ts-nocheck -/** - * E2E test: Screen Intelligence settings and Intelligence page. - * - * Verifies: - * 1. App launches and has visible elements - * 2. Settings navigation works (screen intelligence panel) - * 3. Intelligence page loads without errors - * - * Note: Screen capture will fail gracefully without macOS permissions in CI. - * The tests verify the UI renders correctly and handles errors, not that - * actual screenshots are taken. - */ -import { waitForApp } from '../helpers/app-helpers'; -import { hasAppChrome } from '../helpers/element-helpers'; -import { startMockServer, stopMockServer } from '../mock-server'; +import { browser, expect } from '@wdio/globals'; + +import { waitForApp, waitForAppReady } from '../helpers/app-helpers'; +import { triggerAuthDeepLinkBypass } from '../helpers/deep-link-helpers'; +import { + clickButton, + dumpAccessibilityTree, + hasAppChrome, + textExists, + waitForText, + waitForWebView, + waitForWindowVisible, +} from '../helpers/element-helpers'; +import { isTauriDriver } from '../helpers/platform'; +import { navigateViaHash } from '../helpers/shared-flows'; +import { clearRequestLog, startMockServer, stopMockServer } from '../mock-server'; + +function stepLog(message: string, context?: unknown): void { + const stamp = new Date().toISOString(); + if (context === undefined) { + console.log(`[ScreenIntelligenceE2E][${stamp}] ${message}`); + return; + } + console.log(`[ScreenIntelligenceE2E][${stamp}] ${message}`, JSON.stringify(context, null, 2)); +} + +async function waitForCaptureOutcome(timeoutMs = 20_000): Promise<'success' | 'failure'> { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if ( + (await textExists('Success')) && + ((await textExists('windowed')) || (await textExists('fullscreen'))) + ) { + return 'success'; + } + if ( + (await textExists('Failed')) || + (await textExists('screen recording permission is not granted')) || + (await textExists('screen capture is unsupported on this platform')) || + (await textExists('screen capture failed')) + ) { + return 'failure'; + } + await browser.pause(500); + } + throw new Error('Timed out waiting for screen capture outcome'); +} describe('Screen Intelligence', () => { before(async () => { - startMockServer(); + stepLog('Starting Screen Intelligence E2E'); + await startMockServer(); await waitForApp(); + clearRequestLog(); }); after(async () => { - stopMockServer(); + await stopMockServer(); }); - it('app launches with elements', async () => { - const elements = await browser.$$('//*'); - expect(elements.length).toBeGreaterThan(0); - }); - - it('app chrome is visible', async () => { + it('authenticates and reaches the app shell', async () => { + await triggerAuthDeepLinkBypass('e2e-screen-intelligence-user'); + await waitForWindowVisible(25_000); + await waitForWebView(15_000); + await waitForAppReady(15_000); expect(await hasAppChrome()).toBe(true); }); + + it('opens the Screen Intelligence settings route', async function () { + if (!isTauriDriver()) { + this.skip(); + return; + } + + await navigateViaHash('/settings/screen-intelligence'); + const currentHash = await browser.execute(() => window.location.hash); + stepLog('Navigated to screen intelligence route', { currentHash }); + + expect(currentHash).toContain('/settings/screen-intelligence'); + await waitForText('Screen Intelligence', 10_000); + await waitForText('Screen Intelligence Policy', 10_000); + await waitForText('Permissions', 10_000); + }); + + it('triggers capture test and reaches a stable UI outcome', async function () { + if (!isTauriDriver()) { + this.skip(); + return; + } + + if (!(await textExists('Screen Intelligence Policy'))) { + await navigateViaHash('/settings/screen-intelligence'); + await waitForText('Screen Intelligence Policy', 10_000); + } + + await clickButton('Expand', 10_000); + await waitForText('Capture Test', 10_000); + await clickButton('Test Capture', 10_000); + + const outcome = await waitForCaptureOutcome(); + stepLog('Capture test outcome', { outcome }); + + if (outcome === 'success') { + const hasPreviewImage = await browser.execute(() => { + const img = document.querySelector('img[alt="Capture test result"]'); + return !!img && !!img.getAttribute('src'); + }); + expect(hasPreviewImage).toBe(true); + expect((await textExists('windowed')) || (await textExists('fullscreen'))).toBe(true); + return; + } + + const hasFailureGuidance = + (await textExists('Failed')) || + (await textExists('screen recording permission is not granted')) || + (await textExists('screen capture is unsupported on this platform')) || + (await textExists('screen capture failed')); + if (!hasFailureGuidance) { + const tree = await dumpAccessibilityTree(); + stepLog('Capture failure outcome missing expected guidance', { tree: tree.slice(0, 4000) }); + } + expect(hasFailureGuidance).toBe(true); + }); }); diff --git a/src/openhuman/accessibility/capture.rs b/src/openhuman/accessibility/capture.rs index d25c5d46c..1fb62882c 100644 --- a/src/openhuman/accessibility/capture.rs +++ b/src/openhuman/accessibility/capture.rs @@ -1,9 +1,15 @@ //! Timestamp helper and screen capture via platform-native tools. use super::types::AppContext; +#[cfg(target_os = "macos")] +use super::{detect_permissions, PermissionState}; +#[cfg(target_os = "macos")] +use std::path::{Path, PathBuf}; /// Maximum screenshot size in bytes before downscaling is attempted. pub const MAX_SCREENSHOT_BYTES: usize = 1_500_000; +#[cfg(target_os = "macos")] +const SCREENSHOT_DOWNSCALE_WIDTH: &str = "1280"; /// Capture mode used for a screenshot. #[derive(Debug, Clone, PartialEq, Eq)] @@ -21,6 +27,77 @@ impl std::fmt::Display for CaptureMode { } } +fn capture_mode_for_context(context: Option<&AppContext>) -> CaptureMode { + match context.and_then(|ctx| ctx.bounds) { + Some(bounds) if bounds.width > 0 && bounds.height > 0 => CaptureMode::Windowed, + _ => CaptureMode::Fullscreen, + } +} + +#[cfg(target_os = "macos")] +fn log_capture_mode_decision(context: Option<&AppContext>, capture_mode: &CaptureMode) { + match (capture_mode, context.and_then(|ctx| ctx.bounds)) { + (CaptureMode::Windowed, Some(bounds)) => { + tracing::debug!( + "[accessibility] capture mode=windowed rect={},{},{},{} app={:?}", + bounds.x, + bounds.y, + bounds.width, + bounds.height, + context.and_then(|ctx| ctx.app_name.as_deref()) + ); + } + (CaptureMode::Windowed, None) => { + tracing::debug!( + "[accessibility] capture mode resolved to windowed without bounds; treating as fullscreen fallback" + ); + } + (CaptureMode::Fullscreen, Some(bounds)) => { + tracing::debug!( + "[accessibility] invalid bounds ({}x{}), falling back to fullscreen", + bounds.width, + bounds.height + ); + } + (CaptureMode::Fullscreen, None) => { + tracing::debug!( + "[accessibility] no window bounds available, falling back to fullscreen" + ); + } + } +} + +#[cfg(target_os = "macos")] +fn downscale_width_for_capture( + bytes_len: usize, + _capture_mode: &CaptureMode, +) -> Option<&'static str> { + (bytes_len > MAX_SCREENSHOT_BYTES).then_some(SCREENSHOT_DOWNSCALE_WIDTH) +} + +#[cfg(target_os = "macos")] +struct TemporaryScreenshotFile { + path: PathBuf, +} + +#[cfg(target_os = "macos")] +impl TemporaryScreenshotFile { + fn new(path: PathBuf) -> Self { + Self { path } + } + + fn path(&self) -> &Path { + &self.path + } +} + +#[cfg(target_os = "macos")] +impl Drop for TemporaryScreenshotFile { + fn drop(&mut self) { + let _ = std::fs::remove_file(&self.path); + } +} + pub fn capture_screen_image_ref_for_context( context: Option<&AppContext>, ) -> Result { @@ -29,89 +106,78 @@ pub fn capture_screen_image_ref_for_context( use base64::{engine::general_purpose::STANDARD as BASE64_STANDARD, Engine as _}; use uuid::Uuid; - let tmp_path = std::env::temp_dir().join(format!( + let tmp_file = TemporaryScreenshotFile::new(std::env::temp_dir().join(format!( "openhuman_screen_intelligence_{}.png", Uuid::new_v4() - )); + ))); - let bounds = context.and_then(|ctx| ctx.bounds); - - // Determine capture mode: windowed if we have valid bounds, fullscreen otherwise. - let capture_mode = match &bounds { - Some(b) if b.width > 0 && b.height > 0 => CaptureMode::Windowed, - Some(b) => { - tracing::debug!( - "[accessibility] invalid bounds ({}x{}), falling back to fullscreen", - b.width, - b.height - ); - CaptureMode::Fullscreen - } - None => { - tracing::debug!( - "[accessibility] no window bounds available, falling back to fullscreen" - ); - CaptureMode::Fullscreen - } - }; + let capture_mode = capture_mode_for_context(context); + log_capture_mode_decision(context, &capture_mode); let mut cmd = std::process::Command::new("screencapture"); cmd.arg("-x").arg("-t").arg("png"); if capture_mode == CaptureMode::Windowed { - let b = bounds.as_ref().unwrap(); + let b = &context + .and_then(|ctx| ctx.bounds) + .expect("windowed capture requires bounds"); let rect = format!("{},{},{},{}", b.x, b.y, b.width, b.height); - tracing::debug!( - "[accessibility] capture mode=windowed rect={rect} app={:?}", - context.and_then(|c| c.app_name.as_deref()) - ); cmd.arg("-R").arg(&rect); } else { tracing::debug!("[accessibility] capture mode=fullscreen (primary display)"); } - cmd.arg(&tmp_path); + cmd.arg(tmp_file.path()); - let status = cmd - .status() + let output = cmd + .output() .map_err(|e| format!("screencapture failed to start: {e}"))?; - if !status.success() { + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); + let permissions = detect_permissions(); tracing::debug!( - "[accessibility] screencapture exited with status: {:?}", - status.code() + "[accessibility] screencapture failed status={:?} stderr={:?} screen_recording={:?}", + output.status.code(), + stderr, + permissions.screen_recording ); - return Err("screencapture returned non-zero status".to_string()); + if permissions.screen_recording != PermissionState::Granted { + return Err("screen recording permission is not granted".to_string()); + } + if stderr.is_empty() { + return Err( + "screen capture failed: screencapture returned non-zero status".to_string(), + ); + } + return Err(format!("screen capture failed: {}", stderr)); } - let bytes = - std::fs::read(&tmp_path).map_err(|e| format!("failed to read screenshot: {e}"))?; + let bytes = std::fs::read(tmp_file.path()) + .map_err(|e| format!("failed to read captured screenshot: {e}"))?; tracing::debug!( "[accessibility] captured {} bytes (mode={})", bytes.len(), capture_mode ); - // If fullscreen capture is too large, try downscaling with sips. - if bytes.len() > MAX_SCREENSHOT_BYTES && capture_mode == CaptureMode::Fullscreen { + if let Some(width) = downscale_width_for_capture(bytes.len(), &capture_mode) { tracing::debug!( - "[accessibility] fullscreen capture {} bytes exceeds limit, downscaling with sips", - bytes.len() + "[accessibility] {} capture {} bytes exceeds limit, retrying downscale width={}", + capture_mode, + bytes.len(), + width ); - let sips_status = std::process::Command::new("sips") + let sips_output = std::process::Command::new("sips") .arg("--resampleWidth") - .arg("1280") - .arg(&tmp_path) - .status(); - match sips_status { - Ok(s) if s.success() => { - let resized = match std::fs::read(&tmp_path) { - Ok(bytes) => bytes, - Err(e) => { - let _ = std::fs::remove_file(&tmp_path); - return Err(format!("failed to read resized screenshot: {e}")); - } + .arg(width) + .arg(tmp_file.path()) + .output(); + match sips_output { + Ok(output) if output.status.success() => { + let resized = match std::fs::read(tmp_file.path()) { + Ok(resized) => resized, + Err(e) => return Err(format!("failed to read resized screenshot: {e}")), }; - let _ = std::fs::remove_file(&tmp_path); tracing::debug!("[accessibility] resized to {} bytes", resized.len()); if resized.len() > MAX_SCREENSHOT_BYTES { return Err( @@ -121,23 +187,23 @@ pub fn capture_screen_image_ref_for_context( let encoded = BASE64_STANDARD.encode(resized); return Ok(format!("data:image/png;base64,{encoded}")); } - Ok(s) => { - let _ = std::fs::remove_file(&tmp_path); - tracing::debug!("[accessibility] sips failed with status: {:?}", s.code()); + Ok(output) => { + tracing::debug!( + "[accessibility] sips failed status={:?} stderr={:?}", + output.status.code(), + String::from_utf8_lossy(&output.stderr).trim() + ); return Err( "captured screenshot exceeds size limit and downscale failed".to_string(), ); } Err(e) => { - let _ = std::fs::remove_file(&tmp_path); tracing::debug!("[accessibility] sips not available: {e}"); return Err("captured screenshot exceeds size limit".to_string()); } } } - let _ = std::fs::remove_file(&tmp_path); - if bytes.len() > MAX_SCREENSHOT_BYTES { return Err("captured screenshot exceeds size limit".to_string()); } @@ -152,3 +218,65 @@ pub fn capture_screen_image_ref_for_context( Err("screen capture is unsupported on this platform".to_string()) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::openhuman::accessibility::ElementBounds; + + #[test] + fn capture_mode_uses_window_bounds_when_positive() { + let context = AppContext { + app_name: Some("Code".to_string()), + window_title: Some("main.rs".to_string()), + bounds: Some(ElementBounds { + x: 10, + y: 20, + width: 1440, + height: 900, + }), + }; + + assert_eq!( + capture_mode_for_context(Some(&context)), + CaptureMode::Windowed + ); + } + + #[test] + fn capture_mode_falls_back_to_fullscreen_for_missing_or_invalid_bounds() { + let invalid_context = AppContext { + app_name: Some("Finder".to_string()), + window_title: Some("Desktop".to_string()), + bounds: Some(ElementBounds { + x: 0, + y: 0, + width: 0, + height: 900, + }), + }; + + assert_eq!( + capture_mode_for_context(Some(&invalid_context)), + CaptureMode::Fullscreen + ); + assert_eq!(capture_mode_for_context(None), CaptureMode::Fullscreen); + } + + #[cfg(target_os = "macos")] + #[test] + fn oversized_windowed_capture_is_eligible_for_downscale_retry() { + assert_eq!( + downscale_width_for_capture(MAX_SCREENSHOT_BYTES + 1, &CaptureMode::Windowed), + Some(SCREENSHOT_DOWNSCALE_WIDTH) + ); + assert_eq!( + downscale_width_for_capture(MAX_SCREENSHOT_BYTES + 1, &CaptureMode::Fullscreen), + Some(SCREENSHOT_DOWNSCALE_WIDTH) + ); + assert_eq!( + downscale_width_for_capture(MAX_SCREENSHOT_BYTES, &CaptureMode::Windowed), + None + ); + } +} diff --git a/src/openhuman/screen_intelligence/engine.rs b/src/openhuman/screen_intelligence/engine.rs index d89e64eef..d26506cdd 100644 --- a/src/openhuman/screen_intelligence/engine.rs +++ b/src/openhuman/screen_intelligence/engine.rs @@ -120,40 +120,49 @@ impl AccessibilityEngine { return Err("screen intelligence is macOS-only in V1".to_string()); } + let mut spawned_new_session = false; { let mut state = self.inner.lock().await; if state.session.is_some() { - return Ok(self.status().await.session); - } - state.permissions = detect_permissions(); - if state.permissions.screen_recording != PermissionState::Granted { - return Err("screen recording permission is not granted".to_string()); - } + tracing::debug!( + "[screen_intelligence] enable requested while session already active" + ); + } else { + state.permissions = detect_permissions(); + if state.permissions.screen_recording != PermissionState::Granted { + return Err("screen recording permission is not granted".to_string()); + } - let now = now_ms(); - state.features.screen_monitoring = true; - state.features.predictive_input = state.config.autocomplete_enabled; - state.session = Some(SessionRuntime { - started_at_ms: now, - expires_at_ms: i64::MAX, - ttl_secs: 0, - panic_hotkey: state.config.panic_stop_hotkey.clone(), - stop_reason: None, - last_capture_at_ms: None, - frames: VecDeque::new(), - last_context: None, - task: None, - vision_enabled: state.config.vision_enabled, - vision_state: "idle".to_string(), - vision_queue_depth: 0, - last_vision_at_ms: None, - last_vision_summary: None, - vision_summaries: VecDeque::new(), - vision_task: None, - vision_tx: None, - }); - state.last_event = Some("screen_intelligence_enabled".to_string()); - state.last_error = None; + let now = now_ms(); + state.features.screen_monitoring = true; + state.features.predictive_input = state.config.autocomplete_enabled; + state.session = Some(SessionRuntime { + started_at_ms: now, + expires_at_ms: i64::MAX, + ttl_secs: 0, + panic_hotkey: state.config.panic_stop_hotkey.clone(), + stop_reason: None, + last_capture_at_ms: None, + frames: VecDeque::new(), + last_context: None, + task: None, + vision_enabled: state.config.vision_enabled, + vision_state: "idle".to_string(), + vision_queue_depth: 0, + last_vision_at_ms: None, + last_vision_summary: None, + vision_summaries: VecDeque::new(), + vision_task: None, + vision_tx: None, + }); + state.last_event = Some("screen_intelligence_enabled".to_string()); + state.last_error = None; + spawned_new_session = true; + } + } + + if !spawned_new_session { + return Ok(self.status().await.session); } let (vision_tx, vision_rx) = tokio::sync::mpsc::unbounded_channel::(); @@ -937,3 +946,52 @@ impl AccessibilityEngine { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[cfg(target_os = "macos")] + #[tokio::test] + async fn enable_with_existing_session_does_not_deadlock() { + let engine = Arc::new(AccessibilityEngine { + inner: Mutex::new(EngineState::new(ScreenIntelligenceConfig { + enabled: true, + ..Default::default() + })), + }); + + { + let mut state = engine.inner.lock().await; + state.session = Some(SessionRuntime { + started_at_ms: now_ms(), + expires_at_ms: i64::MAX, + ttl_secs: 300, + panic_hotkey: state.config.panic_stop_hotkey.clone(), + stop_reason: None, + last_capture_at_ms: None, + frames: VecDeque::new(), + last_context: None, + task: None, + vision_enabled: state.config.vision_enabled, + vision_state: "idle".to_string(), + vision_queue_depth: 0, + last_vision_at_ms: None, + last_vision_summary: None, + vision_summaries: VecDeque::new(), + vision_task: None, + vision_tx: None, + }); + } + + let result = time::timeout(Duration::from_millis(250), engine.enable()).await; + assert!( + result.is_ok(), + "enable should not deadlock with an active session" + ); + assert!( + result.unwrap().is_ok(), + "enable should return the existing session status" + ); + } +} diff --git a/src/openhuman/screen_intelligence/tests.rs b/src/openhuman/screen_intelligence/tests.rs index e3c4c87ff..14c74b4d0 100644 --- a/src/openhuman/screen_intelligence/tests.rs +++ b/src/openhuman/screen_intelligence/tests.rs @@ -516,6 +516,29 @@ async fn capture_test_returns_diagnostics() { if !cfg!(target_os = "macos") { assert!(!result.ok, "should fail on non-macOS"); - assert!(result.error.is_some()); + assert_eq!( + result.error.as_deref(), + Some("screen capture is unsupported on this platform") + ); } } + +#[tokio::test] +async fn capture_now_without_session_is_rejected_without_hanging() { + let engine = Arc::new(AccessibilityEngine { + inner: Mutex::new(EngineState::new(ScreenIntelligenceConfig::default())), + }); + + let result = engine + .capture_now() + .await + .expect("capture_now should not error"); + assert!( + !result.accepted, + "capture_now should be rejected without a session" + ); + assert!( + result.frame.is_none(), + "capture_now should not produce a frame without a session" + ); +} diff --git a/tests/json_rpc_e2e.rs b/tests/json_rpc_e2e.rs index a7ba391c7..2ca3a444f 100644 --- a/tests/json_rpc_e2e.rs +++ b/tests/json_rpc_e2e.rs @@ -685,6 +685,88 @@ async fn json_rpc_rejects_non_object_params_with_clear_error() { rpc_join.abort(); } +#[tokio::test] +async fn json_rpc_screen_intelligence_capture_test_returns_stable_shape() { + let _env_lock = json_rpc_e2e_env_lock(); + let tmp = tempdir().expect("tempdir"); + let home = tmp.path(); + let openhuman_home = home.join(".openhuman"); + + let _home_guard = EnvVarGuard::set_to_path("HOME", home); + let _workspace_guard = EnvVarGuard::unset("OPENHUMAN_WORKSPACE"); + let _backend_url_guard = EnvVarGuard::unset("BACKEND_URL"); + let _vite_backend_guard = EnvVarGuard::unset("VITE_BACKEND_URL"); + + let (mock_addr, mock_join) = serve_on_ephemeral(mock_upstream_router()).await; + let mock_origin = format!("http://{}", mock_addr); + write_min_config(&openhuman_home, &mock_origin); + + let (rpc_addr, rpc_join) = serve_on_ephemeral(build_core_http_router(false)).await; + let rpc_base = format!("http://{}", rpc_addr); + tokio::time::sleep(Duration::from_millis(100)).await; + + let capture = post_json_rpc( + &rpc_base, + 1002, + "openhuman.screen_intelligence_capture_test", + json!({}), + ) + .await; + let capture_outer = assert_no_jsonrpc_error(&capture, "screen_intelligence_capture_test"); + let capture_result = capture_outer.get("result").unwrap_or(capture_outer); + + assert!( + capture_result.get("ok").and_then(Value::as_bool).is_some(), + "expected bool ok field: {capture_result}" + ); + assert!( + matches!( + capture_result.get("capture_mode").and_then(Value::as_str), + Some("windowed" | "fullscreen") + ), + "expected capture_mode field: {capture_result}" + ); + assert!( + capture_result + .get("timing_ms") + .and_then(Value::as_u64) + .is_some(), + "expected timing_ms field: {capture_result}" + ); + + let ok = capture_result + .get("ok") + .and_then(Value::as_bool) + .expect("ok should be bool"); + let image_ref = capture_result.get("image_ref").and_then(Value::as_str); + let error = capture_result.get("error").and_then(Value::as_str); + + if ok { + assert!( + image_ref + .map(|value| value.starts_with("data:image/png;base64,")) + .unwrap_or(false), + "successful capture should include a PNG data URL: {capture_result}" + ); + assert!( + error.is_none(), + "successful capture should not include an error" + ); + } else { + assert!( + image_ref.is_none(), + "failed capture should not include image data" + ); + assert!( + error.is_some(), + "failed capture should include an error message" + ); + } + + mock_join.abort(); + rpc_join.abort(); +} + // --------------------------------------------------------------------------- // Skills registry E2E: fetch, search, install, list, uninstall // ---------------------------------------------------------------------------