jonesy 0.9.0

//! LSP server for jonesy panic point diagnostics.
//!
//! This module implements a Language Server Protocol server that publishes
//! panic point diagnostics to IDEs and code editors. It runs alongside
//! rust-analyzer, publishing its own diagnostics.
//!
//! File watching is implemented natively using the `notify` crate for reliable
//! detection of binary changes, rather than relying on LSP client file watchers.

use std::collections::{HashMap, HashSet};
use std::path::{Path, PathBuf};
use std::sync::Arc;
use std::sync::atomic::{AtomicU32, Ordering};
use std::time::Duration;
use tokio::sync::{Mutex, RwLock};
use tower_lsp::jsonrpc::Result;
use tower_lsp::lsp_types::notification::Progress;
use tower_lsp::lsp_types::request::WorkDoneProgressCreate;
use tower_lsp::lsp_types::*;
use tower_lsp::{Client, LanguageServer, LspService, Server};

use crate::call_tree::CrateCodePoint;
use crate::cargo::{find_binary, find_library};
use crate::file_watcher::{self, WatcherConfig};
use crate::project_context::ProjectContext;
use notify::RecommendedWatcher;

/// Counter for generating unique progress tokens
static PROGRESS_TOKEN_COUNTER: AtomicU32 = AtomicU32::new(0);

/// State shared across the LSP server
struct ServerState {
    /// Workspace root path
    workspace_root: Option<PathBuf>,
    /// Cached panic points by file URI
    panic_points: HashMap<Url, Vec<CrateCodePoint>>,
    /// Files that have been opened (for re-publishing after analysis)
    opened_files: HashSet<Url>,
}

impl ServerState {
    fn new() -> Self {
        Self {
            workspace_root: None,
            panic_points: HashMap::new(),
            opened_files: HashSet::new(),
        }
    }
}

/// The jonesy LSP server backend
struct JonesyLspServer {
    client: Client,
    state: Arc<RwLock<ServerState>>,
    /// Lock to serialize analysis runs and prevent out-of-order diagnostics
    analysis_lock: Arc<Mutex<()>>,
    /// Native file watcher - must be kept alive for watcher to function.
    /// Stored separately from the events receiver which is consumed by the debounce task.
    #[allow(dead_code)] // Kept alive to maintain file watching
    watcher: Arc<RwLock<Option<RecommendedWatcher>>>,
}

impl JonesyLspServer {
    pub fn new(client: Client) -> Self {
        Self {
            client,
            state: Arc::new(RwLock::new(ServerState::new())),
            analysis_lock: Arc::new(Mutex::new(())),
            watcher: Arc::new(RwLock::new(None)),
        }
    }

    /// Convert a CrateCodePoint to an LSP Diagnostic
    fn code_point_to_diagnostic(point: &CrateCodePoint) -> Diagnostic {
        // Get all causes sorted by error code for determinism
        let sorted_causes: Vec<_> = {
            let mut causes: Vec<_> = point.causes.iter().collect();
            causes.sort_by_key(|c| c.error_code());
            causes
        };

        // Build message showing all causes
        let (message, suggestion, error_code, docs_url) = if sorted_causes.is_empty() {
            ("potential panic point".to_string(), None, None, None)
        } else {
            let descriptions: Vec<_> = sorted_causes
                .iter()
                .map(|c| format!("{}/{}: {}", c.error_code(), c.id(), c.description()))
                .collect();
            let primary = sorted_causes[0];
            (
                format!("panic point: {}", descriptions.join(", ")),
                Some(
                    primary
                        .format_suggestion(point.is_direct_panic, point.called_function.as_deref()),
                ),
                Some(primary.error_code().to_string()),
                Url::parse(&primary.docs_url()).ok(),
            )
        };

        let range = Range {
            start: Position {
                line: point.line.saturating_sub(1), // LSP uses 0-based lines
                character: point.column.unwrap_or(1).saturating_sub(1),
            },
            end: Position {
                line: point.line.saturating_sub(1),
                character: point.column.unwrap_or(1).saturating_sub(1) + 10, // Approximate width
            },
        };

        // Store cause info in data field for use by code actions
        // Use sorted_causes for consistency with the displayed message
        let cause_ids: Vec<String> = sorted_causes.iter().map(|c| c.id().to_string()).collect();
        let data = serde_json::json!({
            "causes": cause_ids,
            "function": &point.name,
            "file": &point.file,
            "called_function": &point.called_function,
            "is_direct_panic": point.is_direct_panic,
        });

        // Create code_description with documentation URL if available
        let code_description = docs_url.map(|href| CodeDescription { href });

        let mut diagnostic = Diagnostic {
            range,
            severity: Some(DiagnosticSeverity::WARNING),
            code: error_code.map(NumberOrString::String),
            code_description,
            source: Some("jonesy".to_string()),
            message,
            related_information: None,
            tags: None,
            data: Some(data),
        };

        // Add suggestion as related information if available
        if let Some(help) = suggestion {
            if !help.is_empty() {
                diagnostic.message = format!("{}\nhelp: {}", diagnostic.message, help);
            }
        }

        diagnostic
    }

    /// Create a progress token and request the client to show progress UI.
    /// Returns the token if successful, None if the client doesn't support it.
    async fn create_progress(&self) -> Option<ProgressToken> {
        let token_id = PROGRESS_TOKEN_COUNTER.fetch_add(1, Ordering::SeqCst);
        let token = ProgressToken::Number(token_id as i32);

        // Request the client to create a progress indicator
        match self
            .client
            .send_request::<WorkDoneProgressCreate>(WorkDoneProgressCreateParams {
                token: token.clone(),
            })
            .await
        {
            Ok(()) => Some(token),
            Err(e) => {
                // Client may not support progress - log and continue without it
                self.client
                    .log_message(MessageType::LOG, format!("Progress not supported: {}", e))
                    .await;
                None
            }
        }
    }

    /// Send a progress begin notification
    async fn progress_begin(&self, token: &ProgressToken, title: &str, message: Option<&str>) {
        self.client
            .send_notification::<Progress>(ProgressParams {
                token: token.clone(),
                value: ProgressParamsValue::WorkDone(WorkDoneProgress::Begin(
                    WorkDoneProgressBegin {
                        title: title.to_string(),
                        cancellable: Some(false),
                        message: message.map(String::from),
                        percentage: Some(0),
                    },
                )),
            })
            .await;
    }

    /// Send a progress report notification
    async fn progress_report(&self, token: &ProgressToken, message: &str, percentage: u32) {
        self.client
            .send_notification::<Progress>(ProgressParams {
                token: token.clone(),
                value: ProgressParamsValue::WorkDone(WorkDoneProgress::Report(
                    WorkDoneProgressReport {
                        cancellable: Some(false),
                        message: Some(message.to_string()),
                        percentage: Some(percentage),
                    },
                )),
            })
            .await;
    }

    /// Send a progress end notification
    async fn progress_end(&self, token: &ProgressToken, message: &str) {
        self.client
            .send_notification::<Progress>(ProgressParams {
                token: token.clone(),
                value: ProgressParamsValue::WorkDone(WorkDoneProgress::End(WorkDoneProgressEnd {
                    message: Some(message.to_string()),
                })),
            })
            .await;
    }

    /// Register file watchers for binaries and config files.
    /// Watches target/debug/ for binary changes and config files (jonesy.toml, Cargo.toml).
    async fn register_file_watchers(&self) {
        // Clone workspace_root and release lock before async operations
        let workspace_root = {
            let state = self.state.read().await;
            state.workspace_root.clone()
        };
        let Some(workspace_root) = workspace_root else {
            return;
        };

        // Build glob patterns for target/debug/ binaries
        let target_debug = workspace_root.join("target/debug");
        let target_debug_str = target_debug.to_string_lossy();

        // Watch for all files in target/debug/ (binaries, rlibs, dylibs)
        let mut watchers = vec![
            FileSystemWatcher {
                glob_pattern: GlobPattern::String(format!("{}/*", target_debug_str)),
                kind: Some(WatchKind::Create | WatchKind::Change),
            },
            // Also watch for dSYM bundles on macOS
            FileSystemWatcher {
                glob_pattern: GlobPattern::String(format!("{}/*.dSYM/**", target_debug_str)),
                kind: Some(WatchKind::Create | WatchKind::Change),
            },
        ];

        // Watch config files (jonesy.toml and Cargo.toml files)
        let config_files = find_config_files(&workspace_root);
        for config_path in &config_files {
            watchers.push(FileSystemWatcher {
                glob_pattern: GlobPattern::String(config_path.to_string_lossy().to_string()),
                kind: Some(WatchKind::Create | WatchKind::Change | WatchKind::Delete),
            });
        }

        let registration_options = DidChangeWatchedFilesRegistrationOptions { watchers };

        let registration = Registration {
            id: "jonesy-file-watcher".to_string(),
            method: "workspace/didChangeWatchedFiles".to_string(),
            register_options: Some(serde_json::to_value(registration_options).unwrap()),
        };

        match self.client.register_capability(vec![registration]).await {
            Ok(()) => {
                self.client
                    .log_message(
                        MessageType::INFO,
                        format!(
                            "Watching {} for binary changes and {} config file(s)",
                            target_debug_str,
                            config_files.len()
                        ),
                    )
                    .await;
            }
            Err(e) => {
                self.client
                    .log_message(
                        MessageType::WARNING,
                        format!("Failed to register file watchers: {}", e),
                    )
                    .await;
            }
        }
    }

    /// Start native file watching using the `notify` crate.
    ///
    /// This provides reliable detection of binary changes, unlike LSP file watchers
    /// which many IDEs don't implement for paths outside source directories.
    ///
    /// Returns true if native watching started successfully, false otherwise.
    /// LSP file watchers should only be registered as fallback when this returns false.
    async fn start_native_file_watcher(&self) -> bool {
        let workspace_root = {
            let state = self.state.read().await;
            state.workspace_root.clone()
        };

        let Some(workspace_root) = workspace_root else {
            return false;
        };

        let target_dir = workspace_root.join("target/debug");
        let config_files = find_config_files(&workspace_root);

        let config = WatcherConfig {
            target_dir: target_dir.clone(),
            config_files: config_files.clone(),
            debounce: Duration::from_millis(500),
        };

        // Start the file watcher
        let watcher_handle = match file_watcher::start_watching(config) {
            Ok(handle) => handle,
            Err(e) => {
                self.client
                    .log_message(
                        MessageType::WARNING,
                        format!("Failed to start native file watcher: {}", e),
                    )
                    .await;
                return false;
            }
        };

        self.client
            .log_message(
                MessageType::INFO,
                format!("Native file watcher started for {}", target_dir.display()),
            )
            .await;

        // Destructure handle to get events receiver and watcher separately
        let file_watcher::WatcherHandle { events, watcher } = watcher_handle;

        // Store watcher to keep it alive for the server's lifetime
        *self.watcher.write().await = Some(watcher);

        // Create debounced event stream from the events receiver
        let debounced = file_watcher::debounced_events(events, Duration::from_millis(500)).await;

        // Clone what we need for the spawned task
        let client = self.client.clone();
        let state = self.state.clone();
        let analysis_lock = self.analysis_lock.clone();

        // Spawn task to listen for file changes and trigger analysis
        tokio::spawn(async move {
            let mut debounced = debounced;

            while debounced.recv().await.is_some() {
                client
                    .log_message(MessageType::INFO, "File change detected, re-analyzing...")
                    .await;

                // Run analysis (using the same logic as analyze_and_publish)
                run_analysis_task(&client, &state, &analysis_lock).await;
            }
        });

        true
    }

