From 279506a3ea9232551192fa13db2af227f22ded5e Mon Sep 17 00:00:00 2001 From: Harivansh Rathi Date: Wed, 25 Mar 2026 18:21:56 -0400 Subject: [PATCH] Stabilize deskctl runtime foundation Co-authored-by: Codex --- README.md | 19 +- SKILL.md | 3 +- .../stabilize-v0-2-foundation/tasks.md | 18 +- src/backend/mod.rs | 35 +- src/backend/x11.rs | 69 +--- src/cli/connection.rs | 261 +++++++++----- src/cli/mod.rs | 9 +- src/core/doctor.rs | 239 +++++++++++++ src/core/mod.rs | 2 + src/core/paths.rs | 29 ++ src/core/refs.rs | 155 ++++++++- src/core/session.rs | 4 +- src/core/types.rs | 2 +- src/daemon/handler.rs | 322 +++++++++++------- src/daemon/mod.rs | 8 +- src/daemon/state.rs | 6 +- src/main.rs | 2 + src/test_support.rs | 150 ++++++++ 18 files changed, 1029 insertions(+), 304 deletions(-) create mode 100644 src/core/doctor.rs create mode 100644 src/core/paths.rs create mode 100644 src/test_support.rs diff --git a/README.md b/README.md index 9167596..a1405d7 100644 --- a/README.md +++ b/README.md @@ -38,6 +38,9 @@ At the moment there are no extra native build dependencies beyond a Rust toolcha ## Quick Start ```bash +# Diagnose the environment first +deskctl doctor + # See the desktop deskctl snapshot @@ -70,9 +73,21 @@ For deskctl to be fully functional on a fresh VM you still need: - a window manager or desktop environment that exposes standard EWMH properties such as `_NET_CLIENT_LIST_STACKING` and `_NET_ACTIVE_WINDOW` - an X server with the extensions needed for input simulation and screen metadata, which is standard on normal desktop X11 setups -## Wayland Support +If setup fails, run: -Coming soon. The trait-based backend design means adding Hyprland/Wayland support is a single trait implementation with zero refactoring of the core which is good. +```bash +deskctl doctor +``` + +## Contract Notes + +- `@wN` refs are short-lived handles assigned by `snapshot` and `list-windows` +- `--json` output includes a stable `window_id` for programmatic targeting within the current daemon session +- `list-windows` is a cheap read-only operation and does not capture or write a screenshot + +## Support Boundary + +`deskctl` supports Linux X11 in this phase. Wayland and Hyprland are explicitly out of scope for the current runtime contract. ## Acknowledgements diff --git a/SKILL.md b/SKILL.md index 0407b0f..50a8f00 100644 --- a/SKILL.md +++ b/SKILL.md @@ -60,6 +60,7 @@ deskctl resize-window @w1 800 600 # Resize window ### Utilities ```bash +deskctl doctor # Diagnose X11, screenshot, and daemon health deskctl get-screen-size # Screen resolution deskctl get-mouse-position # Current cursor position deskctl launch firefox # Launch an application @@ -86,7 +87,7 @@ After `snapshot` or `list-windows`, windows are assigned short refs: - `@w1` is the topmost (usually focused) window - `@w2`, `@w3`, etc. follow z-order (front to back) - Refs reset on each `snapshot` call -- Use `--json` to see stable `xcb_id` for programmatic tracking +- Use `--json` to see stable `window_id` values for programmatic tracking within the current daemon session ## Example Agent Workflow diff --git a/openspec/changes/stabilize-v0-2-foundation/tasks.md b/openspec/changes/stabilize-v0-2-foundation/tasks.md index cf2918b..85220a2 100644 --- a/openspec/changes/stabilize-v0-2-foundation/tasks.md +++ b/openspec/changes/stabilize-v0-2-foundation/tasks.md @@ -1,17 +1,17 @@ ## 1. Contract and protocol stabilization -- [ ] 1.1 Define the public `window_id` contract in shared types/protocol code and remove backend-handle assumptions from public runtime responses -- [ ] 1.2 Update daemon state and selector resolution to map `window_id` and refs to internal backend handles without exposing X11-specific IDs publicly -- [ ] 1.3 Update CLI text and JSON response handling to use the new public identity consistently +- [x] 1.1 Define the public `window_id` contract in shared types/protocol code and remove backend-handle assumptions from public runtime responses +- [x] 1.2 Update daemon state and selector resolution to map `window_id` and refs to internal backend handles without exposing X11-specific IDs publicly +- [x] 1.3 Update CLI text and JSON response handling to use the new public identity consistently ## 2. Cheap reads and diagnostics -- [ ] 2.1 Split backend window enumeration from screenshot capture and route `list-windows` through a read-only path with no screenshot side effects -- [ ] 2.2 Add a daemon-independent `deskctl doctor` command that probes X11 environment setup, socket health, window enumeration, and screenshot viability -- [ ] 2.3 Harden daemon startup and reconnect behavior with stale socket cleanup, health probing, and clearer failure messages +- [x] 2.1 Split backend window enumeration from screenshot capture and route `list-windows` through a read-only path with no screenshot side effects +- [x] 2.2 Add a daemon-independent `deskctl doctor` command that probes X11 environment setup, socket health, window enumeration, and screenshot viability +- [x] 2.3 Harden daemon startup and reconnect behavior with stale socket cleanup, health probing, and clearer failure messages ## 3. Validation and follow-through -- [ ] 3.1 Add unit tests for selector parsing, public ID resolution, and read-only behavior -- [ ] 3.2 Add X11 integration coverage for `doctor`, `list-windows`, and daemon recovery behavior -- [ ] 3.3 Update user-facing docs and examples to reflect the new contract, `doctor`, and the explicit X11 support boundary +- [x] 3.1 Add unit tests for selector parsing, public ID resolution, and read-only behavior +- [x] 3.2 Add X11 integration coverage for `doctor`, `list-windows`, and daemon recovery behavior +- [x] 3.3 Update user-facing docs and examples to reflect the new contract, `doctor`, and the explicit X11 support boundary diff --git a/src/backend/mod.rs b/src/backend/mod.rs index 4d1767a..53ea405 100644 --- a/src/backend/mod.rs +++ b/src/backend/mod.rs @@ -1,25 +1,41 @@ pub mod annotate; pub mod x11; -use crate::core::types::Snapshot; use anyhow::Result; +use image::RgbaImage; + +#[derive(Debug, Clone)] +pub struct BackendWindow { + pub native_id: u32, + pub title: String, + pub app_name: String, + pub x: i32, + pub y: i32, + pub width: u32, + pub height: u32, + pub focused: bool, + pub minimized: bool, +} #[allow(dead_code)] pub trait DesktopBackend: Send { - /// Capture a screenshot and return a z-ordered window tree with @wN refs. - fn snapshot(&mut self, annotate: bool) -> Result; + /// Collect z-ordered windows for read-only queries and targeting. + fn list_windows(&mut self) -> Result>; - /// Focus a window by its X11 window ID. - fn focus_window(&mut self, xcb_id: u32) -> Result<()>; + /// Capture the current desktop image without writing it to disk. + fn capture_screenshot(&mut self) -> Result; + + /// Focus a window by its backend-native window handle. + fn focus_window(&mut self, native_id: u32) -> Result<()>; /// Move a window to absolute coordinates. - fn move_window(&mut self, xcb_id: u32, x: i32, y: i32) -> Result<()>; + fn move_window(&mut self, native_id: u32, x: i32, y: i32) -> Result<()>; /// Resize a window. - fn resize_window(&mut self, xcb_id: u32, w: u32, h: u32) -> Result<()>; + fn resize_window(&mut self, native_id: u32, w: u32, h: u32) -> Result<()>; /// Close a window gracefully. - fn close_window(&mut self, xcb_id: u32) -> Result<()>; + fn close_window(&mut self, native_id: u32) -> Result<()>; /// Click at absolute coordinates. fn click(&mut self, x: i32, y: i32) -> Result<()>; @@ -51,9 +67,6 @@ pub trait DesktopBackend: Send { /// Get the current mouse position. fn mouse_position(&self) -> Result<(i32, i32)>; - /// Take a screenshot and save to a path (no window tree). - fn screenshot(&mut self, path: &str, annotate: bool) -> Result; - /// Launch an application. fn launch(&self, command: &str, args: &[String]) -> Result; } diff --git a/src/backend/x11.rs b/src/backend/x11.rs index 9502281..721b96d 100644 --- a/src/backend/x11.rs +++ b/src/backend/x11.rs @@ -9,8 +9,7 @@ use x11rb::protocol::xproto::{ }; use x11rb::rust_connection::RustConnection; -use super::annotate::annotate_screenshot; -use crate::core::types::{Snapshot, WindowInfo}; +use crate::backend::BackendWindow; struct Atoms { client_list_stacking: Atom, @@ -71,10 +70,9 @@ impl X11Backend { Ok(windows) } - fn collect_window_infos(&self) -> Result> { + fn collect_window_infos(&self) -> Result> { let active_window = self.active_window()?; let mut window_infos = Vec::new(); - let mut ref_counter = 1usize; for window in self.stacked_windows()? { let title = self.window_title(window).unwrap_or_default(); @@ -89,9 +87,8 @@ impl X11Backend { }; let minimized = self.window_is_minimized(window).unwrap_or(false); - window_infos.push(WindowInfo { - ref_id: format!("w{ref_counter}"), - xcb_id: window, + window_infos.push(BackendWindow { + native_id: window, title, app_name, x, @@ -101,7 +98,6 @@ impl X11Backend { focused: active_window == Some(window), minimized, }); - ref_counter += 1; } Ok(window_infos) @@ -231,32 +227,15 @@ impl X11Backend { } impl super::DesktopBackend for X11Backend { - fn snapshot(&mut self, annotate: bool) -> Result { - let window_infos = self.collect_window_infos()?; - let mut image = self.capture_root_image()?; - - // Annotate if requested - draw bounding boxes and @wN labels - if annotate { - annotate_screenshot(&mut image, &window_infos); - } - - // Save screenshot - let timestamp = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap_or_default() - .as_millis(); - let screenshot_path = format!("/tmp/deskctl-{timestamp}.png"); - image - .save(&screenshot_path) - .context("Failed to save screenshot")?; - - Ok(Snapshot { - screenshot: screenshot_path, - windows: window_infos, - }) + fn list_windows(&mut self) -> Result> { + self.collect_window_infos() } - fn focus_window(&mut self, xcb_id: u32) -> Result<()> { + fn capture_screenshot(&mut self) -> Result { + self.capture_root_image() + } + + fn focus_window(&mut self, native_id: u32) -> Result<()> { // Use _NET_ACTIVE_WINDOW client message (avoids focus-stealing prevention) let net_active = self .conn @@ -269,7 +248,7 @@ impl super::DesktopBackend for X11Backend { response_type: x11rb::protocol::xproto::CLIENT_MESSAGE_EVENT, format: 32, sequence: 0, - window: xcb_id, + window: native_id, type_: net_active, data: ClientMessageData::from([ 2u32, 0, 0, 0, 0, // source=2 (pager), timestamp=0, currently_active=0 @@ -288,25 +267,25 @@ impl super::DesktopBackend for X11Backend { Ok(()) } - fn move_window(&mut self, xcb_id: u32, x: i32, y: i32) -> Result<()> { + fn move_window(&mut self, native_id: u32, x: i32, y: i32) -> Result<()> { self.conn - .configure_window(xcb_id, &ConfigureWindowAux::new().x(x).y(y))?; + .configure_window(native_id, &ConfigureWindowAux::new().x(x).y(y))?; self.conn .flush() .context("Failed to flush X11 connection")?; Ok(()) } - fn resize_window(&mut self, xcb_id: u32, w: u32, h: u32) -> Result<()> { + fn resize_window(&mut self, native_id: u32, w: u32, h: u32) -> Result<()> { self.conn - .configure_window(xcb_id, &ConfigureWindowAux::new().width(w).height(h))?; + .configure_window(native_id, &ConfigureWindowAux::new().width(w).height(h))?; self.conn .flush() .context("Failed to flush X11 connection")?; Ok(()) } - fn close_window(&mut self, xcb_id: u32) -> Result<()> { + fn close_window(&mut self, native_id: u32) -> Result<()> { // Use _NET_CLOSE_WINDOW for graceful close (respects WM protocols) let net_close = self .conn @@ -319,7 +298,7 @@ impl super::DesktopBackend for X11Backend { response_type: x11rb::protocol::xproto::CLIENT_MESSAGE_EVENT, format: 32, sequence: 0, - window: xcb_id, + window: native_id, type_: net_close, data: ClientMessageData::from([ 0u32, 2, 0, 0, 0, // timestamp=0, source=2 (pager) @@ -463,18 +442,6 @@ impl super::DesktopBackend for X11Backend { Ok((reply.root_x as i32, reply.root_y as i32)) } - fn screenshot(&mut self, path: &str, annotate: bool) -> Result { - let mut image = self.capture_root_image()?; - - if annotate { - let window_infos = self.collect_window_infos()?; - annotate_screenshot(&mut image, &window_infos); - } - - image.save(path).context("Failed to save screenshot")?; - Ok(path.to_string()) - } - fn launch(&self, command: &str, args: &[String]) -> Result { let child = std::process::Command::new(command) .args(args) diff --git a/src/cli/connection.rs b/src/cli/connection.rs index f960ee3..1237a85 100644 --- a/src/cli/connection.rs +++ b/src/cli/connection.rs @@ -1,41 +1,56 @@ use std::io::{BufRead, BufReader, Write}; use std::os::unix::net::UnixStream; use std::os::unix::process::CommandExt; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use std::thread; use std::time::Duration; -use anyhow::{bail, Context, Result}; +use anyhow::{Context, Result, bail}; use crate::cli::GlobalOpts; +use crate::core::doctor::{DoctorReport, run as run_doctor_report}; +use crate::core::paths::{pid_path_for_session, socket_dir, socket_path_for_session}; use crate::core::protocol::{Request, Response}; -fn socket_dir() -> PathBuf { - if let Ok(dir) = std::env::var("DESKCTL_SOCKET_DIR") { - return PathBuf::from(dir); - } - if let Ok(runtime) = std::env::var("XDG_RUNTIME_DIR") { - return PathBuf::from(runtime).join("deskctl"); - } - dirs::home_dir() - .unwrap_or_else(|| PathBuf::from("/tmp")) - .join(".deskctl") -} - fn socket_path(opts: &GlobalOpts) -> PathBuf { if let Some(ref path) = opts.socket { return path.clone(); } - socket_dir().join(format!("{}.sock", opts.session)) + socket_path_for_session(&opts.session) } fn pid_path(opts: &GlobalOpts) -> PathBuf { - socket_dir().join(format!("{}.pid", opts.session)) + pid_path_for_session(&opts.session) } -fn try_connect(opts: &GlobalOpts) -> Option { - UnixStream::connect(socket_path(opts)).ok() +fn connect_socket(path: &Path) -> Result { + UnixStream::connect(path).with_context(|| format!("Failed to connect to {}", path.display())) +} + +fn is_stale_socket_error(error: &std::io::Error) -> bool { + matches!( + error.kind(), + std::io::ErrorKind::ConnectionRefused | std::io::ErrorKind::NotFound + ) +} + +fn cleanup_stale_socket(opts: &GlobalOpts) -> Result { + let path = socket_path(opts); + if !path.exists() { + return Ok(false); + } + + match UnixStream::connect(&path) { + Ok(_) => Ok(false), + Err(error) if is_stale_socket_error(&error) => { + std::fs::remove_file(&path) + .with_context(|| format!("Failed to remove stale socket {}", path.display()))?; + Ok(true) + } + Err(error) => Err(error) + .with_context(|| format!("Failed to inspect daemon socket {}", path.display())), + } } fn spawn_daemon(opts: &GlobalOpts) -> Result<()> { @@ -51,9 +66,8 @@ fn spawn_daemon(opts: &GlobalOpts) -> Result<()> { .env("DESKCTL_PID_PATH", pid_path(opts)) .stdin(Stdio::null()) .stdout(Stdio::null()) - .stderr(Stdio::piped()); + .stderr(Stdio::null()); - // Detach the daemon process on Unix unsafe { cmd.pre_exec(|| { libc::setsid(); @@ -65,82 +79,120 @@ fn spawn_daemon(opts: &GlobalOpts) -> Result<()> { Ok(()) } +fn send_request_over_stream(mut stream: UnixStream, request: &Request) -> Result { + stream.set_read_timeout(Some(Duration::from_secs(30)))?; + stream.set_write_timeout(Some(Duration::from_secs(5)))?; + + let json = serde_json::to_string(request)?; + writeln!(stream, "{json}")?; + stream.flush()?; + + let mut reader = BufReader::new(&stream); + let mut line = String::new(); + reader.read_line(&mut line)?; + + serde_json::from_str(line.trim()).context("Failed to parse daemon response") +} + +fn ping_daemon(opts: &GlobalOpts) -> Result<()> { + let response = send_request_over_stream(connect_socket(&socket_path(opts))?, &Request::new("ping"))?; + if response.success { + Ok(()) + } else { + bail!( + "{}", + response + .error + .unwrap_or_else(|| "Daemon health probe failed".to_string()) + ); + } +} + fn ensure_daemon(opts: &GlobalOpts) -> Result { - // Try connecting first - if let Some(stream) = try_connect(opts) { - return Ok(stream); + if ping_daemon(opts).is_ok() { + return connect_socket(&socket_path(opts)); + } + + let removed_stale_socket = cleanup_stale_socket(opts)?; + if removed_stale_socket && ping_daemon(opts).is_ok() { + return connect_socket(&socket_path(opts)); } - // Spawn daemon spawn_daemon(opts)?; - // Retry with backoff let max_retries = 20; let base_delay = Duration::from_millis(50); - for i in 0..max_retries { - thread::sleep(base_delay * (i + 1).min(4)); - if let Some(stream) = try_connect(opts) { - return Ok(stream); + for attempt in 0..max_retries { + thread::sleep(base_delay * (attempt + 1).min(4)); + if ping_daemon(opts).is_ok() { + return connect_socket(&socket_path(opts)); } } bail!( - "Failed to connect to daemon after {} retries.\n\ - Socket path: {}", + "Failed to start a healthy daemon after {} retries.\nSocket path: {}", max_retries, socket_path(opts).display() ); } pub fn send_command(opts: &GlobalOpts, request: &Request) -> Result { - let mut stream = ensure_daemon(opts)?; - stream.set_read_timeout(Some(Duration::from_secs(30)))?; - stream.set_write_timeout(Some(Duration::from_secs(5)))?; - - // Send NDJSON request - let json = serde_json::to_string(request)?; - writeln!(stream, "{json}")?; - stream.flush()?; - - // Read NDJSON response - let mut reader = BufReader::new(&stream); - let mut line = String::new(); - reader.read_line(&mut line)?; - - let response: Response = - serde_json::from_str(line.trim()).context("Failed to parse daemon response")?; - - Ok(response) + send_request_over_stream(ensure_daemon(opts)?, request) } -pub fn start_daemon(opts: &GlobalOpts) -> Result<()> { - if try_connect(opts).is_some() { - println!("Daemon already running ({})", socket_path(opts).display()); - return Ok(()); - } - spawn_daemon(opts)?; - // Wait briefly and verify - thread::sleep(Duration::from_millis(200)); - if try_connect(opts).is_some() { - println!("Daemon started ({})", socket_path(opts).display()); - } else { - bail!("Daemon failed to start"); +pub fn run_doctor(opts: &GlobalOpts) -> Result<()> { + let report = run_doctor_report(&socket_path(opts)); + print_doctor_report(&report, opts.json)?; + if !report.healthy { + std::process::exit(1); } Ok(()) } -pub fn stop_daemon(opts: &GlobalOpts) -> Result<()> { - match try_connect(opts) { - Some(mut stream) => { - let req = Request::new("shutdown"); - let json = serde_json::to_string(&req)?; - writeln!(stream, "{json}")?; - stream.flush()?; - println!("Daemon stopped"); +pub fn start_daemon(opts: &GlobalOpts) -> Result<()> { + if ping_daemon(opts).is_ok() { + println!("Daemon already running ({})", socket_path(opts).display()); + return Ok(()); + } + + if cleanup_stale_socket(opts)? { + println!("Removed stale socket: {}", socket_path(opts).display()); + } + + spawn_daemon(opts)?; + + let max_retries = 20; + for attempt in 0..max_retries { + thread::sleep(Duration::from_millis(50 * (attempt + 1).min(4) as u64)); + if ping_daemon(opts).is_ok() { + println!("Daemon started ({})", socket_path(opts).display()); + return Ok(()); } - None => { - // Try to clean up stale socket - let path = socket_path(opts); + } + + bail!( + "Daemon failed to become healthy.\nSocket path: {}", + socket_path(opts).display() + ); +} + +pub fn stop_daemon(opts: &GlobalOpts) -> Result<()> { + let path = socket_path(opts); + match UnixStream::connect(&path) { + Ok(stream) => { + let response = send_request_over_stream(stream, &Request::new("shutdown"))?; + if response.success { + println!("Daemon stopped"); + } else { + bail!( + "{}", + response + .error + .unwrap_or_else(|| "Failed to stop daemon".to_string()) + ); + } + } + Err(error) if is_stale_socket_error(&error) => { if path.exists() { std::fs::remove_file(&path)?; println!("Removed stale socket: {}", path.display()); @@ -148,15 +200,72 @@ pub fn stop_daemon(opts: &GlobalOpts) -> Result<()> { println!("Daemon not running"); } } + Err(error) => { + return Err(error) + .with_context(|| format!("Failed to inspect daemon socket {}", path.display())); + } } Ok(()) } pub fn daemon_status(opts: &GlobalOpts) -> Result<()> { - if try_connect(opts).is_some() { - println!("Daemon running ({})", socket_path(opts).display()); - } else { - println!("Daemon not running"); + let path = socket_path(opts); + match ping_daemon(opts) { + Ok(()) => println!("Daemon running ({})", path.display()), + Err(_) if path.exists() => println!("Daemon socket exists but is unhealthy ({})", path.display()), + Err(_) => println!("Daemon not running"), } Ok(()) } + +fn print_doctor_report(report: &DoctorReport, json_output: bool) -> Result<()> { + if json_output { + println!("{}", serde_json::to_string_pretty(report)?); + return Ok(()); + } + + println!( + "deskctl doctor: {}", + if report.healthy { "healthy" } else { "issues found" } + ); + for check in &report.checks { + let status = if check.ok { "OK" } else { "FAIL" }; + println!("[{status}] {}: {}", check.name, check.details); + if let Some(fix) = &check.fix { + println!(" fix: {fix}"); + } + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::{cleanup_stale_socket, socket_path}; + use crate::cli::GlobalOpts; + use std::time::{SystemTime, UNIX_EPOCH}; + + #[test] + fn cleanup_stale_socket_removes_refused_socket_path() { + let temp = std::env::temp_dir().join(format!( + "deskctl-test-{}", + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + std::fs::create_dir_all(&temp).unwrap(); + let opts = GlobalOpts { + socket: Some(temp.join("stale.sock")), + session: "test".to_string(), + json: false, + }; + + let listener = std::os::unix::net::UnixListener::bind(socket_path(&opts)).unwrap(); + drop(listener); + + assert!(cleanup_stale_socket(&opts).unwrap()); + assert!(!socket_path(&opts).exists()); + + let _ = std::fs::remove_dir_all(&temp); + } +} diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 51891c0..3500522 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -100,6 +100,8 @@ pub enum Command { GetScreenSize, /// Get current mouse position GetMousePosition, + /// Diagnose X11 runtime, screenshot, and daemon health + Doctor, /// Take a screenshot without window tree Screenshot { /// Save path (default: /tmp/deskctl-{timestamp}.png) @@ -179,6 +181,10 @@ pub fn run() -> Result<()> { }; } + if let Command::Doctor = app.command { + return connection::run_doctor(&app.global); + } + // All other commands need a daemon connection let request = build_request(&app.command)?; let response = connection::send_command(&app.global, &request)?; @@ -237,6 +243,7 @@ fn build_request(cmd: &Command) -> Result { Command::ListWindows => Request::new("list-windows"), Command::GetScreenSize => Request::new("get-screen-size"), Command::GetMousePosition => Request::new("get-mouse-position"), + Command::Doctor => unreachable!(), Command::Screenshot { path, annotate } => { let mut req = Request::new("screenshot").with_extra("annotate", json!(annotate)); if let Some(p) = path { @@ -261,7 +268,7 @@ fn print_response(cmd: &Command, response: &Response) -> Result<()> { } if let Some(ref data) = response.data { // For snapshot, print compact text format - if matches!(cmd, Command::Snapshot { .. }) { + if matches!(cmd, Command::Snapshot { .. } | Command::ListWindows) { if let Some(screenshot) = data.get("screenshot").and_then(|v| v.as_str()) { println!("Screenshot: {screenshot}"); } diff --git a/src/core/doctor.rs b/src/core/doctor.rs new file mode 100644 index 0000000..3d240c3 --- /dev/null +++ b/src/core/doctor.rs @@ -0,0 +1,239 @@ +use std::io::{BufRead, BufReader, Write}; +use std::os::unix::net::UnixStream; +use std::path::Path; +use std::time::Duration; + +use anyhow::Result; +use serde::Serialize; + +use crate::backend::{x11::X11Backend, DesktopBackend}; +use crate::core::protocol::{Request, Response}; +use crate::core::session::detect_session; + +#[derive(Debug, Serialize)] +pub struct DoctorReport { + pub healthy: bool, + pub checks: Vec, +} + +#[derive(Debug, Serialize)] +pub struct DoctorCheck { + pub name: String, + pub ok: bool, + pub details: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub fix: Option, +} + +pub fn run(socket_path: &Path) -> DoctorReport { + let mut checks = Vec::new(); + + let display = std::env::var("DISPLAY").ok(); + checks.push(match display { + Some(ref value) if !value.is_empty() => check_ok("display", format!("DISPLAY={value}")), + _ => check_fail( + "display", + "DISPLAY is not set".to_string(), + "Export DISPLAY to point at the active X11 server.".to_string(), + ), + }); + + checks.push(match detect_session() { + Ok(_) => check_ok("session", "X11 session detected".to_string()), + Err(error) => check_fail( + "session", + error.to_string(), + "Run deskctl inside an X11 session. Wayland is not supported in this phase." + .to_string(), + ), + }); + + let mut backend = match X11Backend::new() { + Ok(backend) => { + checks.push(check_ok( + "backend", + "Connected to the X11 backend successfully".to_string(), + )); + Some(backend) + } + Err(error) => { + checks.push(check_fail( + "backend", + error.to_string(), + "Ensure the X server is reachable and the current session can access it." + .to_string(), + )); + None + } + }; + + if let Some(backend) = backend.as_mut() { + checks.push(match backend.list_windows() { + Ok(windows) => check_ok( + "window-enumeration", + format!("Enumerated {} visible windows", windows.len()), + ), + Err(error) => check_fail( + "window-enumeration", + error.to_string(), + "Verify the desktop session exposes EWMH window metadata and the X11 connection is healthy." + .to_string(), + ), + }); + + checks.push(match backend.capture_screenshot() { + Ok(image) => check_ok( + "screenshot", + format!("Captured {}x{} desktop image", image.width(), image.height()), + ), + Err(error) => check_fail( + "screenshot", + error.to_string(), + "Verify the X11 session permits desktop capture on the active display." + .to_string(), + ), + }); + } else { + checks.push(check_fail( + "window-enumeration", + "Skipped because backend initialization failed".to_string(), + "Fix the X11 backend error before retrying.".to_string(), + )); + checks.push(check_fail( + "screenshot", + "Skipped because backend initialization failed".to_string(), + "Fix the X11 backend error before retrying.".to_string(), + )); + } + + checks.push(check_socket_dir(socket_path)); + checks.push(check_daemon_socket(socket_path)); + + let healthy = checks.iter().all(|check| check.ok); + DoctorReport { healthy, checks } +} + +fn check_socket_dir(socket_path: &Path) -> DoctorCheck { + let Some(socket_dir) = socket_path.parent() else { + return check_fail( + "socket-dir", + format!("Socket path {} has no parent directory", socket_path.display()), + "Use a socket path inside a writable directory.".to_string(), + ); + }; + + match std::fs::create_dir_all(socket_dir) { + Ok(()) => check_ok( + "socket-dir", + format!("Socket directory is ready at {}", socket_dir.display()), + ), + Err(error) => check_fail( + "socket-dir", + error.to_string(), + format!("Ensure {} exists and is writable.", socket_dir.display()), + ), + } +} + +fn check_daemon_socket(socket_path: &Path) -> DoctorCheck { + if !socket_path.exists() { + return check_ok( + "daemon-socket", + format!("No stale socket found at {}", socket_path.display()), + ); + } + + match ping_socket(socket_path) { + Ok(()) => check_ok( + "daemon-socket", + format!("Daemon is healthy at {}", socket_path.display()), + ), + Err(error) => check_fail( + "daemon-socket", + error.to_string(), + format!( + "Remove the stale socket at {} or run `deskctl daemon stop`.", + socket_path.display() + ), + ), + } +} + +fn ping_socket(socket_path: &Path) -> Result<()> { + let mut stream = UnixStream::connect(socket_path)?; + stream.set_read_timeout(Some(Duration::from_secs(1)))?; + stream.set_write_timeout(Some(Duration::from_secs(1)))?; + + let request = Request::new("ping"); + let json = serde_json::to_string(&request)?; + writeln!(stream, "{json}")?; + stream.flush()?; + + let mut reader = BufReader::new(&stream); + let mut line = String::new(); + reader.read_line(&mut line)?; + let response: Response = serde_json::from_str(line.trim())?; + + if response.success { + Ok(()) + } else { + anyhow::bail!( + "{}", + response + .error + .unwrap_or_else(|| "Daemon health probe failed".to_string()) + ) + } +} + +fn check_ok(name: &str, details: String) -> DoctorCheck { + DoctorCheck { + name: name.to_string(), + ok: true, + details, + fix: None, + } +} + +fn check_fail(name: &str, details: String, fix: String) -> DoctorCheck { + DoctorCheck { + name: name.to_string(), + ok: false, + details, + fix: Some(fix), + } +} + +#[cfg(all(test, target_os = "linux"))] +mod tests { + use super::run; + use crate::test_support::{X11TestEnv, env_lock}; + + #[test] + fn doctor_reports_healthy_x11_environment_under_xvfb() { + let _guard = env_lock().lock().unwrap(); + let Some(env) = X11TestEnv::new().unwrap() else { + eprintln!("Skipping Xvfb-dependent doctor test"); + return; + }; + env.create_window("deskctl doctor test", "DeskctlDoctor").unwrap(); + + let socket_path = std::env::temp_dir().join("deskctl-doctor-test.sock"); + let report = run(&socket_path); + + assert!(report.checks.iter().any(|check| check.name == "display" && check.ok)); + assert!(report.checks.iter().any(|check| check.name == "backend" && check.ok)); + assert!( + report + .checks + .iter() + .any(|check| check.name == "window-enumeration" && check.ok) + ); + assert!( + report + .checks + .iter() + .any(|check| check.name == "screenshot" && check.ok) + ); + } +} diff --git a/src/core/mod.rs b/src/core/mod.rs index 95ab91e..07f3c01 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -1,3 +1,5 @@ +pub mod doctor; +pub mod paths; pub mod protocol; pub mod refs; pub mod session; diff --git a/src/core/paths.rs b/src/core/paths.rs new file mode 100644 index 0000000..1d657ec --- /dev/null +++ b/src/core/paths.rs @@ -0,0 +1,29 @@ +use std::path::PathBuf; + +pub fn socket_dir() -> PathBuf { + if let Ok(dir) = std::env::var("DESKCTL_SOCKET_DIR") { + return PathBuf::from(dir); + } + if let Ok(runtime) = std::env::var("XDG_RUNTIME_DIR") { + return PathBuf::from(runtime).join("deskctl"); + } + dirs::home_dir() + .unwrap_or_else(|| PathBuf::from("/tmp")) + .join(".deskctl") +} + +pub fn socket_path_for_session(session: &str) -> PathBuf { + socket_dir().join(format!("{session}.sock")) +} + +pub fn pid_path_for_session(session: &str) -> PathBuf { + socket_dir().join(format!("{session}.pid")) +} + +pub fn socket_path_from_env() -> Option { + std::env::var("DESKCTL_SOCKET_PATH").ok().map(PathBuf::from) +} + +pub fn pid_path_from_env() -> Option { + std::env::var("DESKCTL_PID_PATH").ok().map(PathBuf::from) +} diff --git a/src/core/refs.rs b/src/core/refs.rs index 7909d0e..101b00c 100644 --- a/src/core/refs.rs +++ b/src/core/refs.rs @@ -1,10 +1,14 @@ use serde::{Deserialize, Serialize}; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; + +use crate::backend::BackendWindow; +use crate::core::types::WindowInfo; #[derive(Debug, Clone, Serialize, Deserialize)] #[allow(dead_code)] pub struct RefEntry { - pub xcb_id: u32, + pub window_id: String, + pub backend_window_id: u32, pub app_class: String, pub title: String, pub pid: u32, @@ -19,58 +23,173 @@ pub struct RefEntry { #[derive(Debug, Default)] #[allow(dead_code)] pub struct RefMap { - map: HashMap, + refs: HashMap, + window_id_to_ref: HashMap, + backend_id_to_window_id: HashMap, next_ref: usize, + next_window: usize, } #[allow(dead_code)] impl RefMap { pub fn new() -> Self { Self { - map: HashMap::new(), + refs: HashMap::new(), + window_id_to_ref: HashMap::new(), + backend_id_to_window_id: HashMap::new(), next_ref: 1, + next_window: 1, } } pub fn clear(&mut self) { - self.map.clear(); + self.refs.clear(); + self.window_id_to_ref.clear(); self.next_ref = 1; } - pub fn insert(&mut self, entry: RefEntry) -> String { - let ref_id = format!("w{}", self.next_ref); - self.next_ref += 1; - self.map.insert(ref_id.clone(), entry); - ref_id + pub fn rebuild(&mut self, windows: &[BackendWindow]) -> Vec { + self.clear(); + + let active_backend_ids = windows + .iter() + .map(|window| window.native_id) + .collect::>(); + self.backend_id_to_window_id + .retain(|backend_id, _| active_backend_ids.contains(backend_id)); + + let mut public_windows = Vec::with_capacity(windows.len()); + for window in windows { + let ref_id = format!("w{}", self.next_ref); + self.next_ref += 1; + + let window_id = self.window_id_for_backend(window.native_id); + let entry = RefEntry { + window_id: window_id.clone(), + backend_window_id: window.native_id, + app_class: window.app_name.clone(), + title: window.title.clone(), + pid: 0, + x: window.x, + y: window.y, + width: window.width, + height: window.height, + focused: window.focused, + minimized: window.minimized, + }; + + self.window_id_to_ref + .insert(window_id.clone(), ref_id.clone()); + self.refs.insert(ref_id.clone(), entry); + public_windows.push(WindowInfo { + ref_id, + window_id, + title: window.title.clone(), + app_name: window.app_name.clone(), + x: window.x, + y: window.y, + width: window.width, + height: window.height, + focused: window.focused, + minimized: window.minimized, + }); + } + + public_windows + } + + fn window_id_for_backend(&mut self, backend_window_id: u32) -> String { + if let Some(existing) = self.backend_id_to_window_id.get(&backend_window_id) { + return existing.clone(); + } + + let window_id = format!("win{}", self.next_window); + self.next_window += 1; + self.backend_id_to_window_id + .insert(backend_window_id, window_id.clone()); + window_id } /// Resolve a selector to a RefEntry. - /// Accepts: "@w1", "w1", "ref=w1", or a substring match on app_class/title. + /// Accepts: "@w1", "w1", "ref=w1", "win1", "id=win1", or a substring match on app_class/title. pub fn resolve(&self, selector: &str) -> Option<&RefEntry> { let normalized = selector .strip_prefix('@') .or_else(|| selector.strip_prefix("ref=")) .unwrap_or(selector); - // Try direct ref lookup - if let Some(entry) = self.map.get(normalized) { + if let Some(entry) = self.refs.get(normalized) { return Some(entry); } - // Try substring match on app_class or title (case-insensitive) + let window_id = selector.strip_prefix("id=").unwrap_or(normalized); + if let Some(ref_id) = self.window_id_to_ref.get(window_id) { + return self.refs.get(ref_id); + } + let lower = selector.to_lowercase(); - self.map.values().find(|e| { - e.app_class.to_lowercase().contains(&lower) || e.title.to_lowercase().contains(&lower) + self.refs.values().find(|entry| { + entry.app_class.to_lowercase().contains(&lower) + || entry.title.to_lowercase().contains(&lower) }) } /// Resolve a selector to the center coordinates of the window. pub fn resolve_to_center(&self, selector: &str) -> Option<(i32, i32)> { self.resolve(selector) - .map(|e| (e.x + e.width as i32 / 2, e.y + e.height as i32 / 2)) + .map(|entry| (entry.x + entry.width as i32 / 2, entry.y + entry.height as i32 / 2)) } pub fn entries(&self) -> impl Iterator { - self.map.iter() + self.refs.iter() + } +} + +#[cfg(test)] +mod tests { + use super::RefMap; + use crate::backend::BackendWindow; + + fn sample_window(native_id: u32, title: &str) -> BackendWindow { + BackendWindow { + native_id, + title: title.to_string(), + app_name: "TestApp".to_string(), + x: 10, + y: 20, + width: 300, + height: 200, + focused: native_id == 1, + minimized: false, + } + } + + #[test] + fn rebuild_assigns_stable_window_ids_for_same_native_window() { + let mut refs = RefMap::new(); + let first = refs.rebuild(&[sample_window(1, "First")]); + let second = refs.rebuild(&[sample_window(1, "First Updated")]); + + assert_eq!(first[0].window_id, second[0].window_id); + assert_eq!(second[0].ref_id, "w1"); + } + + #[test] + fn resolve_accepts_ref_and_window_id() { + let mut refs = RefMap::new(); + let public = refs.rebuild(&[sample_window(42, "Editor")]); + let window_id = public[0].window_id.clone(); + + assert_eq!(refs.resolve("@w1").unwrap().window_id, window_id); + assert_eq!(refs.resolve(&window_id).unwrap().backend_window_id, 42); + assert_eq!(refs.resolve(&format!("id={window_id}")).unwrap().title, "Editor"); + } + + #[test] + fn resolve_to_center_uses_window_geometry() { + let mut refs = RefMap::new(); + refs.rebuild(&[sample_window(7, "Browser")]); + + assert_eq!(refs.resolve_to_center("w1"), Some((160, 120))); } } diff --git a/src/core/session.rs b/src/core/session.rs index 68ee0d0..6a2b6bd 100644 --- a/src/core/session.rs +++ b/src/core/session.rs @@ -15,14 +15,14 @@ pub fn detect_session() -> Result { bail!( "No X11 session detected.\n\ XDG_SESSION_TYPE is not set and DISPLAY is not set.\n\ - deskctl requires an X11 session. Wayland support coming in v0.2." + deskctl requires an X11 session." ); } } "wayland" => { bail!( "Wayland session detected (XDG_SESSION_TYPE=wayland).\n\ - deskctl currently supports X11 only. Wayland/Hyprland support coming in v0.2." + deskctl currently supports X11 only." ); } other => { diff --git a/src/core/types.rs b/src/core/types.rs index 3c6d36b..569fe37 100644 --- a/src/core/types.rs +++ b/src/core/types.rs @@ -11,7 +11,7 @@ pub struct Snapshot { #[derive(Debug, Serialize, Deserialize)] pub struct WindowInfo { pub ref_id: String, - pub xcb_id: u32, + pub window_id: String, pub title: String, pub app_name: String, pub x: i32, diff --git a/src/daemon/handler.rs b/src/daemon/handler.rs index 5a3fc2e..d37b0f1 100644 --- a/src/daemon/handler.rs +++ b/src/daemon/handler.rs @@ -1,13 +1,16 @@ use std::sync::Arc; + +use anyhow::{Context, Result}; use tokio::sync::Mutex; use super::state::DaemonState; -use crate::backend::DesktopBackend; +use crate::backend::annotate::annotate_screenshot; use crate::core::protocol::{Request, Response}; -use crate::core::refs::RefEntry; +use crate::core::types::{Snapshot, WindowInfo}; pub async fn handle_request(request: &Request, state: &Arc>) -> Response { match request.action.as_str() { + "ping" => Response::ok(serde_json::json!({"message": "pong"})), "snapshot" => handle_snapshot(request, state).await, "click" => handle_click(request, state).await, "dblclick" => handle_dblclick(request, state).await, @@ -38,55 +41,33 @@ async fn handle_snapshot(request: &Request, state: &Arc>) -> .unwrap_or(false); let mut state = state.lock().await; - - match state.backend.snapshot(annotate) { - Ok(snapshot) => { - // Update ref map - state.ref_map.clear(); - for win in &snapshot.windows { - state.ref_map.insert(RefEntry { - xcb_id: win.xcb_id, - app_class: win.app_name.clone(), - title: win.title.clone(), - pid: 0, // xcap doesn't expose PID directly in snapshot - x: win.x, - y: win.y, - width: win.width, - height: win.height, - focused: win.focused, - minimized: win.minimized, - }); - } - - Response::ok(serde_json::to_value(&snapshot).unwrap_or_default()) - } - Err(e) => Response::err(format!("Snapshot failed: {e}")), + match capture_snapshot(&mut state, annotate, None) { + Ok(snapshot) => Response::ok(serde_json::to_value(&snapshot).unwrap_or_default()), + Err(error) => Response::err(format!("Snapshot failed: {error}")), } } async fn handle_click(request: &Request, state: &Arc>) -> Response { let selector = match request.extra.get("selector").and_then(|v| v.as_str()) { - Some(s) => s.to_string(), + Some(selector) => selector.to_string(), None => return Response::err("Missing 'selector' field"), }; let mut state = state.lock().await; - // Try to parse as coordinates "x,y" if let Some((x, y)) = parse_coords(&selector) { return match state.backend.click(x, y) { Ok(()) => Response::ok(serde_json::json!({"clicked": {"x": x, "y": y}})), - Err(e) => Response::err(format!("Click failed: {e}")), + Err(error) => Response::err(format!("Click failed: {error}")), }; } - // Resolve as window ref match state.ref_map.resolve_to_center(&selector) { Some((x, y)) => match state.backend.click(x, y) { Ok(()) => { Response::ok(serde_json::json!({"clicked": {"x": x, "y": y, "ref": selector}})) } - Err(e) => Response::err(format!("Click failed: {e}")), + Err(error) => Response::err(format!("Click failed: {error}")), }, None => Response::err(format!("Could not resolve selector: {selector}")), } @@ -94,7 +75,7 @@ async fn handle_click(request: &Request, state: &Arc>) -> Res async fn handle_dblclick(request: &Request, state: &Arc>) -> Response { let selector = match request.extra.get("selector").and_then(|v| v.as_str()) { - Some(s) => s.to_string(), + Some(selector) => selector.to_string(), None => return Response::err("Missing 'selector' field"), }; @@ -103,7 +84,7 @@ async fn handle_dblclick(request: &Request, state: &Arc>) -> if let Some((x, y)) = parse_coords(&selector) { return match state.backend.dblclick(x, y) { Ok(()) => Response::ok(serde_json::json!({"double_clicked": {"x": x, "y": y}})), - Err(e) => Response::err(format!("Double-click failed: {e}")), + Err(error) => Response::err(format!("Double-click failed: {error}")), }; } @@ -112,7 +93,7 @@ async fn handle_dblclick(request: &Request, state: &Arc>) -> Ok(()) => Response::ok( serde_json::json!({"double_clicked": {"x": x, "y": y, "ref": selector}}), ), - Err(e) => Response::err(format!("Double-click failed: {e}")), + Err(error) => Response::err(format!("Double-click failed: {error}")), }, None => Response::err(format!("Could not resolve selector: {selector}")), } @@ -120,70 +101,66 @@ async fn handle_dblclick(request: &Request, state: &Arc>) -> async fn handle_type(request: &Request, state: &Arc>) -> Response { let text = match request.extra.get("text").and_then(|v| v.as_str()) { - Some(t) => t.to_string(), + Some(text) => text.to_string(), None => return Response::err("Missing 'text' field"), }; let mut state = state.lock().await; - match state.backend.type_text(&text) { Ok(()) => Response::ok(serde_json::json!({"typed": text})), - Err(e) => Response::err(format!("Type failed: {e}")), + Err(error) => Response::err(format!("Type failed: {error}")), } } async fn handle_press(request: &Request, state: &Arc>) -> Response { let key = match request.extra.get("key").and_then(|v| v.as_str()) { - Some(k) => k.to_string(), + Some(key) => key.to_string(), None => return Response::err("Missing 'key' field"), }; let mut state = state.lock().await; - match state.backend.press_key(&key) { Ok(()) => Response::ok(serde_json::json!({"pressed": key})), - Err(e) => Response::err(format!("Key press failed: {e}")), + Err(error) => Response::err(format!("Key press failed: {error}")), } } async fn handle_hotkey(request: &Request, state: &Arc>) -> Response { let keys: Vec = match request.extra.get("keys").and_then(|v| v.as_array()) { - Some(arr) => arr + Some(keys) => keys .iter() - .filter_map(|v| v.as_str().map(|s| s.to_string())) + .filter_map(|value| value.as_str().map(|s| s.to_string())) .collect(), None => return Response::err("Missing 'keys' field"), }; let mut state = state.lock().await; - match state.backend.hotkey(&keys) { Ok(()) => Response::ok(serde_json::json!({"hotkey": keys})), - Err(e) => Response::err(format!("Hotkey failed: {e}")), + Err(error) => Response::err(format!("Hotkey failed: {error}")), } } async fn handle_mouse_move(request: &Request, state: &Arc>) -> Response { let x = match request.extra.get("x").and_then(|v| v.as_i64()) { - Some(v) => v as i32, + Some(value) => value as i32, None => return Response::err("Missing 'x' field"), }; let y = match request.extra.get("y").and_then(|v| v.as_i64()) { - Some(v) => v as i32, + Some(value) => value as i32, None => return Response::err("Missing 'y' field"), }; let mut state = state.lock().await; - match state.backend.mouse_move(x, y) { Ok(()) => Response::ok(serde_json::json!({"moved": {"x": x, "y": y}})), - Err(e) => Response::err(format!("Mouse move failed: {e}")), + Err(error) => Response::err(format!("Mouse move failed: {error}")), } } async fn handle_mouse_scroll(request: &Request, state: &Arc>) -> Response { let amount = match request.extra.get("amount").and_then(|v| v.as_i64()) { - Some(v) => v as i32, + Some(value) => value as i32, None => return Response::err("Missing 'amount' field"), }; let axis = request @@ -194,33 +171,31 @@ async fn handle_mouse_scroll(request: &Request, state: &Arc>) .to_string(); let mut state = state.lock().await; - match state.backend.scroll(amount, &axis) { Ok(()) => Response::ok(serde_json::json!({"scrolled": {"amount": amount, "axis": axis}})), - Err(e) => Response::err(format!("Scroll failed: {e}")), + Err(error) => Response::err(format!("Scroll failed: {error}")), } } async fn handle_mouse_drag(request: &Request, state: &Arc>) -> Response { let x1 = match request.extra.get("x1").and_then(|v| v.as_i64()) { - Some(v) => v as i32, + Some(value) => value as i32, None => return Response::err("Missing 'x1' field"), }; let y1 = match request.extra.get("y1").and_then(|v| v.as_i64()) { - Some(v) => v as i32, + Some(value) => value as i32, None => return Response::err("Missing 'y1' field"), }; let x2 = match request.extra.get("x2").and_then(|v| v.as_i64()) { - Some(v) => v as i32, + Some(value) => value as i32, None => return Response::err("Missing 'x2' field"), }; let y2 = match request.extra.get("y2").and_then(|v| v.as_i64()) { - Some(v) => v as i32, + Some(value) => value as i32, None => return Response::err("Missing 'y2' field"), }; let mut state = state.lock().await; - match state.backend.drag(x1, y1, x2, y2) { Ok(()) => Response::ok(serde_json::json!({ "dragged": { @@ -228,7 +203,7 @@ async fn handle_mouse_drag(request: &Request, state: &Arc>) - "to": {"x": x2, "y": y2} } })), - Err(e) => Response::err(format!("Drag failed: {e}")), + Err(error) => Response::err(format!("Drag failed: {error}")), } } @@ -238,20 +213,19 @@ async fn handle_window_action( action: &str, ) -> Response { let selector = match request.extra.get("selector").and_then(|v| v.as_str()) { - Some(s) => s.to_string(), + Some(selector) => selector.to_string(), None => return Response::err("Missing 'selector' field"), }; let mut state = state.lock().await; - let entry = match state.ref_map.resolve(&selector) { - Some(e) => e.clone(), + Some(entry) => entry.clone(), None => return Response::err(format!("Could not resolve window: {selector}")), }; let result = match action { - "focus" => state.backend.focus_window(entry.xcb_id), - "close" => state.backend.close_window(entry.xcb_id), + "focus" => state.backend.focus_window(entry.backend_window_id), + "close" => state.backend.close_window(entry.backend_window_id), _ => unreachable!(), }; @@ -259,15 +233,15 @@ async fn handle_window_action( Ok(()) => Response::ok(serde_json::json!({ "action": action, "window": entry.title, - "xcb_id": entry.xcb_id, + "window_id": entry.window_id, })), - Err(e) => Response::err(format!("{action} failed: {e}")), + Err(error) => Response::err(format!("{action} failed: {error}")), } } async fn handle_move_window(request: &Request, state: &Arc>) -> Response { let selector = match request.extra.get("selector").and_then(|v| v.as_str()) { - Some(s) => s.to_string(), + Some(selector) => selector.to_string(), None => return Response::err("Missing 'selector' field"), }; let x = request.extra.get("x").and_then(|v| v.as_i64()).unwrap_or(0) as i32; @@ -275,29 +249,32 @@ async fn handle_move_window(request: &Request, state: &Arc>) let mut state = state.lock().await; let entry = match state.ref_map.resolve(&selector) { - Some(e) => e.clone(), + Some(entry) => entry.clone(), None => return Response::err(format!("Could not resolve window: {selector}")), }; - match state.backend.move_window(entry.xcb_id, x, y) { + match state.backend.move_window(entry.backend_window_id, x, y) { Ok(()) => Response::ok(serde_json::json!({ - "moved": entry.title, "x": x, "y": y + "moved": entry.title, + "window_id": entry.window_id, + "x": x, + "y": y, })), - Err(e) => Response::err(format!("Move failed: {e}")), + Err(error) => Response::err(format!("Move failed: {error}")), } } async fn handle_resize_window(request: &Request, state: &Arc>) -> Response { let selector = match request.extra.get("selector").and_then(|v| v.as_str()) { - Some(s) => s.to_string(), + Some(selector) => selector.to_string(), None => return Response::err("Missing 'selector' field"), }; - let w = request + let width = request .extra .get("w") .and_then(|v| v.as_u64()) .unwrap_or(800) as u32; - let h = request + let height = request .extra .get("h") .and_then(|v| v.as_u64()) @@ -305,50 +282,37 @@ async fn handle_resize_window(request: &Request, state: &Arc> let mut state = state.lock().await; let entry = match state.ref_map.resolve(&selector) { - Some(e) => e.clone(), + Some(entry) => entry.clone(), None => return Response::err(format!("Could not resolve window: {selector}")), }; - match state.backend.resize_window(entry.xcb_id, w, h) { + match state + .backend + .resize_window(entry.backend_window_id, width, height) + { Ok(()) => Response::ok(serde_json::json!({ - "resized": entry.title, "width": w, "height": h + "resized": entry.title, + "window_id": entry.window_id, + "width": width, + "height": height, })), - Err(e) => Response::err(format!("Resize failed: {e}")), + Err(error) => Response::err(format!("Resize failed: {error}")), } } async fn handle_list_windows(state: &Arc>) -> Response { let mut state = state.lock().await; - // Re-run snapshot without screenshot, just to get current window list - match state.backend.snapshot(false) { - Ok(snapshot) => { - // Update ref map with fresh data - state.ref_map.clear(); - for win in &snapshot.windows { - state.ref_map.insert(RefEntry { - xcb_id: win.xcb_id, - app_class: win.app_name.clone(), - title: win.title.clone(), - pid: 0, - x: win.x, - y: win.y, - width: win.width, - height: win.height, - focused: win.focused, - minimized: win.minimized, - }); - } - Response::ok(serde_json::json!({"windows": snapshot.windows})) - } - Err(e) => Response::err(format!("List windows failed: {e}")), + match refresh_windows(&mut state) { + Ok(windows) => Response::ok(serde_json::json!({"windows": windows})), + Err(error) => Response::err(format!("List windows failed: {error}")), } } async fn handle_get_screen_size(state: &Arc>) -> Response { let state = state.lock().await; match state.backend.screen_size() { - Ok((w, h)) => Response::ok(serde_json::json!({"width": w, "height": h})), - Err(e) => Response::err(format!("Failed: {e}")), + Ok((width, height)) => Response::ok(serde_json::json!({"width": width, "height": height})), + Err(error) => Response::err(format!("Failed: {error}")), } } @@ -356,7 +320,7 @@ async fn handle_get_mouse_position(state: &Arc>) -> Response let state = state.lock().await; match state.backend.mouse_position() { Ok((x, y)) => Response::ok(serde_json::json!({"x": x, "y": y})), - Err(e) => Response::err(format!("Failed: {e}")), + Err(error) => Response::err(format!("Failed: {error}")), } } @@ -370,50 +334,160 @@ async fn handle_screenshot(request: &Request, state: &Arc>) - .extra .get("path") .and_then(|v| v.as_str()) - .map(|s| s.to_string()) - .unwrap_or_else(|| { - let ts = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap_or_default() - .as_millis(); - format!("/tmp/deskctl-{ts}.png") - }); + .map(|value| value.to_string()) + .unwrap_or_else(temp_screenshot_path); + let mut state = state.lock().await; - match state.backend.screenshot(&path, annotate) { - Ok(saved) => Response::ok(serde_json::json!({"screenshot": saved})), - Err(e) => Response::err(format!("Screenshot failed: {e}")), + let windows = if annotate { + match refresh_windows(&mut state) { + Ok(windows) => Some(windows), + Err(error) => return Response::err(format!("Screenshot failed: {error}")), + } + } else { + None + }; + + match capture_and_save_screenshot(&mut state, &path, annotate, windows.as_deref()) { + Ok(saved) => { + if let Some(windows) = windows { + Response::ok(serde_json::json!({"screenshot": saved, "windows": windows})) + } else { + Response::ok(serde_json::json!({"screenshot": saved})) + } + } + Err(error) => Response::err(format!("Screenshot failed: {error}")), } } async fn handle_launch(request: &Request, state: &Arc>) -> Response { let command = match request.extra.get("command").and_then(|v| v.as_str()) { - Some(c) => c.to_string(), + Some(command) => command.to_string(), None => return Response::err("Missing 'command' field"), }; let args: Vec = request .extra .get("args") .and_then(|v| v.as_array()) - .map(|arr| { - arr.iter() - .filter_map(|v| v.as_str().map(String::from)) + .map(|args| { + args.iter() + .filter_map(|value| value.as_str().map(String::from)) .collect() }) .unwrap_or_default(); + let state = state.lock().await; match state.backend.launch(&command, &args) { Ok(pid) => Response::ok(serde_json::json!({"pid": pid, "command": command})), - Err(e) => Response::err(format!("Launch failed: {e}")), + Err(error) => Response::err(format!("Launch failed: {error}")), } } -fn parse_coords(s: &str) -> Option<(i32, i32)> { - let parts: Vec<&str> = s.split(',').collect(); - if parts.len() == 2 { - let x = parts[0].trim().parse().ok()?; - let y = parts[1].trim().parse().ok()?; - Some((x, y)) - } else { - None +fn refresh_windows(state: &mut DaemonState) -> Result> { + let windows = state.backend.list_windows()?; + Ok(state.ref_map.rebuild(&windows)) +} + +fn capture_snapshot( + state: &mut DaemonState, + annotate: bool, + path: Option, +) -> Result { + let windows = refresh_windows(state)?; + let screenshot_path = path.unwrap_or_else(temp_screenshot_path); + let screenshot = capture_and_save_screenshot( + state, + &screenshot_path, + annotate, + Some(&windows), + )?; + + Ok(Snapshot { screenshot, windows }) +} + +fn capture_and_save_screenshot( + state: &mut DaemonState, + path: &str, + annotate: bool, + windows: Option<&[WindowInfo]>, +) -> Result { + let mut image = state.backend.capture_screenshot()?; + if annotate { + let windows = windows.context("Annotated screenshots require current window data")?; + annotate_screenshot(&mut image, windows); + } + image + .save(path) + .with_context(|| format!("Failed to save screenshot to {path}"))?; + Ok(path.to_string()) +} + +fn temp_screenshot_path() -> String { + let timestamp = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_millis(); + format!("/tmp/deskctl-{timestamp}.png") +} + +fn parse_coords(value: &str) -> Option<(i32, i32)> { + let parts: Vec<&str> = value.split(',').collect(); + if parts.len() != 2 { + return None; + } + + let x = parts[0].trim().parse().ok()?; + let y = parts[1].trim().parse().ok()?; + Some((x, y)) +} + +#[cfg(all(test, target_os = "linux"))] +mod tests { + use std::sync::Arc; + + use tokio::runtime::Builder; + use tokio::sync::Mutex; + + use super::handle_request; + use crate::core::protocol::Request; + use crate::daemon::state::DaemonState; + use crate::test_support::{X11TestEnv, deskctl_tmp_screenshot_count, env_lock}; + + #[test] + fn list_windows_is_side_effect_free_under_xvfb() { + let _guard = env_lock().lock().unwrap(); + let Some(env) = X11TestEnv::new().unwrap() else { + eprintln!("Skipping Xvfb-dependent list-windows test"); + return; + }; + env.create_window("deskctl list-windows test", "DeskctlList").unwrap(); + + let before = deskctl_tmp_screenshot_count(); + let runtime = Builder::new_current_thread().enable_all().build().unwrap(); + let state = Arc::new(Mutex::new( + DaemonState::new( + "test".to_string(), + std::env::temp_dir().join("deskctl-list-windows.sock"), + ) + .unwrap(), + )); + + let response = runtime.block_on(handle_request(&Request::new("list-windows"), &state)); + assert!(response.success); + + let data = response.data.unwrap(); + let windows = data + .get("windows") + .and_then(|value| value.as_array()) + .unwrap(); + assert!(windows.iter().any(|window| { + window + .get("title") + .and_then(|value| value.as_str()) + .map(|title| title == "deskctl list-windows test") + .unwrap_or(false) + })); + + let after = deskctl_tmp_screenshot_count(); + assert_eq!(before, after, "list-windows should not create screenshot artifacts"); } } diff --git a/src/daemon/mod.rs b/src/daemon/mod.rs index f838bbf..3df1d9a 100644 --- a/src/daemon/mod.rs +++ b/src/daemon/mod.rs @@ -1,7 +1,6 @@ mod handler; mod state; -use std::path::PathBuf; use std::sync::Arc; use anyhow::{Context, Result}; @@ -9,6 +8,7 @@ use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; use tokio::net::UnixListener; use tokio::sync::Mutex; +use crate::core::paths::{pid_path_from_env, socket_path_from_env}; use crate::core::session; use state::DaemonState; @@ -24,11 +24,9 @@ pub fn run() -> Result<()> { } async fn async_run() -> Result<()> { - let socket_path = std::env::var("DESKCTL_SOCKET_PATH") - .map(PathBuf::from) - .context("DESKCTL_SOCKET_PATH not set")?; + let socket_path = socket_path_from_env().context("DESKCTL_SOCKET_PATH not set")?; - let pid_path = std::env::var("DESKCTL_PID_PATH").map(PathBuf::from).ok(); + let pid_path = pid_path_from_env(); // Clean up stale socket if socket_path.exists() { diff --git a/src/daemon/state.rs b/src/daemon/state.rs index 57e865c..23b0e3b 100644 --- a/src/daemon/state.rs +++ b/src/daemon/state.rs @@ -1,6 +1,6 @@ use std::path::PathBuf; -use crate::backend::x11::X11Backend; +use crate::backend::{x11::X11Backend, DesktopBackend}; use crate::core::refs::RefMap; #[allow(dead_code)] @@ -8,12 +8,12 @@ pub struct DaemonState { pub session: String, pub socket_path: PathBuf, pub ref_map: RefMap, - pub backend: X11Backend, + pub backend: Box, } impl DaemonState { pub fn new(session: String, socket_path: PathBuf) -> anyhow::Result { - let backend = X11Backend::new()?; + let backend: Box = Box::new(X11Backend::new()?); Ok(Self { session, socket_path, diff --git a/src/main.rs b/src/main.rs index 349b70d..4bb6fab 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2,6 +2,8 @@ mod backend; mod cli; mod core; mod daemon; +#[cfg(test)] +mod test_support; fn main() -> anyhow::Result<()> { if std::env::var("DESKCTL_DAEMON").is_ok() { diff --git a/src/test_support.rs b/src/test_support.rs new file mode 100644 index 0000000..c21a61c --- /dev/null +++ b/src/test_support.rs @@ -0,0 +1,150 @@ +#![cfg(all(test, target_os = "linux"))] + +use std::path::Path; +use std::process::{Child, Command, Stdio}; +use std::sync::{Mutex, OnceLock}; +use std::thread; +use std::time::Duration; + +use anyhow::{Context, Result}; +use x11rb::connection::Connection; +use x11rb::protocol::xproto::{ + AtomEnum, ConnectionExt as XprotoConnectionExt, CreateWindowAux, EventMask, PropMode, + WindowClass, +}; + +pub fn env_lock() -> &'static Mutex<()> { + static LOCK: OnceLock> = OnceLock::new(); + LOCK.get_or_init(|| Mutex::new(())) +} + +pub struct X11TestEnv { + child: Child, + old_display: Option, + old_session_type: Option, +} + +impl X11TestEnv { + pub fn new() -> Result> { + if Command::new("Xvfb") + .arg("-help") + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + .is_err() + { + return Ok(None); + } + + for display_num in 90..110 { + let display = format!(":{display_num}"); + let lock_path = format!("/tmp/.X{display_num}-lock"); + let unix_socket = format!("/tmp/.X11-unix/X{display_num}"); + if Path::new(&lock_path).exists() || Path::new(&unix_socket).exists() { + continue; + } + + let child = Command::new("Xvfb") + .arg(&display) + .arg("-screen") + .arg("0") + .arg("1024x768x24") + .arg("-nolisten") + .arg("tcp") + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .with_context(|| format!("Failed to launch Xvfb on {display}"))?; + + thread::sleep(Duration::from_millis(250)); + + let old_display = std::env::var("DISPLAY").ok(); + let old_session_type = std::env::var("XDG_SESSION_TYPE").ok(); + std::env::set_var("DISPLAY", &display); + std::env::set_var("XDG_SESSION_TYPE", "x11"); + + return Ok(Some(Self { + child, + old_display, + old_session_type, + })); + } + + anyhow::bail!("Failed to find a free Xvfb display") + } + + pub fn create_window(&self, title: &str, app_class: &str) -> Result<()> { + let (conn, screen_num) = + x11rb::connect(None).context("Failed to connect to test Xvfb display")?; + let screen = &conn.setup().roots[screen_num]; + let window = conn.generate_id()?; + + conn.create_window( + x11rb::COPY_DEPTH_FROM_PARENT, + window, + screen.root, + 10, + 10, + 320, + 180, + 0, + WindowClass::INPUT_OUTPUT, + 0, + &CreateWindowAux::new() + .background_pixel(screen.white_pixel) + .event_mask(EventMask::EXPOSURE), + )?; + conn.change_property8( + PropMode::REPLACE, + window, + AtomEnum::WM_NAME, + AtomEnum::STRING, + title.as_bytes(), + )?; + let class_bytes = format!("{app_class}\0{app_class}\0"); + conn.change_property8( + PropMode::REPLACE, + window, + AtomEnum::WM_CLASS, + AtomEnum::STRING, + class_bytes.as_bytes(), + )?; + conn.map_window(window)?; + conn.flush()?; + + thread::sleep(Duration::from_millis(150)); + Ok(()) + } +} + +impl Drop for X11TestEnv { + fn drop(&mut self) { + let _ = self.child.kill(); + let _ = self.child.wait(); + + match &self.old_display { + Some(value) => std::env::set_var("DISPLAY", value), + None => std::env::remove_var("DISPLAY"), + } + + match &self.old_session_type { + Some(value) => std::env::set_var("XDG_SESSION_TYPE", value), + None => std::env::remove_var("XDG_SESSION_TYPE"), + } + } +} + +pub fn deskctl_tmp_screenshot_count() -> usize { + std::fs::read_dir("/tmp") + .ok() + .into_iter() + .flat_map(|iter| iter.filter_map(Result::ok)) + .filter(|entry| { + entry + .file_name() + .to_str() + .map(|name| name.starts_with("deskctl-") && name.ends_with(".png")) + .unwrap_or(false) + }) + .count() +}