    /// Analyze the workspace and publish diagnostics
    /// Returns true if analysis succeeded, false otherwise
    async fn analyze_and_publish(&self) -> bool {
        // Serialize analysis runs to prevent out-of-order diagnostics
        let _guard = self.analysis_lock.lock().await;

        // Extract workspace root early and release state lock
        let (workspace_root, target_dir) = {
            let state = self.state.read().await;
            let Some(root) = state.workspace_root.clone() else {
                self.client
                    .log_message(MessageType::WARNING, "No workspace root set")
                    .await;
                return false;
            };
            let target_dir = root.join("target").to_string_lossy().to_string();
            (root, target_dir)
        };

        self.client
            .log_message(
                MessageType::INFO,
                format!("Analyzing workspace: {}", workspace_root.display()),
            )
            .await;

        // First, discover workspace structure
        let workspace_info = {
            let workspace_root = workspace_root.clone();
            tokio::task::spawn_blocking(move || discover_workspace(&workspace_root))
                .await
                .ok()
                .flatten()
        };

        if let Some(info) = &workspace_info {
            self.client
                .log_message(
                    MessageType::INFO,
                    format!("Workspace members: {}", info.members.join(", ")),
                )
                .await;
            self.client
                .log_message(
                    MessageType::INFO,
                    format!(
                        "Found {} targets: {}",
                        info.targets.len(),
                        info.targets.join(", ")
                    ),
                )
                .await;
        }

        // Get list of targets to analyze
        let targets = {
            let workspace_root = workspace_root.clone();
            tokio::task::spawn_blocking(move || find_workspace_binaries(&workspace_root))
                .await
                .ok()
                .and_then(|r| r.ok())
                .unwrap_or_default()
        };

        if targets.is_empty() {
            self.client
                .log_message(MessageType::WARNING, "No targets found to analyze")
                .await;
            return false;
        }

        self.client
            .log_message(
                MessageType::INFO,
                format!("Starting analysis of {} targets...", targets.len()),
            )
            .await;

        // Create progress indicator for IDE status bar
        let progress_token = self.create_progress().await;
        if let Some(ref token) = progress_token {
            self.progress_begin(token, "Panic Analysis", Some("Analyzing targets..."))
                .await;
        }

        // Build ProjectContext once for the workspace
        let project_context = {
            let root = workspace_root.clone();
            tokio::task::spawn_blocking(move || ProjectContext::from_project_root(&root))
                .await
                .unwrap_or_else(|e| Err(format!("Failed to build project context: {e}")))
        };
        let project_context = match project_context {
            Ok(ctx) => Arc::new(ctx),
            Err(e) => {
                self.client
                    .log_message(MessageType::ERROR, format!("ProjectContext error: {e}"))
                    .await;
                if let Some(ref token) = progress_token {
                    self.progress_end(token, "ProjectContext error").await;
                }
                return false;
            }
        };

        // Track all diagnostics by file URI (accumulates across targets)
        let mut points_by_file: HashMap<Url, Vec<CrateCodePoint>> = HashMap::new();
        let mut seen: std::collections::HashSet<(String, u32, u32)> =
            std::collections::HashSet::new();
        let mut total_points = 0usize;

        // Analyze each target and publish diagnostics incrementally
        let total_targets = targets.len();
        for (target_idx, target) in targets.iter().enumerate() {
            let target_name = target
                .file_name()
                .map(|n| n.to_string_lossy().to_string())
                .unwrap_or_else(|| "unknown".to_string());

            // Update progress indicator
            if let Some(ref token) = progress_token {
                let percentage = (((target_idx + 1) * 100) / total_targets) as u32;
                self.progress_report(
                    token,
                    &format!(
                        "Analyzing {} ({}/{})",
                        target_name,
                        target_idx + 1,
                        total_targets
                    ),
                    percentage,
                )
                .await;
            }

            let analysis_result = {
                let target = target.clone();
                let project_context = Arc::clone(&project_context);
                tokio::task::spawn_blocking(move || {
                    analyze_single_target(&target, &project_context)
                })
                .await
            };

            match analysis_result {
                Ok(Ok(points)) => {
                    // Filter to new points only (dedup across targets)
                    let new_points: Vec<_> = points
                        .into_iter()
                        .filter(|p| {
                            let key = (p.file.clone(), p.line, p.column.unwrap_or(0));
                            seen.insert(key)
                        })
                        .collect();

                    let point_count = new_points.len();
                    total_points += point_count;

                    self.client
                        .log_message(
                            MessageType::INFO,
                            format!("  {} - {} new panic points", target_name, point_count),
                        )
                        .await;

                    // Group new points by file and publish incrementally
                    let mut files_updated: std::collections::HashSet<Url> =
                        std::collections::HashSet::new();

                    for point in new_points {
                        // Skip files in target/ directory
                        if point.file.starts_with(&target_dir) {
                            continue;
                        }

                        let raw_path = PathBuf::from(&point.file);
                        let file_path = if raw_path.is_absolute() {
                            raw_path
                        } else {
                            workspace_root.join(raw_path)
                        };

                        if let Ok(uri) = Url::from_file_path(&file_path) {
                            files_updated.insert(uri.clone());
                            points_by_file.entry(uri).or_default().push(point);
                        }
                    }

                    // Publish updated diagnostics for files that changed
                    for uri in files_updated {
                        if let Some(points) = points_by_file.get(&uri) {
                            let diagnostics: Vec<Diagnostic> =
                                points.iter().map(Self::code_point_to_diagnostic).collect();

                            self.client
                                .publish_diagnostics(uri.clone(), diagnostics, None)
                                .await;
                        }
                    }
                }
                Ok(Err(e)) => {
                    self.client
                        .log_message(
                            MessageType::LOG,
                            format!("  {} - skipped: {}", target_name, e),
                        )
                        .await;
                }
                Err(_) => {
                    self.client
                        .log_message(
                            MessageType::WARNING,
                            format!("  {} - analysis failed", target_name),
                        )
                        .await;
                }
            }
        }

        // Update state with final results
        let mut state = self.state.write().await;
        let old_files: std::collections::HashSet<_> = state.panic_points.keys().cloned().collect();
        let new_files: std::collections::HashSet<_> = points_by_file.keys().cloned().collect();
        state.panic_points = points_by_file;
        drop(state);

        // Clear diagnostics for files that no longer have panic points
        for uri in old_files.difference(&new_files) {
            self.client
                .publish_diagnostics(uri.clone(), vec![], None)
                .await;
        }

        // Complete progress indicator
        if let Some(ref token) = progress_token {
            self.progress_end(
                token,
                &format!(
                    "Found {} panic points in {} files",
                    total_points,
                    new_files.len()
                ),
            )
            .await;
        }

        self.client
            .log_message(
                MessageType::INFO,
                format!(
                    "Analysis complete: {} panic points in {} files",
                    total_points,
                    new_files.len()
                ),
            )
            .await;

        // Re-publish diagnostics to files that were opened before/during analysis
        // Snapshot all data while holding the lock once, then publish outside the lock
        let republish: Vec<(Url, Vec<Diagnostic>)> = {
            let state = self.state.read().await;
            state
                .opened_files
                .iter()
                .filter_map(|uri| {
                    state.panic_points.get(uri).map(|points| {
                        let diagnostics =
                            points.iter().map(Self::code_point_to_diagnostic).collect();
                        (uri.clone(), diagnostics)
                    })
                })
                .collect()
        };

        for (uri, diagnostics) in republish {
            if !diagnostics.is_empty() {
                self.client
                    .publish_diagnostics(uri, diagnostics, None)
                    .await;
            }
        }
        true
    }

    /// Create a code action that inserts or extends an inline allow comment.
    /// If the line already has a `// jonesy:allow(...)` comment, the new cause
    /// is merged into the existing parenthesised list instead of appending a
    /// second comment.
    fn create_inline_allow_action(
        uri: &Url,
        range: Range,
        cause: &str,
        diagnostic: &Diagnostic,
    ) -> Option<CodeAction> {
        let title = if cause == "*" {
            "Allow all panics on this line".to_string()
        } else {
            format!("Allow '{}' on this line", cause)
        };

        // Try to read the line to check for an existing jonesy:allow comment
        // Accept both "// jonesy:allow(" and "// jonesy: allow("
        let existing_allow = uri.to_file_path().ok().and_then(|path| {
            let content = std::fs::read_to_string(&path).ok()?;
            let line = content.lines().nth(range.start.line as usize)?;
            let allow_start = line
                .find("// jonesy:allow(")
                .or_else(|| line.find("// jonesy: allow("))?;
            let paren_start = line[allow_start..].find('(')? + allow_start + 1;
            let rest = &line[paren_start..];
            let paren_end = rest.find(')')?;
            let existing_causes = &rest[..paren_end];
            Some((
                allow_start as u32,
                existing_causes.to_string(),
                line.len() as u32,
            ))
        });

        let edit = if let Some((col_start, existing_causes, _line_len)) = existing_allow {
            // Merge: replace the existing comment with combined causes
            let mut causes: Vec<&str> = existing_causes.split(',').map(|s| s.trim()).collect();
            if !causes.contains(&cause) {
                causes.push(cause);
            }
            let merged = causes.join(", ");
            let comment = format!("// jonesy:allow({})", merged);
            TextEdit {
                range: Range {
                    start: Position {
                        line: range.start.line,
                        character: col_start,
                    },
                    end: Position {
                        line: range.start.line,
                        character: 10000, // Replace to end of line
                    },
                },
                new_text: comment,
            }
        } else {
            // No existing comment — append new one at end of line
            TextEdit {
                range: Range {
                    start: Position {
                        line: range.start.line,
                        character: 10000,
                    },
                    end: Position {
                        line: range.start.line,
                        character: 10000,
                    },
                },
                new_text: format!(" // jonesy:allow({})", cause),
            }
        };

        let mut changes = HashMap::new();
        changes.insert(uri.clone(), vec![edit]);

        Some(CodeAction {
            title,
            kind: Some(CodeActionKind::QUICKFIX),
            diagnostics: Some(vec![diagnostic.clone()]),
            edit: Some(WorkspaceEdit {
                changes: Some(changes),
                document_changes: None,
                change_annotations: None,
            }),
            command: None,
            is_preferred: Some(cause != "*"), // Prefer specific cause over wildcard
            disabled: None,
            data: None,
        })
    }

    /// Create a code action that adds a file-scoped rule to jonesy.toml.
    fn create_file_allow_action(
        uri: &Url,
        cause: &str,
        workspace_root: &Path,
        diagnostic: &Diagnostic,
    ) -> Option<CodeAction> {
        // Extract filename from URI
        let filename = uri.path().rsplit('/').next()?;

        let title = format!("Allow '{}' in {}", cause, filename);
        let rule_text = format!(
            "\n[[rules]]\npath = \"**/{}\"\nallow = [\"{}\"]\n",
            filename, cause
        );

        let jonesy_toml_path = workspace_root.join("jonesy.toml");
        let jonesy_toml_uri = Url::from_file_path(&jonesy_toml_path).ok()?;

        // Check if jonesy.toml exists and get its length
        let (file_exists, file_length) = if jonesy_toml_path.exists() {
            let content = std::fs::read_to_string(&jonesy_toml_path).unwrap_or_default();
            let lines = content.lines().count() as u32;
            (true, lines)
        } else {
            (false, 0)
        };

        let mut document_changes = Vec::new();

        // If file doesn't exist, create it first
        if !file_exists {
            document_changes.push(DocumentChangeOperation::Op(ResourceOp::Create(
                CreateFile {
                    uri: jonesy_toml_uri.clone(),
                    options: Some(CreateFileOptions {
                        overwrite: Some(false),
                        ignore_if_exists: Some(true),
                    }),
                    annotation_id: None,
                },
            )));
        }

        // Add the rule to the file
        let edit = TextEdit {
            range: Range {
                start: Position {
                    line: file_length,
                    character: 0,
                },
                end: Position {
                    line: file_length,
                    character: 0,
                },
            },
            new_text: rule_text,
        };

        document_changes.push(DocumentChangeOperation::Edit(TextDocumentEdit {
            text_document: OptionalVersionedTextDocumentIdentifier {
                uri: jonesy_toml_uri,
                version: None,
            },
            edits: vec![OneOf::Left(edit)],
        }));

        Some(CodeAction {
            title,
            kind: Some(CodeActionKind::QUICKFIX),
            diagnostics: Some(vec![diagnostic.clone()]),
            edit: Some(WorkspaceEdit {
                changes: None,
                document_changes: Some(DocumentChanges::Operations(document_changes)),
                change_annotations: None,
            }),
            command: None,
            is_preferred: Some(false),
            disabled: None,
            data: None,
        })
    }

    /// Create a code action that adds a function-scoped rule to jonesy.toml.
    fn create_function_allow_action(
        function: &str,
        cause: &str,
        workspace_root: &Path,
        diagnostic: &Diagnostic,
    ) -> Option<CodeAction> {
        // Use full function path for precise matching
        let title = format!("Allow '{}' in this function", cause);
        let rule_text = format!(
            "\n[[rules]]\nfunction = \"{}\"\nallow = [\"{}\"]\n",
            function, cause
        );

        let jonesy_toml_path = workspace_root.join("jonesy.toml");
        let jonesy_toml_uri = Url::from_file_path(&jonesy_toml_path).ok()?;

        // Check if jonesy.toml exists and get its length
        let (file_exists, file_length) = if jonesy_toml_path.exists() {
            let content = std::fs::read_to_string(&jonesy_toml_path).unwrap_or_default();
            let lines = content.lines().count() as u32;
            (true, lines)
        } else {
            (false, 0)
        };

        let mut document_changes = Vec::new();

        // If file doesn't exist, create it first
        if !file_exists {
            document_changes.push(DocumentChangeOperation::Op(ResourceOp::Create(
                CreateFile {
                    uri: jonesy_toml_uri.clone(),
                    options: Some(CreateFileOptions {
                        overwrite: Some(false),
                        ignore_if_exists: Some(true),
                    }),
                    annotation_id: None,
                },
            )));
        }

        // Add the rule to the file
        let edit = TextEdit {
            range: Range {
                start: Position {
                    line: file_length,
                    character: 0,
                },
                end: Position {
                    line: file_length,
                    character: 0,
                },
            },
            new_text: rule_text,
        };

        document_changes.push(DocumentChangeOperation::Edit(TextDocumentEdit {
            text_document: OptionalVersionedTextDocumentIdentifier {
                uri: jonesy_toml_uri,
                version: None,
            },
            edits: vec![OneOf::Left(edit)],
        }));

        Some(CodeAction {
            title,
            kind: Some(CodeActionKind::QUICKFIX),
            diagnostics: Some(vec![diagnostic.clone()]),
            edit: Some(WorkspaceEdit {
                changes: None,
                document_changes: Some(DocumentChanges::Operations(document_changes)),
                change_annotations: None,
            }),
            command: None,
            is_preferred: Some(false),
            disabled: None,
            data: None,
        })
    }

    /// Create a code action that adds a function-scoped rule to jonesy.toml
    /// to silence all panics from calls to a specific function.
    /// This is for indirect panics where the panic originates in a called function.
    fn create_called_function_allow_action(
        called_function: &str,
        cause: &str,
        workspace_root: &Path,
        diagnostic: &Diagnostic,
    ) -> Option<CodeAction> {
        let title = format!("Allow '{cause}' on calls to '{called_function}()'");
        let rule_text =
            format!("\n[[rules]]\nfunction = \"{called_function}\"\nallow = [\"{cause}\"]\n");

        let jonesy_toml_path = workspace_root.join("jonesy.toml");
        let jonesy_toml_uri = Url::from_file_path(&jonesy_toml_path).ok()?;

        let (file_exists, file_length) = if jonesy_toml_path.exists() {
            let content = std::fs::read_to_string(&jonesy_toml_path).unwrap_or_default();
            let lines = content.lines().count() as u32;
            (true, lines)
        } else {
            (false, 0)
        };

        let mut document_changes = Vec::new();

        if !file_exists {
            document_changes.push(DocumentChangeOperation::Op(ResourceOp::Create(
                CreateFile {
                    uri: jonesy_toml_uri.clone(),
                    options: Some(CreateFileOptions {
                        overwrite: Some(false),
                        ignore_if_exists: Some(true),
                    }),
                    annotation_id: None,
                },
            )));
        }

        let edit = TextEdit {
            range: Range {
                start: Position {
                    line: file_length,
                    character: 0,
                },
                end: Position {
                    line: file_length,
                    character: 0,
                },
            },
            new_text: rule_text,
        };

        document_changes.push(DocumentChangeOperation::Edit(TextDocumentEdit {
            text_document: OptionalVersionedTextDocumentIdentifier {
                uri: jonesy_toml_uri,
                version: None,
            },
            edits: vec![OneOf::Left(edit)],
        }));

        Some(CodeAction {
            title,
            kind: Some(CodeActionKind::QUICKFIX),
            diagnostics: Some(vec![diagnostic.clone()]),
            edit: Some(WorkspaceEdit {
                changes: None,
                document_changes: Some(DocumentChanges::Operations(document_changes)),
                change_annotations: None,
            }),
            command: None,
            is_preferred: Some(false),
            disabled: None,
            data: None,
        })
    }

    /// Create a code action that adds a module-scoped rule to jonesy.toml.
    /// Returns the path pattern (e.g., "**/tests/**") and whether it applies.
    fn get_module_pattern(uri: &Url) -> Option<(&'static str, &'static str)> {
        let path = uri.path();
        // Check for common test directories/files
        if path.contains("/tests/") {
            Some(("**/tests/**", "tests"))
        } else if path.contains("/benches/") {
            Some(("**/benches/**", "benches"))
        } else if path.contains("/examples/") {
            Some(("**/examples/**", "examples"))
        } else if path.ends_with("_test.rs") {
            Some(("**/*_test.rs", "test files"))
        } else if path.ends_with("_tests.rs") {
            Some(("**/*_tests.rs", "test files"))
        } else {
            None
        }
    }

    fn create_module_allow_action(
        uri: &Url,
        cause: &str,
        workspace_root: &Path,
        diagnostic: &Diagnostic,
    ) -> Option<CodeAction> {
        let (pattern, module_name) = Self::get_module_pattern(uri)?;

        let title = format!("Allow '{}' in {}", cause, module_name);
        let rule_text = format!(
            "\n[[rules]]\npath = \"{}\"\nallow = [\"{}\"]\n",
            pattern, cause
        );

        let jonesy_toml_path = workspace_root.join("jonesy.toml");
        let jonesy_toml_uri = Url::from_file_path(&jonesy_toml_path).ok()?;

        // Check if jonesy.toml exists and get its length
        let (file_exists, file_length) = if jonesy_toml_path.exists() {
            let content = std::fs::read_to_string(&jonesy_toml_path).unwrap_or_default();
            let lines = content.lines().count() as u32;
            (true, lines)
        } else {
            (false, 0)
        };

        let mut document_changes = Vec::new();

        // If file doesn't exist, create it first
        if !file_exists {
            document_changes.push(DocumentChangeOperation::Op(ResourceOp::Create(
                CreateFile {
                    uri: jonesy_toml_uri.clone(),
                    options: Some(CreateFileOptions {
                        overwrite: Some(false),
                        ignore_if_exists: Some(true),
                    }),
                    annotation_id: None,
                },
            )));
        }

        // Add the rule to the file
        let edit = TextEdit {
            range: Range {
                start: Position {
                    line: file_length,
                    character: 0,
                },
                end: Position {
                    line: file_length,
                    character: 0,
                },
            },
            new_text: rule_text,
        };

        document_changes.push(DocumentChangeOperation::Edit(TextDocumentEdit {
            text_document: OptionalVersionedTextDocumentIdentifier {
                uri: jonesy_toml_uri,
                version: None,
            },
            edits: vec![OneOf::Left(edit)],
        }));

        Some(CodeAction {
            title,
            kind: Some(CodeActionKind::QUICKFIX),
            diagnostics: Some(vec![diagnostic.clone()]),
            edit: Some(WorkspaceEdit {
                changes: None,
                document_changes: Some(DocumentChanges::Operations(document_changes)),
                change_annotations: None,
            }),
            command: None,
            is_preferred: Some(false),
            disabled: None,
            data: None,
        })
    }

    /// Create a code action that adds a crate-level allow rule to jonesy.toml.
    fn create_crate_allow_action(
        cause: &str,
        workspace_root: &Path,
        diagnostic: &Diagnostic,
    ) -> Option<CodeAction> {
        let title = format!("Allow '{}' in this crate", cause);

        let jonesy_toml_path = workspace_root.join("jonesy.toml");
        let jonesy_toml_uri = Url::from_file_path(&jonesy_toml_path).ok()?;

        // Read existing content to check if allow already exists
        let (file_exists, existing_content) = if jonesy_toml_path.exists() {
            let content = std::fs::read_to_string(&jonesy_toml_path).unwrap_or_default();
            (true, content)
        } else {
            (false, String::new())
        };

        // Build the new allow line
        let new_allow = format!("allow = [\"{}\"]", cause);

        // Find where sections start (root-level keys must appear before any [section])
        let first_section_pos = existing_content
            .find("\n[")
            .or_else(|| {
                if existing_content.starts_with('[') {
                    Some(0)
                } else {
                    None
                }
            })
            .unwrap_or(existing_content.len());

        // Only look for root-level allow (before any section headers)
        let root_content = &existing_content[..first_section_pos];

        // Determine the edit based on existing content
        let (edit_range, new_text) = if let Some(start_idx) = root_content.find("allow = [") {
            // Update existing root-level allow line
            let prefix = &existing_content[..start_idx];
            let line_num = prefix.lines().count() as u32;
            let line_start = prefix.rfind('\n').map(|i| i + 1).unwrap_or(0);
            let char_offset = (start_idx - line_start) as u32;

            // Find the end of the allow array
            if let Some(end_bracket) = existing_content[start_idx..].find(']') {
                let end_pos = start_idx + end_bracket + 1;
                let old_allow = &existing_content[start_idx..end_pos];

                // Parse existing causes and add new one if not present
                let mut causes: Vec<String> = old_allow
                    .trim_start_matches("allow = [")
                    .trim_end_matches(']')
                    .split(',')
                    .map(|s| s.trim().trim_matches('"').to_string())
                    .filter(|s| !s.is_empty())
                    .collect();

                if causes.contains(&cause.to_string()) {
                    return None; // Already allowed
                }
                causes.push(cause.to_string());

                let new_allow_line = format!(
                    "allow = [{}]",
                    causes
                        .iter()
                        .map(|c| format!("\"{}\"", c))
                        .collect::<Vec<_>>()
                        .join(", ")
                );

                let end_line = existing_content[..end_pos].lines().count() as u32 - 1;
                let end_char =
                    (end_pos - existing_content[..end_pos].rfind('\n').unwrap_or(0)) as u32;

                (
                    Range {
                        start: Position {
                            line: line_num.saturating_sub(1),
                            character: char_offset,
                        },
                        end: Position {
                            line: end_line,
                            character: end_char,
                        },
                    },
                    new_allow_line,
                )
            } else {
                return None;
            }
        } else {
            // Add new allow line at the beginning of the file
            (
                Range {
                    start: Position {
                        line: 0,
                        character: 0,
                    },
                    end: Position {
                        line: 0,
                        character: 0,
                    },
                },
                format!("{}\n", new_allow),
            )
        };

        let mut document_changes = Vec::new();

        // If file doesn't exist, create it first
        if !file_exists {
            document_changes.push(DocumentChangeOperation::Op(ResourceOp::Create(
                CreateFile {
                    uri: jonesy_toml_uri.clone(),
                    options: Some(CreateFileOptions {
                        overwrite: Some(false),
                        ignore_if_exists: Some(true),
                    }),
                    annotation_id: None,
                },
            )));
        }

        let edit = TextEdit {
            range: edit_range,
            new_text,
        };

        document_changes.push(DocumentChangeOperation::Edit(TextDocumentEdit {
            text_document: OptionalVersionedTextDocumentIdentifier {
                uri: jonesy_toml_uri,
                version: None,
            },
            edits: vec![OneOf::Left(edit)],
        }));

        Some(CodeAction {
            title,
            kind: Some(CodeActionKind::QUICKFIX),
            diagnostics: Some(vec![diagnostic.clone()]),
            edit: Some(WorkspaceEdit {
                changes: None,
                document_changes: Some(DocumentChanges::Operations(document_changes)),
                change_annotations: None,
            }),
            command: None,
            is_preferred: Some(false),
            disabled: None,
            data: None,
        })
    }
}

#[tower_lsp::async_trait]
impl LanguageServer for JonesyLspServer {
    async fn initialize(&self, params: InitializeParams) -> Result<InitializeResult> {
        // Store workspace root - try root_uri first, then fallback to workspace_folders
        let workspace_path = if let Some(root_uri) = params.root_uri {
            root_uri.to_file_path().ok()
        } else if let Some(folders) = params.workspace_folders {
            folders.first().and_then(|f| f.uri.to_file_path().ok())
        } else {
            None
        };

        if let Some(path) = workspace_path {
            let mut state = self.state.write().await;
            state.workspace_root = Some(path);
        }

        Ok(InitializeResult {
            capabilities: ServerCapabilities {
                text_document_sync: Some(TextDocumentSyncCapability::Options(
                    TextDocumentSyncOptions {
                        open_close: Some(true),
                        change: Some(TextDocumentSyncKind::INCREMENTAL),
                        will_save: None,
                        will_save_wait_until: None,
                        save: Some(TextDocumentSyncSaveOptions::Supported(true)),
                    },
                )),
                execute_command_provider: Some(ExecuteCommandOptions {
                    commands: vec!["jonesy.analyze".to_string()],
                    work_done_progress_options: WorkDoneProgressOptions::default(),
                }),
                code_action_provider: Some(CodeActionProviderCapability::Simple(true)),
                ..Default::default()
            },
            server_info: Some(ServerInfo {
                name: "jonesy".to_string(),
                version: Some(crate::args::VERSION.to_string()),
            }),
        })
    }

    async fn initialized(&self, _: InitializedParams) {
        self.client
            .log_message(MessageType::INFO, "Jonesy LSP server initialized")
            .await;

        // Start native file watcher for reliable binary change detection
        let native_watcher_started = self.start_native_file_watcher().await;

        // Only register LSP file watchers as fallback if native watcher failed
        if !native_watcher_started {
            self.client
                .log_message(
                    MessageType::INFO,
                    "Falling back to LSP file watchers (may not work for binary changes)",
                )
                .await;
            self.register_file_watchers().await;
        }

        // Run initial analysis
        self.analyze_and_publish().await;
    }

    async fn shutdown(&self) -> Result<()> {
        Ok(())
    }

    async fn did_open(&self, params: DidOpenTextDocumentParams) {
        let uri = params.text_document.uri;

        // Track opened file for re-publishing diagnostics
        self.state.write().await.opened_files.insert(uri.clone());

        // If we have cached diagnostics for this file, publish them now
        let state = self.state.read().await;
        if let Some(points) = state.panic_points.get(&uri) {
            let diagnostics: Vec<Diagnostic> =
                points.iter().map(Self::code_point_to_diagnostic).collect();
            drop(state); // Release lock before async call

            if !diagnostics.is_empty() {
                self.client
                    .publish_diagnostics(uri, diagnostics, None)
                    .await;
            }
        }
    }

    async fn did_change(&self, _params: DidChangeTextDocumentParams) {
        // No-op: we analyze binaries, not source text
    }

    async fn did_close(&self, params: DidCloseTextDocumentParams) {
        // Remove from opened files tracking
        self.state
            .write()
            .await
            .opened_files
            .remove(&params.text_document.uri);
    }

    async fn did_save(&self, _params: DidSaveTextDocumentParams) {
        // No-op: we watch target/debug/ for binary changes instead of
        // re-analyzing on every file save. This avoids redundant analysis
        // when the user saves files without building.
        //
        // Analysis is triggered by:
        // 1. did_change_watched_files (when binaries or config files change)
        // 2. Manual "jonesy.analyze" command
    }

    async fn did_change_watched_files(&self, params: DidChangeWatchedFilesParams) {
        // Re-analyze when watched files change (binaries or config files)
        let changed_paths: Vec<_> = params
            .changes
            .iter()
            .filter_map(|c| c.uri.to_file_path().ok())
            .collect();

        if changed_paths.is_empty() {
            return;
        }

        // Categorize changes for logging
        let config_changes: Vec<_> = changed_paths
            .iter()
            .filter(|p| {
                p.file_name()
                    .map(|n| n == "jonesy.toml" || n == "Cargo.toml")
                    .unwrap_or(false)
            })
            .filter_map(|p| p.file_name().map(|n| n.to_string_lossy().to_string()))
            .collect();

        let binary_changes: Vec<_> = changed_paths
            .iter()
            .filter(|p| {
                p.file_name()
                    .map(|n| n != "jonesy.toml" && n != "Cargo.toml")
                    .unwrap_or(true)
            })
            .filter_map(|p| p.file_name().map(|n| n.to_string_lossy().to_string()))
            .collect();

        // Log what changed
        if !config_changes.is_empty() {
            self.client
                .log_message(
                    MessageType::INFO,
                    format!("Config changes detected: {}", config_changes.join(", ")),
                )
                .await;

            // Re-register watchers in case workspace membership changed
            // (e.g., Cargo.toml added/removed workspace members)
            self.register_file_watchers().await;
        }
        if !binary_changes.is_empty() {
            self.client
                .log_message(
                    MessageType::INFO,
                    format!("Binary changes detected: {}", binary_changes.join(", ")),
                )
                .await;
        }

        self.analyze_and_publish().await;
    }

    async fn code_action(&self, params: CodeActionParams) -> Result<Option<CodeActionResponse>> {
        let mut actions: Vec<CodeActionOrCommand> = Vec::new();

        // Get workspace root for jonesy.toml path
        let workspace_root = {
            let state = self.state.read().await;
            state.workspace_root.clone()
        };

        // Filter to jonesy diagnostics only
        let jonesy_diagnostics: Vec<_> = params
            .context
            .diagnostics
            .iter()
            .filter(|d| d.source.as_deref() == Some("jonesy"))
            .collect();

        for diag in jonesy_diagnostics {
            // Extract cause info from diagnostic data
            if let Some(data) = &diag.data {
                let causes: Vec<String> = data
                    .get("causes")
                    .and_then(|v| serde_json::from_value(v.clone()).ok())
                    .unwrap_or_default();
                let function: String = data
                    .get("function")
                    .and_then(|v| v.as_str())
                    .unwrap_or("")
                    .to_string();
                let called_function: Option<String> = data
                    .get("called_function")
                    .and_then(|v| v.as_str())
                    .map(|s| s.to_string());

                // Track which causes we've added actions for (avoid duplicates)
                let mut seen_causes = std::collections::HashSet::new();

                for cause in &causes {
                    if !seen_causes.insert(cause.clone()) {
                        continue;
                    }

                    // Action 1: Allow on this line (inline comment)
                    if let Some(action) = Self::create_inline_allow_action(
                        &params.text_document.uri,
                        diag.range,
                        cause,
                        diag,
                    ) {
                        actions.push(CodeActionOrCommand::CodeAction(action));
                    }

                    // Action 2: Allow in this file (scoped rule in jonesy.toml)
                    if let Some(ref root) = workspace_root {
                        if let Some(action) = Self::create_file_allow_action(
                            &params.text_document.uri,
                            cause,
                            root,
                            diag,
                        ) {
                            actions.push(CodeActionOrCommand::CodeAction(action));
                        }

                        // Action 3: Allow in this function (scoped rule in jonesy.toml)
                        if !function.is_empty() {
                            if let Some(action) =
                                Self::create_function_allow_action(&function, cause, root, diag)
                            {
                                actions.push(CodeActionOrCommand::CodeAction(action));
                            }
                        }

                        // Action 4: Allow this cause on calls to called function
                        if let Some(ref called_fn) = called_function {
                            if let Some(action) = Self::create_called_function_allow_action(
                                called_fn, cause, root, diag,
                            ) {
                                actions.push(CodeActionOrCommand::CodeAction(action));
                            }
                        }

                        // Action 5: Allow in module (tests, benches, examples)
                        if let Some(action) = Self::create_module_allow_action(
                            &params.text_document.uri,
                            cause,
                            root,
                            diag,
                        ) {
                            actions.push(CodeActionOrCommand::CodeAction(action));
                        }

                        // Action 6: Allow in this crate (global allow)
                        if let Some(action) = Self::create_crate_allow_action(cause, root, diag) {
                            actions.push(CodeActionOrCommand::CodeAction(action));
                        }
                    }
                }

                // Action 7: Allow all panics on this line (wildcard)
                if causes.len() > 1 {
                    if let Some(action) = Self::create_inline_allow_action(
                        &params.text_document.uri,
                        diag.range,
                        "*",
                        diag,
                    ) {
                        actions.push(CodeActionOrCommand::CodeAction(action));
                    }
                }
            }
        }

        // Always add the manual analyze action
        let analyze_action = CodeAction {
            title: "Run Jonesy Panic Analysis".to_string(),
            kind: Some(CodeActionKind::SOURCE),
            diagnostics: None,
            edit: None,
            command: Some(Command {
                title: "Run Jonesy Panic Analysis".to_string(),
                command: "jonesy.analyze".to_string(),
                arguments: None,
            }),
            is_preferred: Some(false),
            disabled: None,
            data: None,
        };
        actions.push(CodeActionOrCommand::CodeAction(analyze_action));

        Ok(Some(actions))
    }

    async fn execute_command(
        &self,
        params: ExecuteCommandParams,
    ) -> Result<Option<serde_json::Value>> {
        if params.command == "jonesy.analyze" {
            let success = self.analyze_and_publish().await;
            Ok(Some(serde_json::json!({"success": success})))
        } else {
            Ok(None)
        }
    }
}

/// Run analysis from a spawned task (file watcher callback).
///
/// This is a standalone function that can be called from background tasks
/// without needing a reference to the full JonesyLspServer.
///
/// Uses caching in `target/jonesy/` to avoid re-analyzing unchanged targets.
async fn run_analysis_task(
    client: &Client,
    state: &Arc<RwLock<ServerState>>,
    analysis_lock: &Arc<Mutex<()>>,
) {
    use crate::analysis_cache::{AnalysisCache, build_workspace_state};

    // Serialize analysis runs
    let _guard = analysis_lock.lock().await;

    // Get workspace root
    let workspace_root = {
        let state = state.read().await;
        state.workspace_root.clone()
    };

    let Some(workspace_root) = workspace_root else {
        return;
    };

    let target_dir = workspace_root.join("target").to_string_lossy().to_string();

    // Load analysis cache
    let mut cache = {
        let root = workspace_root.clone();
        tokio::task::spawn_blocking(move || AnalysisCache::load(&root))
            .await
            .unwrap_or_default()
    };

    // Build current workspace state and detect changes
    let current_workspace_state = {
        let root = workspace_root.clone();
        tokio::task::spawn_blocking(move || build_workspace_state(&root))
            .await
            .unwrap_or_default()
    };

    let workspace_changes = cache.detect_workspace_changes(&current_workspace_state);
    let mut force_full_analysis = workspace_changes.needs_full_reanalysis();

    // Check if config files (jonesy.toml, Cargo.toml) have changed.
    // Config changes affect all targets, so force a full re-analysis.
    // Snapshot current hashes NOW so we persist exactly what we analyzed against,
    // even if the file changes again during the analysis run.
    let config_files = find_config_files(&workspace_root);
    let mut config_snapshots: Vec<(PathBuf, u64)> = Vec::new();
    for config_path in &config_files {
        let config_deleted = !config_path.exists() && cache.has_config(config_path);
        if config_deleted || (config_path.exists() && cache.config_changed(config_path)) {
            client
                .log_message(
                    MessageType::INFO,
                    format!(
                        "Config changed: {}",
                        config_path
                            .file_name()
                            .unwrap_or_default()
                            .to_string_lossy()
                    ),
                )
                .await;
            force_full_analysis = true;
            if config_deleted {
                cache.remove_config(config_path);
            } else {
                // Snapshot the hash now, before analysis starts
                let hash = crate::analysis_cache::hash_file_content(config_path).unwrap_or(0);
                config_snapshots.push((config_path.clone(), hash));
            }
        }
    }

    if workspace_changes.has_changes() {
        let change_summary = format_change_summary(&workspace_changes, &current_workspace_state);
        client.log_message(MessageType::INFO, change_summary).await;
    }

    // Get targets
    let targets = {
        let root = workspace_root.clone();
        tokio::task::spawn_blocking(move || find_workspace_binaries(&root))
            .await
            .ok()
            .and_then(|r| r.ok())
            .unwrap_or_default()
    };

    if targets.is_empty() {
        return;
    }

    // Build ProjectContext once for the workspace
    let project_context = {
        let root = workspace_root.clone();
        tokio::task::spawn_blocking(move || ProjectContext::from_project_root(&root))
            .await
            .unwrap_or_else(|e| Err(format!("Failed to build project context: {e}")))
    };
    let project_context = match project_context {
        Ok(ctx) => Arc::new(ctx),
        Err(e) => {
            client
                .log_message(MessageType::ERROR, format!("ProjectContext error: {e}"))
                .await;
            return;
        }
    };

    // Track diagnostics
    let mut points_by_file: HashMap<Url, Vec<CrateCodePoint>> = HashMap::new();
    let mut seen: HashSet<(String, u32, u32)> = HashSet::new();
    let mut analyzed_count = 0usize;
    let mut skipped_count = 0usize;

    // Analyze each target (skip unchanged ones unless force_full_analysis or workspace changes affect it)
    for target in &targets {
        // Check if target needs re-analysis
        let needs_analysis = force_full_analysis
            || cache.target_needs_analysis(target)
            || workspace_changes.affects_target(target);

        if !needs_analysis {
            skipped_count += 1;
            continue;
        }

        analyzed_count += 1;
        let analysis_result = {
            let target = target.clone();
            let project_context = Arc::clone(&project_context);
            tokio::task::spawn_blocking(move || analyze_single_target(&target, &project_context))
                .await
        };

        if let Ok(Ok(points)) = analysis_result {
            let point_count = points.len();
            let new_points: Vec<_> = points
                .into_iter()
                .filter(|p| {
                    let key = (p.file.clone(), p.line, p.column.unwrap_or(0));
                    seen.insert(key)
                })
                .collect();

            // Update cache for this target
            cache.update_target(target, point_count);

            // Group by file
            for point in new_points {
                if point.file.starts_with(&target_dir) {
                    continue;
                }

                let raw_path = PathBuf::from(&point.file);
                let file_path = if raw_path.is_absolute() {
                    raw_path
                } else {
                    workspace_root.join(raw_path)
                };

                if let Ok(uri) = Url::from_file_path(&file_path) {
                    points_by_file.entry(uri).or_default().push(point);
                }
            }
        }
    }

    // Persist the config hashes snapshotted BEFORE analysis started.
    // Using the pre-analysis snapshot (not current on-disk content) ensures that
    // if the config changed again during the run, the next watcher event will
    // still detect that change and re-analyze.
    for (config_path, hash) in &config_snapshots {
        cache.update_config_with_hash(config_path, *hash);
    }

    // Update workspace state in cache and save
    cache.update_workspace(current_workspace_state);
    cache.prune_stale_targets();
    let save_result = {
        let root = workspace_root.clone();
        tokio::task::spawn_blocking(move || cache.save(&root)).await
    };
    if let Ok(Err(e)) = save_result {
        client
            .log_message(MessageType::WARNING, format!("Failed to save cache: {}", e))
            .await;
    }

    // Log analysis summary
    if skipped_count > 0 {
        client
            .log_message(
                MessageType::LOG,
                format!(
                    "Analyzed {} targets, skipped {} unchanged",
                    analyzed_count, skipped_count
                ),
            )
            .await;
    }

    // Update state and publish diagnostics
    // When some targets were skipped, merge with existing state to preserve their diagnostics
    let old_files: HashSet<_>;
    let new_files: HashSet<_> = points_by_file.keys().cloned().collect();

    {
        let mut state = state.write().await;
        old_files = state.panic_points.keys().cloned().collect();

        if skipped_count > 0 {
            // Merge: update analyzed files, keep diagnostics from skipped targets
            for (uri, points) in points_by_file.clone() {
                state.panic_points.insert(uri, points);
            }
        } else {
            // Full analysis: replace entirely
            state.panic_points = points_by_file.clone();
        }
    }

    // Publish diagnostics for files with panic points
    for (uri, points) in &points_by_file {
        let diagnostics: Vec<Diagnostic> = points
            .iter()
            .map(JonesyLspServer::code_point_to_diagnostic)
            .collect();
        client
            .publish_diagnostics(uri.clone(), diagnostics, None)
            .await;
    }

    // Clear diagnostics for files that no longer have panic points
    // Only do this when all targets were analyzed (no skipped targets)
    if skipped_count == 0 {
        for uri in old_files.difference(&new_files) {
            client.publish_diagnostics(uri.clone(), vec![], None).await;
        }
    }

    // Report deduplicated totals from state (includes merged results from skipped targets)
    let (total_files, published_points) = {
        let state = state.read().await;
        (
            state.panic_points.len(),
            state
                .panic_points
                .values()
                .map(|points| points.len())
                .sum::<usize>(),
        )
    };

    client
        .log_message(
            MessageType::INFO,
            format!(
                "Analysis complete: {} panic points in {} files",
                published_points, total_files
            ),
        )
        .await;
}

/// Info about workspace structure (for logging before analysis)
struct WorkspaceInfo {
    members: Vec<String>,
    targets: Vec<String>,
}

// ============================================================================
// Pure helper functions - extracted for unit testing
// These can be used in analyze_and_publish/run_analysis_task in the future
// to reduce code duplication. For now they're tested independently.
// ============================================================================

/// Format a workspace/package change summary for logging.
/// Single-package crates get "Package changes: ..." (no members count),
/// workspaces get "Workspace changes: ... members, ...".
fn format_change_summary(
    changes: &crate::analysis_cache::WorkspaceChanges,
    workspace_state: &crate::analysis_cache::WorkspaceState,
) -> String {
    let (members, binaries, libraries) = changes.change_counts();
    if workspace_state.is_single_package() {
        format!(
            "Package changes: {} binaries, {} library affected",
            binaries, libraries,
        )
    } else {
        format!(
            "Workspace changes: {} members, {} binaries, {} libraries affected",
            members, binaries, libraries,
        )
    }
}

/// Deduplicate code points by (file, line, column).
///
/// Points are considered duplicates if they have the same file path, line number,
/// and column (or both have no column). This handles the case where multiple
/// targets report the same panic point from shared code.
#[cfg(test)]
fn deduplicate_code_points(points: Vec<CrateCodePoint>) -> Vec<CrateCodePoint> {
    let mut seen: HashSet<(String, u32, u32)> = HashSet::new();
    points
        .into_iter()
        .filter(|p| {
            let key = (p.file.clone(), p.line, p.column.unwrap_or(0));
            seen.insert(key)
        })
        .collect()
}

/// Group code points by file URI, filtering out paths in target/ directory.
///
/// Returns a HashMap where keys are file URIs and values are the code points
/// found in that file. Points with paths inside the target directory are excluded.
#[cfg(test)]
fn group_points_by_uri(
    points: Vec<CrateCodePoint>,
    workspace_root: &Path,
    target_dir: &str,
) -> HashMap<Url, Vec<CrateCodePoint>> {
    let mut points_by_file: HashMap<Url, Vec<CrateCodePoint>> = HashMap::new();

    for point in points {
        // Skip files in target/ directory
        if point.file.starts_with(target_dir) {
            continue;
        }

        let raw_path = PathBuf::from(&point.file);
        let file_path = if raw_path.is_absolute() {
            raw_path
        } else {
            workspace_root.join(raw_path)
        };

        if let Ok(uri) = Url::from_file_path(&file_path) {
            points_by_file.entry(uri).or_default().push(point);
        }
    }

    points_by_file
}

/// Result of analyzing workspace targets.
#[cfg(test)]
#[derive(Debug, Default)]
struct AnalysisResult {
    /// All code points found, deduplicated across targets
    points: Vec<CrateCodePoint>,
    /// Total number of panic points found (before deduplication)
    total_count: usize,
    /// Number of targets that were successfully analyzed
    analyzed_count: usize,
    /// Number of targets that failed or were skipped
    skipped_count: usize,
}

/// Analyze all targets in a workspace and return deduplicated results.
///
/// This is the core analysis logic extracted from `analyze_and_publish` and
/// `run_analysis_task` for easier testing. It runs analysis on each target
/// synchronously and returns all unique panic points.
#[cfg(test)]
fn analyze_workspace_targets(
    targets: &[PathBuf],
    project_context: &ProjectContext,
) -> AnalysisResult {
    let mut result = AnalysisResult::default();
    let mut seen: HashSet<(String, u32, u32)> = HashSet::new();

    for target in targets {
        match analyze_single_target(target, project_context) {
            Ok(points) => {
                result.analyzed_count += 1;
                let point_count = points.len();
                result.total_count += point_count;

                // Filter to new points only (dedup across targets)
                let new_points: Vec<_> = points
                    .into_iter()
                    .filter(|p| {
                        let key = (p.file.clone(), p.line, p.column.unwrap_or(0));
                        seen.insert(key)
                    })
                    .collect();

                result.points.extend(new_points);
            }
            Err(_) => {
                result.skipped_count += 1;
            }
        }
    }

    result
}

/// Quickly discover workspace structure without running full analysis
fn discover_workspace(workspace_root: &Path) -> Option<WorkspaceInfo> {
    let cargo_toml = workspace_root.join("Cargo.toml");
    let content = std::fs::read_to_string(&cargo_toml).ok()?;
    let manifest = cargo_toml::Manifest::from_slice(content.as_bytes()).ok()?;

    let mut members = Vec::new();
    let mut targets = Vec::new();

    // Get workspace members
    if let Some(workspace) = &manifest.workspace {
        for member in &workspace.members {
            if member.contains('*') {
                // Expand glob
                for path in expand_workspace_glob(workspace_root, member) {
                    if let Some(name) = path.file_name() {
                        members.push(name.to_string_lossy().to_string());
                    }
                }
            } else {
                members.push(member.clone());
            }
        }
    } else if let Some(pkg) = &manifest.package {
        members.push(pkg.name.clone());
    }

    // Get targets
    if let Ok(found_targets) = find_workspace_binaries(workspace_root) {
        for target in found_targets {
            if let Some(name) = target.file_name() {
                targets.push(name.to_string_lossy().to_string());
            }
        }
    }

    Some(WorkspaceInfo { members, targets })
}

/// Analyze a single target (binary or library) and return panic points.
/// This reuses the same analysis functions as the CLI for consistency.
fn analyze_single_target(
    target_path: &Path,
    project_context: &ProjectContext,
) -> std::result::Result<Vec<CrateCodePoint>, String> {
    use crate::analysis::{analyze_archive, analyze_binary_target};
    use crate::args::OutputFormat;
    use crate::config::Config;
    use crate::sym::SymbolTable;
    use goblin::mach::Mach::{Binary, Fat};
    use goblin::mach::SingleArch;
    use goblin::mach::constants::cputype::{CPU_TYPE_ARM64, CPU_TYPE_X86_64};

    let binary_buffer =
        std::fs::read(target_path).map_err(|e| format!("Failed to read target: {}", e))?;

    let symbols =
        SymbolTable::from(&binary_buffer).map_err(|e| format!("Failed to read symbols: {}", e))?;

    let config = Config::load_for_project(Path::new(project_context.project_root()), None)
        .unwrap_or_else(|_| Config::with_defaults());

    // Use quiet output format (no progress display in LSP)
    let output = OutputFormat::quiet();

    match &symbols {
        SymbolTable::MachO(Binary(_)) => {
            let result = analyze_binary_target(
                &symbols,
                &binary_buffer,
                target_path,
                false, // show_timings
                &config,
                &output,
                project_context,
            )?;
            Ok(result.code_points)
        }
        SymbolTable::MachO(Fat(fat)) => {
            // Fat binary - find native architecture slice and analyze it
            // Prefer slice matching the current host architecture
            let preferred_cputype = match std::env::consts::ARCH {
                "aarch64" => Some(CPU_TYPE_ARM64),
                "x86_64" => Some(CPU_TYPE_X86_64),
                _ => None,
            };

            // Find host-native slice, or fall back to first available
            let mut selected_macho = None;
            for entry in fat.into_iter() {
                match entry {
                    Ok(SingleArch::MachO(macho)) => {
                        if preferred_cputype
                            .map(|cpu| macho.header.cputype == cpu)
                            .unwrap_or(false)
                        {
                            selected_macho = Some(macho);
                            break;
                        }
                        // Keep first as fallback
                        if selected_macho.is_none() {
                            selected_macho = Some(macho);
                        }
                    }
                    Ok(SingleArch::Archive(_)) => continue, // Skip archive slices
                    Err(_) => continue,
                }
            }

            match selected_macho {
                Some(_macho) => {
                    // Fat binary: re-parse buffer to get SymbolTable for selected arch
                    let fat_symbols = SymbolTable::from(&binary_buffer)
                        .map_err(|e| format!("Failed to parse fat binary: {e}"))?;
                    let result = analyze_binary_target(
                        &fat_symbols,
                        &binary_buffer,
                        target_path,
                        false, // show_timings
                        &config,
                        &output,
                        project_context,
                    )?;
                    Ok(result.code_points)
                }
                None => Err("Fat binary contains no analyzable MachO slices".to_string()),
            }
        }
        SymbolTable::Elf(_) => {
            let result = analyze_binary_target(
                &symbols,
                &binary_buffer,
                target_path,
                false, // show_timings
                &config,
                &output,
                project_context,
            )?;
            Ok(result.code_points)
        }
        SymbolTable::Archive(archive) => {
            let result = analyze_archive(
                archive,
                &binary_buffer,
                target_path,
                false, // show_timings
                &config,
                &output,
                project_context,
            )?;
            Ok(result.code_points)
        }
    }
}

/// Find all config files that affect jonesy analysis.
/// Returns paths to jonesy.toml and all Cargo.toml files (workspace + members).
fn find_config_files(workspace_root: &Path) -> Vec<PathBuf> {
    let mut config_files = Vec::new();

    // Always watch jonesy.toml if it exists (or might be created)
    config_files.push(workspace_root.join("jonesy.toml"));

    // Watch workspace Cargo.toml
    let cargo_toml = workspace_root.join("Cargo.toml");
    if !cargo_toml.exists() {
        return config_files;
    }
    config_files.push(cargo_toml.clone());

    // Parse manifest to find workspace members
    let Ok(content) = std::fs::read_to_string(&cargo_toml) else {
        return config_files;
    };
    let Ok(manifest) = cargo_toml::Manifest::from_slice(content.as_bytes()) else {
        return config_files;
    };

    // Add Cargo.toml for each workspace member
    if let Some(workspace) = &manifest.workspace {
        for member in &workspace.members {
            let member_paths: Vec<PathBuf> = if member.contains('*') {
                expand_workspace_glob(workspace_root, member)
            } else {
                vec![workspace_root.join(member)]
            };

            for member_path in member_paths {
                let member_cargo = member_path.join("Cargo.toml");
                if member_cargo.exists() {
                    config_files.push(member_cargo);
                }
            }
        }
    }

    // Deduplicate paths (can occur with overlapping glob patterns)
    config_files.sort();
    config_files.dedup();
    config_files
}

/// Find binary and library files in the workspace
fn find_workspace_binaries(workspace_root: &Path) -> std::result::Result<Vec<PathBuf>, String> {
    let target_debug = workspace_root.join("target/debug");
    if !target_debug.exists() {
        return Ok(Vec::new());
    }

    // Look for Cargo.toml to find binary names
    let cargo_toml = workspace_root.join("Cargo.toml");
    if !cargo_toml.exists() {
        return Ok(Vec::new());
    }

    let content = std::fs::read_to_string(&cargo_toml)
        .map_err(|e| format!("Failed to read Cargo.toml: {}", e))?;

    let manifest = cargo_toml::Manifest::from_slice(content.as_bytes())
        .map_err(|e| format!("Failed to parse Cargo.toml: {}", e))?;

    let mut targets = Vec::new();

    // Check for package binaries and libraries (non-virtual workspace or single crate)
    if manifest.package.is_some() {
        // Complete the manifest to discover implicit targets (src/main.rs, src/lib.rs, etc.)
        let mut completed_manifest = manifest.clone();
        completed_manifest
            .complete_from_path_and_workspace::<toml::Value>(
                &cargo_toml,
                None::<(&cargo_toml::Manifest<toml::Value>, &std::path::Path)>,
            )
            .map_err(|e| {
                format!(
                    "Failed to complete manifest {}: {}",
                    cargo_toml.display(),
                    e
                )
            })?;
        collect_binaries_from_manifest(&completed_manifest, &target_debug, &mut targets);
    }

    // Check for workspace members
    if let Some(workspace) = &manifest.workspace {
        for member in &workspace.members {
            let member_paths: Vec<PathBuf> = if member.contains('*') {
                // Expand glob patterns like "crates/*"
                expand_workspace_glob(workspace_root, member)
            } else {
                vec![workspace_root.join(member)]
            };

            for member_path in member_paths {
                let member_cargo = member_path.join("Cargo.toml");
                let member_content = match std::fs::read_to_string(&member_cargo) {
                    Ok(content) => content,
                    Err(e) => {
                        eprintln!("Warning: Failed to read {}: {}", member_cargo.display(), e);
                        continue;
                    }
                };
                let mut member_manifest =
                    match cargo_toml::Manifest::from_slice(member_content.as_bytes()) {
                        Ok(m) => m,
                        Err(e) => {
                            eprintln!("Warning: Failed to parse {}: {}", member_cargo.display(), e);
                            continue;
                        }
                    };
                // Complete the manifest to discover implicit targets
                // Continue on error - don't let one bad member break the whole workspace
                if let Err(e) = member_manifest.complete_from_path_and_workspace(
                    &member_cargo,
                    Some((&manifest, cargo_toml.as_path())),
                ) {
                    eprintln!(
                        "Warning: Failed to complete {}: {}",
                        member_cargo.display(),
                        e
                    );
                    continue;
                }
                collect_binaries_from_manifest(&member_manifest, &target_debug, &mut targets);
            }
        }
    }

    Ok(targets)
}

/// Collect binaries from a completed manifest into the targets vector
fn collect_binaries_from_manifest(
    manifest: &cargo_toml::Manifest,
    target_debug: &Path,
    targets: &mut Vec<PathBuf>,
) {
    let Some(pkg) = &manifest.package else {
        return;
    };
    let pkg_name = &pkg.name;

    // Check for [[bin]] targets (populated by complete_from_path_and_workspace)
    // No fallback probe needed - complete_from_path_and_workspace populates bin if there's a binary
    for bin in &manifest.bin {
        let bin_name = bin.name.as_deref().unwrap_or(pkg_name);
        if let Some(bin_path) = find_binary(target_debug, bin_name) {
            if !targets.contains(&bin_path) {
                targets.push(bin_path);
            }
        }
    }

    // Check for library target
    if manifest.lib.is_some() {
        let lib_name = manifest
            .lib
            .as_ref()
            .and_then(|lib| lib.name.as_deref())
            .unwrap_or(pkg_name);
        if let Some(lib_path) = find_library(target_debug, lib_name) {
            if !targets.contains(&lib_path) {
                targets.push(lib_path);
            }
        }
    }
}

/// Expand a workspace glob pattern like "crates/*" or "crates/**" to actual paths
fn expand_workspace_glob(workspace_root: &Path, pattern: &str) -> Vec<PathBuf> {
    // Build full glob pattern rooted at workspace
    let full_pattern = workspace_root.join(pattern);
    let pattern_str = full_pattern.to_string_lossy();

    // Use glob to expand the pattern
    match glob::glob(&pattern_str) {
        Ok(paths) => paths
            .filter_map(|p| p.ok())
            .filter(|p| p.is_dir() && p.join("Cargo.toml").exists())
            .collect(),
        Err(_) => Vec::new(),
    }
}

/// Run the LSP server
pub async fn run_lsp_server() {
    let stdin = tokio::io::stdin();
    let stdout = tokio::io::stdout();

    let (service, socket) = LspService::new(JonesyLspServer::new);
    Server::new(stdin, stdout, socket).serve(service).await;
}

#[cfg(test)]
mod tests {
    use super::*;
    use std::sync::Mutex;

    /// Mutex to serialize tests that build and inspect workspace_test artifacts.
    /// Without this, parallel tests can trigger concurrent cargo builds that
    /// temporarily remove artifacts during rebuilds (especially under coverage).
    static WORKSPACE_TEST_LOCK: Mutex<()> = Mutex::new(());

    /// Find the workspace root by looking for Cargo.toml with [workspace]
    fn find_workspace_root() -> PathBuf {
        let mut current = std::env::current_dir().unwrap();
        loop {
            let cargo_toml = current.join("Cargo.toml");
            if cargo_toml.exists() {
                let content = std::fs::read_to_string(&cargo_toml).unwrap_or_default();
                if content.contains("[workspace]") {
                    return current;
                }
            }
            if !current.pop() {
                panic!("Could not find workspace root");
            }
        }
    }

    /// Build workspace_test, holding WORKSPACE_TEST_LOCK to prevent parallel
    /// tests from interfering with each other's build artifacts.
    fn build_workspace_test(workspace_test_dir: &Path) -> std::sync::MutexGuard<'static, ()> {
        let guard = WORKSPACE_TEST_LOCK.lock().unwrap();

        let status = std::process::Command::new("cargo")
            .arg("build")
            .current_dir(workspace_test_dir)
            .status()
            .expect("Failed to build workspace_test");
        assert!(status.success(), "Failed to build workspace_test");

        // Verify all expected artifacts exist (CI builds take ~2s, wait up to 4s)
        let target_debug = workspace_test_dir.join("target/debug");
        let expected = [
            "crate_a",
            "crate_b_bin",
            "libcrate_b_lib.rlib",
            "libcrate_c.rlib",
        ];

        for _ in 0..8 {
            let all_exist = expected.iter().all(|name| target_debug.join(name).exists());
            if all_exist {
                return guard;
            }
            std::thread::sleep(std::time::Duration::from_millis(500));
        }

        // Final check with diagnostic output
        let missing: Vec<_> = expected
            .iter()
            .filter(|name| !target_debug.join(name).exists())
            .collect();
        if !missing.is_empty() {
            panic!(
                "Build artifacts not found after waiting: {:?}. target/debug contents: {:?}",
                missing,
                std::fs::read_dir(&target_debug)
                    .map(|entries| entries
                        .filter_map(|e| e.ok())
                        .map(|e| e.file_name().to_string_lossy().to_string())
                        .collect::<Vec<_>>())
                    .unwrap_or_default()
            );
        }
        guard
    }

    #[test]
    fn test_find_workspace_binaries_with_custom_lib_name() {
        // Use workspace_test example which has a crate with custom [lib] name
        let workspace_root = find_workspace_root();
        let workspace_test_dir = workspace_root.join("examples").join("workspace_test");

        // Hold the lock through assertions so no parallel test can rebuild artifacts
        let _guard = build_workspace_test(&workspace_test_dir);

        // Find targets
        let targets =
            find_workspace_binaries(&workspace_test_dir).expect("Should find workspace binaries");

        // Extract just the file names for easier comparison
        let target_names: Vec<String> = targets
            .iter()
            .filter_map(|p| p.file_name())
            .map(|n| n.to_string_lossy().to_string())
            .collect();

        // Expected targets:
        // - crate_a (binary from crate_a)
        // - crate_b_bin (binary from crate_b, explicit [[bin]] name)
        // - libcrate_b_lib.rlib (library from crate_b, [lib] name = "crate_b_lib")
        // - libcrate_c.rlib (library-only crate)

        assert!(
            target_names.iter().any(|n| n == "crate_a"),
            "Should find crate_a binary. Found: {:?}",
            target_names
        );
        assert!(
            target_names.iter().any(|n| n == "crate_b_bin"),
            "Should find crate_b_bin binary. Found: {:?}",
            target_names
        );
        assert!(
            target_names.iter().any(|n| n == "libcrate_b_lib.rlib"),
            "Should find libcrate_b_lib.rlib (custom [lib] name). Found: {:?}",
            target_names
        );
        assert!(
            target_names.iter().any(|n| n == "libcrate_c.rlib"),
            "Should find libcrate_c.rlib (library-only crate). Found: {:?}",
            target_names
        );
    }

    #[test]
    fn test_lsp_analysis_matches_cli() {
        use std::collections::HashSet;

        // Use workspace_test example
        let workspace_root = find_workspace_root();
        let workspace_test_dir = workspace_root.join("examples").join("workspace_test");

        // Hold the lock through the entire test so no parallel test can rebuild artifacts
        let _guard = build_workspace_test(&workspace_test_dir);

        // Run CLI and capture panic points
        let cli_output = std::process::Command::new(workspace_root.join("target/debug/jonesy"))
            .arg("--quiet")
            .current_dir(&workspace_test_dir)
            .output()
            .expect("Failed to run jonesy CLI");

        let cli_stdout = String::from_utf8_lossy(&cli_output.stdout);

        // Parse CLI output for panic points (top-level lines starting with " --> ")
        // Skip nested points (lines in the call tree that are indented)
        let cli_points: HashSet<(String, u32)> = cli_stdout
            .lines()
            .filter(|line| line.starts_with(" --> ")) // Only top-level points
            .filter_map(|line| {
                // Parse " --> path/to/file.rs:123:45"
                let arrow_pos = line.find(" --> ")?;
                let location = &line[arrow_pos + 5..];
                let parts: Vec<&str> = location.split(':').collect();
                if parts.len() >= 2 {
                    let file = parts[0].trim();
                    let line_num: u32 = parts[1].parse().ok()?;
                    // Normalize path - extract just the relative part
                    let file = file
                        .rsplit("workspace_test/")
                        .next()
                        .unwrap_or(file)
                        .to_string();
                    Some((file, line_num))
                } else {
                    None
                }
            })
            .collect();

        // Run LSP-style analysis using the same functions
        let targets =
            find_workspace_binaries(&workspace_test_dir).expect("Should find workspace binaries");

        let project_context = ProjectContext::from_project_root(&workspace_test_dir)
            .expect("Should build project context");

        let mut lsp_points: HashSet<(String, u32)> = HashSet::new();

        for target in &targets {
            let result = analyze_single_target(target, &project_context);
            if let Ok(points) = result {
                for point in points {
                    // Normalize path
                    let file = point
                        .file
                        .rsplit("workspace_test/")
                        .next()
                        .unwrap_or(&point.file)
                        .to_string();
                    lsp_points.insert((file, point.line));
                }
            }
        }

        // Compare: LSP should find at least as many points as CLI
        let missing_in_lsp: Vec<_> = cli_points.difference(&lsp_points).collect();
        let extra_in_lsp: Vec<_> = lsp_points.difference(&cli_points).collect();

        if !missing_in_lsp.is_empty() {
            eprintln!("CLI found but LSP missed:");
            for (file, line) in &missing_in_lsp {
                eprintln!("  {}:{}", file, line);
            }
        }

        if !extra_in_lsp.is_empty() {
            eprintln!("LSP found but CLI missed:");
            for (file, line) in &extra_in_lsp {
                eprintln!("  {}:{}", file, line);
            }
        }

        // The LSP should find all points the CLI finds
        assert!(
            missing_in_lsp.is_empty(),
            "LSP analysis should find all panic points that CLI finds. \
             Missing {} points, extra {} points. CLI found {}, LSP found {}",
            missing_in_lsp.len(),
            extra_in_lsp.len(),
            cli_points.len(),
            lsp_points.len()
        );
    }

    #[test]
    fn test_create_inline_allow_action() {
        let uri = Url::parse("file:///tmp/test.rs").unwrap();
        let range = Range {
            start: Position {
                line: 10,
                character: 5,
            },
            end: Position {
                line: 10,
                character: 15,
            },
        };
        let diagnostic = Diagnostic {
            range,
            severity: Some(DiagnosticSeverity::WARNING),
            source: Some("jonesy".to_string()),
            message: "test".to_string(),
            ..Default::default()
        };

        // Test specific cause
        let action =
            JonesyLspServer::create_inline_allow_action(&uri, range, "unwrap", &diagnostic)
                .unwrap();
        assert_eq!(action.title, "Allow 'unwrap' on this line");
        assert_eq!(action.kind, Some(CodeActionKind::QUICKFIX));
        assert!(action.is_preferred.unwrap_or(false)); // Specific cause is preferred

        // Verify the edit inserts the comment
        let edit = action.edit.unwrap();
        let changes = edit.changes.unwrap();
        let edits = changes.get(&uri).unwrap();
        assert_eq!(edits.len(), 1);
        assert_eq!(edits[0].new_text, " // jonesy:allow(unwrap)");

        // Test wildcard
        let action =
            JonesyLspServer::create_inline_allow_action(&uri, range, "*", &diagnostic).unwrap();
        assert_eq!(action.title, "Allow all panics on this line");
        assert!(!action.is_preferred.unwrap_or(true)); // Wildcard is not preferred
    }

    #[test]
    fn test_create_inline_allow_action_merges_causes() {
        use std::io::Write;

        // Create a temp file with an existing inline allow
        let dir = tempfile::tempdir().unwrap();
        let file_path = dir.path().join("test.rs");
        let mut f = std::fs::File::create(&file_path).unwrap();
        writeln!(f, "fn main() {{").unwrap();
        writeln!(f, "    let x = foo(); // jonesy:allow(bounds)").unwrap();
        writeln!(f, "}}").unwrap();

        let uri = Url::from_file_path(&file_path).unwrap();
        let range = Range {
            start: Position {
                line: 1, // the line with the existing allow
                character: 4,
            },
            end: Position {
                line: 1,
                character: 10,
            },
        };
        let diagnostic = Diagnostic {
            range,
            severity: Some(DiagnosticSeverity::WARNING),
            source: Some("jonesy".to_string()),
            message: "test".to_string(),
            ..Default::default()
        };

        // Adding "overflow" should merge with existing "bounds"
        let action =
            JonesyLspServer::create_inline_allow_action(&uri, range, "overflow", &diagnostic)
                .unwrap();
        let edit = action.edit.unwrap();
        let changes = edit.changes.unwrap();
        let edits = changes.get(&uri).unwrap();
        assert_eq!(edits.len(), 1);
        assert_eq!(edits[0].new_text, "// jonesy:allow(bounds, overflow)");

        // Adding "bounds" again should not duplicate
        let action =
            JonesyLspServer::create_inline_allow_action(&uri, range, "bounds", &diagnostic)
                .unwrap();
        let edit = action.edit.unwrap();
        let changes = edit.changes.unwrap();
        let edits = changes.get(&uri).unwrap();
        assert_eq!(edits[0].new_text, "// jonesy:allow(bounds)");
    }

    #[test]
    fn test_create_file_allow_action() {
        let uri = Url::parse("file:///workspace/src/main.rs").unwrap();
        let range = Range::default();
        let diagnostic = Diagnostic {
            range,
            severity: Some(DiagnosticSeverity::WARNING),
            source: Some("jonesy".to_string()),
            message: "test".to_string(),
            ..Default::default()
        };
        let workspace_root = PathBuf::from("/workspace");

        let action =
            JonesyLspServer::create_file_allow_action(&uri, "unwrap", &workspace_root, &diagnostic)
                .unwrap();

        assert_eq!(action.title, "Allow 'unwrap' in main.rs");
        assert_eq!(action.kind, Some(CodeActionKind::QUICKFIX));

        // Verify the edit targets jonesy.toml
        let edit = action.edit.unwrap();
        let doc_changes = edit.document_changes.unwrap();
        match doc_changes {
            DocumentChanges::Operations(ops) => {
                // Should have at least a text edit (possibly a create file too)
                assert!(!ops.is_empty());
            }
            _ => panic!("Expected Operations"),
        }
    }

    #[test]
    fn test_create_function_allow_action() {
        let range = Range::default();
        let diagnostic = Diagnostic {
            range,
            severity: Some(DiagnosticSeverity::WARNING),
            source: Some("jonesy".to_string()),
            message: "test".to_string(),
            ..Default::default()
        };
        let workspace_root = PathBuf::from("/workspace");

        let action = JonesyLspServer::create_function_allow_action(
            "my_crate::module::parse_config",
            "unwrap",
            &workspace_root,
            &diagnostic,
        )
        .unwrap();

        // Uses full function path for precise matching
        assert_eq!(action.title, "Allow 'unwrap' in this function");
        assert_eq!(action.kind, Some(CodeActionKind::QUICKFIX));
    }

    #[test]
    fn test_create_called_function_allow_action() {
        let range = Range::default();
        let diagnostic = Diagnostic {
            range,
            severity: Some(DiagnosticSeverity::WARNING),
            source: Some("jonesy".to_string()),
            message: "test".to_string(),
            ..Default::default()
        };
        let workspace_root = PathBuf::from("/workspace");

        let action = JonesyLspServer::create_called_function_allow_action(
            "my_crate::config::Config::parse",
            "expect",
            &workspace_root,
            &diagnostic,
        )
        .unwrap();

        assert_eq!(
            action.title,
            "Allow 'expect' on calls to 'my_crate::config::Config::parse()'"
        );
        assert_eq!(action.kind, Some(CodeActionKind::QUICKFIX));

        // Verify the generated config rule uses the specific cause, not wildcard
        let edit = action.edit.unwrap();
        let changes = edit.document_changes.unwrap();
        if let DocumentChanges::Operations(ops) = changes {
            let has_correct_rule = ops.iter().any(|op| {
                if let DocumentChangeOperation::Edit(text_edit) = op {
                    text_edit.edits.iter().any(|e| {
                        if let OneOf::Left(te) = e {
                            te.new_text
                                .contains("function = \"my_crate::config::Config::parse\"")
                                && te.new_text.contains("allow = [\"expect\"]")
                        } else {
                            false
                        }
                    })
                } else {
                    false
                }
            });
            assert!(
                has_correct_rule,
                "Rule should use specific cause 'expect', not wildcard"
            );
        } else {
            panic!("Expected Operations variant");
        }
    }

    #[test]
    fn test_create_module_allow_action_tests() {
        let range = Range::default();
        let diagnostic = Diagnostic {
            range,
            severity: Some(DiagnosticSeverity::WARNING),
            source: Some("jonesy".to_string()),
            message: "test".to_string(),
            ..Default::default()
        };
        let workspace_root = PathBuf::from("/workspace");
        let uri = Url::parse("file:///workspace/tests/integration.rs").unwrap();

        let action = JonesyLspServer::create_module_allow_action(
            &uri,
            "unwrap",
            &workspace_root,
            &diagnostic,
        )
        .unwrap();

        assert_eq!(action.title, "Allow 'unwrap' in tests");
        assert_eq!(action.kind, Some(CodeActionKind::QUICKFIX));
    }

    #[test]
    fn test_create_module_allow_action_benches() {
        let range = Range::default();
        let diagnostic = Diagnostic {
            range,
            severity: Some(DiagnosticSeverity::WARNING),
            source: Some("jonesy".to_string()),
            message: "test".to_string(),
            ..Default::default()
        };
        let workspace_root = PathBuf::from("/workspace");
        let uri = Url::parse("file:///workspace/benches/bench.rs").unwrap();

        let action = JonesyLspServer::create_module_allow_action(
            &uri,
            "panic",
            &workspace_root,
            &diagnostic,
        )
        .unwrap();

        assert_eq!(action.title, "Allow 'panic' in benches");
        assert_eq!(action.kind, Some(CodeActionKind::QUICKFIX));
    }

    #[test]
    fn test_create_module_allow_action_none_for_src() {
        let range = Range::default();
        let diagnostic = Diagnostic {
            range,
            severity: Some(DiagnosticSeverity::WARNING),
            source: Some("jonesy".to_string()),
            message: "test".to_string(),
            ..Default::default()
        };
        let workspace_root = PathBuf::from("/workspace");
        let uri = Url::parse("file:///workspace/src/main.rs").unwrap();

        // Should return None for regular src files
        let action = JonesyLspServer::create_module_allow_action(
            &uri,
            "unwrap",
            &workspace_root,
            &diagnostic,
        );

        assert!(action.is_none());
    }

    #[test]
    fn test_create_crate_allow_action() {
        let range = Range::default();
        let diagnostic = Diagnostic {
            range,
            severity: Some(DiagnosticSeverity::WARNING),
            source: Some("jonesy".to_string()),
            message: "test".to_string(),
            ..Default::default()
        };
        let workspace_root = PathBuf::from("/workspace");

        let action =
            JonesyLspServer::create_crate_allow_action("unwrap", &workspace_root, &diagnostic)
                .unwrap();

        assert_eq!(action.title, "Allow 'unwrap' in this crate");
        assert_eq!(action.kind, Some(CodeActionKind::QUICKFIX));
    }

    #[test]
    fn test_find_config_files() {
        use std::collections::HashSet;

        // Use workspace_test example which has multiple workspace members
        let workspace_root = find_workspace_root();
        let workspace_test_dir = workspace_root.join("examples").join("workspace_test");

        let config_files = find_config_files(&workspace_test_dir);

        // Collect unique full paths to verify no duplicates
        let unique_paths: HashSet<String> = config_files
            .iter()
            .map(|p| p.to_string_lossy().to_string())
            .collect();

        // Verify no duplicates (set size should equal vec size)
        assert_eq!(
            config_files.len(),
            unique_paths.len(),
            "Config files should not contain duplicates. Found {} paths but {} unique.",
            config_files.len(),
            unique_paths.len()
        );

        // Should always include jonesy.toml (even if it doesn't exist)
        assert!(
            config_files.iter().any(|p| p.ends_with("jonesy.toml")),
            "Should include jonesy.toml path. Found: {:?}",
            config_files
        );

        // Should include workspace Cargo.toml
        assert!(
            config_files.iter().any(|p| {
                p.parent()
                    .map(|parent| parent.ends_with("workspace_test"))
                    .unwrap_or(false)
                    && p.ends_with("Cargo.toml")
            }),
            "Should include workspace Cargo.toml. Found: {:?}",
            config_files
        );

        // Should include each member's Cargo.toml
        for member in ["crate_a", "crate_b", "crate_c"] {
            assert!(
                config_files.iter().any(|p| {
                    p.parent()
                        .map(|parent| parent.ends_with(member))
                        .unwrap_or(false)
                        && p.ends_with("Cargo.toml")
                }),
                "Should include {}/Cargo.toml. Found: {:?}",
                member,
                config_files
            );
        }

        // Total should be exactly 5: jonesy.toml + workspace Cargo.toml + 3 members
        assert_eq!(
            config_files.len(),
            5,
            "Should find exactly 5 config files. Found: {:?}",
            config_files
        );
    }

    // ========================================================================
    // Unit tests for pure helper functions
    // ========================================================================

    fn make_code_point(file: &str, line: u32, column: Option<u32>) -> CrateCodePoint {
        CrateCodePoint {
            file: file.to_string(),
            line,
            column,
            name: "test_func".to_string(),
            causes: HashSet::new(),
            children: Vec::new(),
            is_direct_panic: false,
            called_function: None,
        }
    }

    #[test]
    fn test_deduplicate_code_points_empty() {
        let points: Vec<CrateCodePoint> = vec![];
        let result = deduplicate_code_points(points);
        assert!(result.is_empty());
    }

    #[test]
    fn test_deduplicate_code_points_no_duplicates() {
        let points = vec![
            make_code_point("src/main.rs", 10, Some(5)),
            make_code_point("src/main.rs", 20, Some(10)),
            make_code_point("src/lib.rs", 10, Some(5)),
        ];
        let result = deduplicate_code_points(points);
        assert_eq!(result.len(), 3);
    }

    #[test]
    fn test_deduplicate_code_points_with_duplicates() {
        let points = vec![
            make_code_point("src/main.rs", 10, Some(5)),
            make_code_point("src/main.rs", 10, Some(5)), // duplicate
            make_code_point("src/main.rs", 20, Some(10)),
            make_code_point("src/main.rs", 10, Some(5)), // duplicate
        ];
        let result = deduplicate_code_points(points);
        assert_eq!(result.len(), 2);
        assert_eq!(result[0].line, 10);
        assert_eq!(result[1].line, 20);
    }

    #[test]
    fn test_deduplicate_code_points_column_matters() {
        // Same file and line, different columns - NOT duplicates
        let points = vec![
            make_code_point("src/main.rs", 10, Some(5)),
            make_code_point("src/main.rs", 10, Some(15)),
        ];
        let result = deduplicate_code_points(points);
        assert_eq!(result.len(), 2);
    }

    #[test]
    fn test_deduplicate_code_points_none_column() {
        // None columns are treated as 0 for deduplication
        let points = vec![
            make_code_point("src/main.rs", 10, None),
            make_code_point("src/main.rs", 10, None), // duplicate
            make_code_point("src/main.rs", 10, Some(0)), // also duplicate (None -> 0)
        ];
        let result = deduplicate_code_points(points);
        assert_eq!(result.len(), 1);
    }

    #[test]
    fn test_group_points_by_uri_empty() {
        let points: Vec<CrateCodePoint> = vec![];
        let workspace_root = PathBuf::from("/workspace");
        let result = group_points_by_uri(points, &workspace_root, "/workspace/target");
        assert!(result.is_empty());
    }

    #[test]
    fn test_group_points_by_uri_filters_target_dir() {
        let points = vec![
            make_code_point("/workspace/src/main.rs", 10, Some(5)),
            make_code_point("/workspace/target/debug/build/foo.rs", 20, Some(10)), // filtered
            make_code_point("/workspace/src/lib.rs", 30, Some(15)),
        ];
        let workspace_root = PathBuf::from("/workspace");
        let result = group_points_by_uri(points, &workspace_root, "/workspace/target");

        assert_eq!(result.len(), 2);
        assert!(result.keys().all(|uri| !uri.path().contains("/target/")));
    }

    #[test]
    fn test_group_points_by_uri_groups_by_file() {
        let points = vec![
            make_code_point("/workspace/src/main.rs", 10, Some(5)),
            make_code_point("/workspace/src/main.rs", 20, Some(10)),
            make_code_point("/workspace/src/lib.rs", 30, Some(15)),
        ];
        let workspace_root = PathBuf::from("/workspace");
        let result = group_points_by_uri(points, &workspace_root, "/workspace/target");

        assert_eq!(result.len(), 2);

        let main_uri = Url::from_file_path("/workspace/src/main.rs").unwrap();
        let lib_uri = Url::from_file_path("/workspace/src/lib.rs").unwrap();

        assert_eq!(result.get(&main_uri).unwrap().len(), 2);
        assert_eq!(result.get(&lib_uri).unwrap().len(), 1);
    }

    #[test]
    fn test_group_points_by_uri_relative_paths() {
        // Relative paths should be joined with workspace_root
        let points = vec![
            make_code_point("src/main.rs", 10, Some(5)),
            make_code_point("src/lib.rs", 20, Some(10)),
        ];
        let workspace_root = PathBuf::from("/workspace");
        let result = group_points_by_uri(points, &workspace_root, "/workspace/target");

        assert_eq!(result.len(), 2);

        let main_uri = Url::from_file_path("/workspace/src/main.rs").unwrap();
        assert!(result.contains_key(&main_uri));
    }

    #[test]
    fn test_analyze_workspace_targets_empty() {
        let targets: Vec<PathBuf> = vec![];
        let project_context = ProjectContext::default();
        let result = analyze_workspace_targets(&targets, &project_context);

        assert!(result.points.is_empty());
        assert_eq!(result.total_count, 0);
        assert_eq!(result.analyzed_count, 0);
        assert_eq!(result.skipped_count, 0);
    }

    #[test]
    fn test_analyze_workspace_targets_nonexistent() {
        // Non-existent targets should be skipped
        let targets = vec![PathBuf::from("/nonexistent/binary")];
        let project_context = ProjectContext::default();
        let result = analyze_workspace_targets(&targets, &project_context);

        assert!(result.points.is_empty());
        assert_eq!(result.analyzed_count, 0);
        assert_eq!(result.skipped_count, 1);
    }

    #[test]
    fn test_analyze_workspace_targets_real_binary() {
        // Test with a real binary from the workspace
        let workspace_root = find_workspace_root();
        let panic_example = workspace_root.join("examples/panic");

        // Build the example first
        let status = std::process::Command::new("cargo")
            .arg("build")
            .current_dir(&panic_example)
            .status();

        if status.is_err() || !status.unwrap().success() {
            // Skip test if build fails
            return;
        }

        let binary = panic_example.join("target/debug/panic");
        if !binary.exists() {
            return;
        }

        let targets = vec![binary];
        let project_context = ProjectContext::from_project_root(&panic_example)
            .expect("Should build project context for panic example");
        let result = analyze_workspace_targets(&targets, &project_context);

        assert_eq!(result.analyzed_count, 1);
        assert_eq!(result.skipped_count, 0);
        // The panic example should have many panic points
        assert!(
            result.total_count > 0,
            "Should find panic points in panic example"
        );
    }

    #[test]
    fn test_analysis_result_default() {
        let result = AnalysisResult::default();
        assert!(result.points.is_empty());
        assert_eq!(result.total_count, 0);
        assert_eq!(result.analyzed_count, 0);
        assert_eq!(result.skipped_count, 0);
    }

    // ========================================================================
    // Tests for code_point_to_diagnostic
    // ========================================================================

    #[test]
    fn test_code_point_to_diagnostic_empty_causes() {
        let point = make_code_point("src/main.rs", 42, Some(10));
        let diag = JonesyLspServer::code_point_to_diagnostic(&point);

        assert_eq!(diag.severity, Some(DiagnosticSeverity::WARNING));
        assert_eq!(diag.source, Some("jonesy".to_string()));
        assert_eq!(diag.message, "potential panic point");
        assert!(diag.code.is_none());
        assert!(diag.code_description.is_none());
        // LSP uses 0-based lines
        assert_eq!(diag.range.start.line, 41);
        assert_eq!(diag.range.start.character, 9);
    }

    #[test]
    fn test_code_point_to_diagnostic_single_cause() {
        use crate::panic_cause::PanicCause;

        let mut causes = HashSet::new();
        causes.insert(PanicCause::Unwrap);

        let point = CrateCodePoint {
            file: "src/main.rs".to_string(),
            line: 10,
            column: Some(5),
            name: "my_func".to_string(),
            causes,
            children: Vec::new(),
            is_direct_panic: true,
            called_function: None,
        };

        let diag = JonesyLspServer::code_point_to_diagnostic(&point);

        assert!(diag.message.contains("unwrap"));
        // Single cause should also show JP code in message (consistent with multi-cause)
        assert!(
            diag.message.contains("JP006"),
            "Single-cause message should include error code. Got: {}",
            diag.message
        );
        assert!(diag.code.is_some());
        assert!(diag.code_description.is_some());
        // Should have help message for direct panic
        assert!(diag.message.contains("help:"));
    }

    #[test]
    fn test_code_point_to_diagnostic_multiple_causes() {
        use crate::panic_cause::PanicCause;

        let mut causes = HashSet::new();
        causes.insert(PanicCause::Unwrap);
        causes.insert(PanicCause::Expect);

        let point = CrateCodePoint {
            file: "src/main.rs".to_string(),
            line: 10,
            column: Some(5),
            name: "my_func".to_string(),
            causes,
            children: Vec::new(),
            is_direct_panic: false,
            called_function: Some("risky_fn".to_string()),
        };

        let diag = JonesyLspServer::code_point_to_diagnostic(&point);

        // Multiple causes shown in message
        assert!(diag.message.contains("JP"));
        // Data should contain cause info
        assert!(diag.data.is_some());
        let data = diag.data.unwrap();
        let causes_arr: Vec<String> = serde_json::from_value(data["causes"].clone()).unwrap();
        assert_eq!(causes_arr.len(), 2);
    }

    #[test]
    fn test_code_point_to_diagnostic_no_column() {
        let point = make_code_point("src/main.rs", 100, None);
        let diag = JonesyLspServer::code_point_to_diagnostic(&point);

        // No column defaults to 1, then subtract 1 for 0-based = 0
        assert_eq!(diag.range.start.character, 0);
    }

    #[test]
    fn test_code_point_to_diagnostic_line_1() {
        // Edge case: line 1 should not underflow
        let point = make_code_point("src/main.rs", 1, Some(1));
        let diag = JonesyLspServer::code_point_to_diagnostic(&point);

        assert_eq!(diag.range.start.line, 0);
        assert_eq!(diag.range.start.character, 0);
    }

    // ========================================================================
    // Tests for get_module_pattern
    // ========================================================================

    #[test]
    fn test_get_module_pattern_tests_dir() {
        let uri = Url::parse("file:///workspace/tests/unit_tests.rs").unwrap();
        let result = JonesyLspServer::get_module_pattern(&uri);
        assert_eq!(result, Some(("**/tests/**", "tests")));
    }

    #[test]
    fn test_get_module_pattern_benches_dir() {
        let uri = Url::parse("file:///workspace/benches/perf.rs").unwrap();
        let result = JonesyLspServer::get_module_pattern(&uri);
        assert_eq!(result, Some(("**/benches/**", "benches")));
    }

    #[test]
    fn test_get_module_pattern_examples_dir() {
        let uri = Url::parse("file:///workspace/examples/demo.rs").unwrap();
        let result = JonesyLspServer::get_module_pattern(&uri);
        assert_eq!(result, Some(("**/examples/**", "examples")));
    }

    #[test]
    fn test_get_module_pattern_test_suffix() {
        let uri = Url::parse("file:///workspace/src/parser_test.rs").unwrap();
        let result = JonesyLspServer::get_module_pattern(&uri);
        assert_eq!(result, Some(("**/*_test.rs", "test files")));
    }

    #[test]
    fn test_get_module_pattern_tests_suffix() {
        let uri = Url::parse("file:///workspace/src/parser_tests.rs").unwrap();
        let result = JonesyLspServer::get_module_pattern(&uri);
        assert_eq!(result, Some(("**/*_tests.rs", "test files")));
    }

    #[test]
    fn test_get_module_pattern_regular_src() {
        let uri = Url::parse("file:///workspace/src/parser.rs").unwrap();
        let result = JonesyLspServer::get_module_pattern(&uri);
        assert!(result.is_none());
    }

    #[test]
    fn test_get_module_pattern_main_rs() {
        let uri = Url::parse("file:///workspace/src/main.rs").unwrap();
        let result = JonesyLspServer::get_module_pattern(&uri);
        assert!(result.is_none());
    }

    // ========================================================================
    // Tests for expand_workspace_glob
    // ========================================================================

    #[test]
    fn test_expand_workspace_glob_nonexistent_dir() {
        let workspace_root = PathBuf::from("/nonexistent/path");
        let result = expand_workspace_glob(&workspace_root, "crates/*");
        assert!(result.is_empty());
    }

    #[test]
    fn test_expand_workspace_glob_real_workspace() {
        let workspace_root = find_workspace_root();
        let result = expand_workspace_glob(&workspace_root, "examples/*");

        // Should find example crates
        assert!(!result.is_empty(), "Should find example directories");

        // All results should have Cargo.toml
        for path in &result {
            assert!(
                path.join("Cargo.toml").exists(),
                "{} should have Cargo.toml",
                path.display()
            );
        }
    }

    // ========================================================================
    // Tests for discover_workspace
    // ========================================================================

    #[test]
    fn test_discover_workspace_real() {
        let workspace_root = find_workspace_root();
        let info = discover_workspace(&workspace_root);

        assert!(info.is_some());
        let info = info.unwrap();

        // Should have workspace members
        assert!(!info.members.is_empty());
    }

    #[test]
    fn test_discover_workspace_single_crate() {
        let workspace_root = find_workspace_root();
        let panic_example = workspace_root.join("examples/panic");

        let info = discover_workspace(&panic_example);
        assert!(info.is_some());
        let info = info.unwrap();

        // Single crate should have the package name as a member
        assert!(!info.members.is_empty());
    }

    #[test]
    fn test_discover_workspace_nonexistent() {
        let workspace_root = PathBuf::from("/nonexistent/path");
        let info = discover_workspace(&workspace_root);
        assert!(info.is_none());
    }

    // ========================================================================
    // Tests for find_workspace_binaries edge cases
    // ========================================================================

    #[test]
    fn test_find_workspace_binaries_no_cargo_toml() {
        let temp_dir = std::env::temp_dir().join("jonesy_test_no_cargo");
        let _ = std::fs::create_dir_all(&temp_dir);

        let result = find_workspace_binaries(&temp_dir);
        assert!(result.is_ok());
        assert!(result.unwrap().is_empty());

        let _ = std::fs::remove_dir_all(&temp_dir);
    }

    #[test]
    fn test_find_workspace_binaries_no_target() {
        let temp_dir = std::env::temp_dir().join("jonesy_test_no_target");
        let _ = std::fs::create_dir_all(&temp_dir);

        // Create minimal Cargo.toml
        std::fs::write(
            temp_dir.join("Cargo.toml"),
            r#"[package]
name = "test"
version = "0.1.0"
"#,
        )
        .unwrap();

        let result = find_workspace_binaries(&temp_dir);
        assert!(result.is_ok());
        assert!(result.unwrap().is_empty());

        let _ = std::fs::remove_dir_all(&temp_dir);
    }

    // ========================================================================
    // Tests for find_config_files edge cases
    // ========================================================================

    #[test]
    fn test_find_config_files_no_cargo_toml() {
        let temp_dir = std::env::temp_dir().join("jonesy_test_config_no_cargo");
        let _ = std::fs::create_dir_all(&temp_dir);

        let files = find_config_files(&temp_dir);

        // Should always include jonesy.toml path
        assert!(files.iter().any(|p| p.ends_with("jonesy.toml")));
        // But not much else without Cargo.toml
        assert_eq!(files.len(), 1);

        let _ = std::fs::remove_dir_all(&temp_dir);
    }

    #[test]
    fn test_find_config_files_single_crate() {
        let temp_dir = std::env::temp_dir().join("jonesy_test_config_single");
        let _ = std::fs::create_dir_all(&temp_dir);

        // Create minimal Cargo.toml (not a workspace)
        std::fs::write(
            temp_dir.join("Cargo.toml"),
            r#"[package]
name = "test"
version = "0.1.0"
"#,
        )
        .unwrap();

        let files = find_config_files(&temp_dir);

        // Should include jonesy.toml and Cargo.toml
        assert!(files.iter().any(|p| p.ends_with("jonesy.toml")));
        assert!(files.iter().any(|p| p.ends_with("Cargo.toml")));
        assert_eq!(files.len(), 2);

        let _ = std::fs::remove_dir_all(&temp_dir);
    }

    #[test]
    fn test_format_change_summary_single_package() {
        use crate::analysis_cache::{AnalysisCache, build_workspace_state};

        // Single package: use a simple crate (e.g., one of our examples)
        let workspace_root = find_workspace_root();
        let example_dir = workspace_root.join("examples").join("rlib");
        let state = build_workspace_state(&example_dir);
        assert!(state.is_single_package());

        // Simulate a library change by detecting changes vs an empty cache
        let cache = AnalysisCache::default();
        let changes = cache.detect_workspace_changes(&state);

        let summary = format_change_summary(&changes, &state);
        assert!(
            summary.starts_with("Package changes:"),
            "Single package should use 'Package changes' format. Got: {}",
            summary
        );
        assert!(
            summary.contains("library"),
            "Should mention library. Got: {}",
            summary
        );
    }

    #[test]
    fn test_format_change_summary_workspace() {
        use crate::analysis_cache::{AnalysisCache, build_workspace_state};

        // Workspace: use the jonesy project root (which has workspace members)
        let workspace_root = find_workspace_root();
        let state = build_workspace_state(&workspace_root);
        assert!(!state.is_single_package());

        // Simulate changes by detecting vs an empty cache
        let cache = AnalysisCache::default();
        let changes = cache.detect_workspace_changes(&state);

        let summary = format_change_summary(&changes, &state);
        assert!(
            summary.starts_with("Workspace changes:"),
            "Workspace should use 'Workspace changes' format. Got: {}",
            summary
        );
        assert!(
            summary.contains("members"),
            "Should mention members. Got: {}",
            summary
        );
    }
}