diff --git a/.alfonso/release-notes/next/lsp-startup-reservations.md b/.alfonso/release-notes/next/lsp-startup-reservations.md new file mode 100644 index 0000000000..0651f9b04e --- /dev/null +++ b/.alfonso/release-notes/next/lsp-startup-reservations.md @@ -0,0 +1 @@ +- Language servers start once per workspace even when several requests arrive together, and file changes made while a server is starting are sent to it once it is ready. diff --git a/benchmarks/aft-search/slice-descriptors/land-lsp-startup-reservations.json b/benchmarks/aft-search/slice-descriptors/land-lsp-startup-reservations.json new file mode 100644 index 0000000000..ebd574711e --- /dev/null +++ b/benchmarks/aft-search/slice-descriptors/land-lsp-startup-reservations.json @@ -0,0 +1,8 @@ +{ + "targeted_mechanism": "none", + "kind": "harness", + "slice_class": "non_ranking", + "fixtures": [ + "harness-goldens" + ] +} diff --git a/crates/aft/src/commands/ast_replace.rs b/crates/aft/src/commands/ast_replace.rs index f3b9d8db2a..dfdba38e43 100644 --- a/crates/aft/src/commands/ast_replace.rs +++ b/crates/aft/src/commands/ast_replace.rs @@ -610,11 +610,11 @@ mod lsp_tests { manager.set_extra_env("AFT_FAKE_LSP_DISABLE_PUSH", "1"); manager.set_extra_env("AFT_FAKE_LSP_ECHO_CHANGES", "1"); assert_eq!( - manager - .ensure_file_open(&file, &ctx.config()) - .unwrap() - .server_keys - .len(), + crate::lsp::manager::settled_lsp(&mut manager, |startup_manager| startup_manager + .ensure_file_open(&file, &ctx.config())) + .unwrap() + .server_keys + .len(), 1 ); } diff --git a/crates/aft/src/commands/lsp_diagnostics.rs b/crates/aft/src/commands/lsp_diagnostics.rs index 64c921ec3a..fc766cc69c 100644 --- a/crates/aft/src/commands/lsp_diagnostics.rs +++ b/crates/aft/src/commands/lsp_diagnostics.rs @@ -157,18 +157,9 @@ fn handle_file_mode( } if outcomes.successful.is_empty() { - // Servers matched but none could start. Return an empty result with + // Servers matched but none are ready. Return an empty result with // detailed per-server status so the user can see why. - return Response::success( - &req.id, - serde_json::json!({ - "diagnostics": [], - "total": 0, - "files_with_errors": 0, - "complete": false, - "lsp_servers_used": server_status, - }), - ); + return Response::success(&req.id, startup_unavailable_response(&canonical, &outcomes)); } // Step 2: snapshot before syncing/opening so push fallback freshness is @@ -277,6 +268,10 @@ fn handle_file_mode( let mut complete = true; for entry in &server_status { match entry.status.as_str() { + crate::lsp::manager::START_IN_PROGRESS_STATUS => { + complete = false; + pending_servers.push(entry.server_id.clone()); + } "pull_ok" | "pull_unchanged" => {} "push_only" | "pull_rejected_push_fallback" | "pull_no_cache_for_unchanged" => { let fresh = outcomes.successful.iter().any(|key| { @@ -303,11 +298,19 @@ fn handle_file_mode( // diagnostics stay stored. ctx.lsp() .close_documents_opened_for_pulls(&canonical, &pull_results); + let unchecked_files = if server_status + .iter() + .any(|entry| entry.status == crate::lsp::manager::START_IN_PROGRESS_STATUS) + { + vec![canonical.display().to_string()] + } else { + Vec::new() + }; let mut response = build_response( &diagnostics, server_status, complete, - Vec::new(), + unchecked_files, None, pending_servers, ); @@ -332,6 +335,7 @@ fn handle_directory_mode( }; let mut server_status: Vec = Vec::new(); + crate::lsp::manager::wait_for_pending_startups_unlocked(|| ctx.lsp(), Some(&canonical)); let mut all_complete = true; // Pull workspace diagnostics from active servers that support it. We @@ -389,7 +393,27 @@ fn handle_directory_mode( // Compute unchecked_files: walk the directory (capped) and list any // file that has no entry in the diagnostic cache. We cap at // DIRECTORY_FILE_CAP to avoid pathological large-directory walks. - let (unchecked_files, walk_truncated) = compute_unchecked_files(ctx, &canonical); + let (mut unchecked_files, walk_truncated) = compute_unchecked_files(ctx, &canonical); + let pending_starts: Vec<_> = ctx + .lsp() + .pending_startup_files() + .into_iter() + .filter(|(key, _)| key.root.starts_with(&canonical) || canonical.starts_with(&key.root)) + .collect(); + let mut pending_servers = Vec::new(); + for (key, file) in pending_starts { + all_complete = false; + pending_servers.push(key.kind.id_str().to_string()); + server_status.push(ServerStatusEntry { + server_id: key.kind.id_str().to_string(), + scope: ServerScope::Workspace, + status: crate::lsp::manager::START_IN_PROGRESS_STATUS.into(), + }); + let file = file.display().to_string(); + if Path::new(&file).starts_with(&canonical) && !unchecked_files.contains(&file) { + unchecked_files.push(file); + } + } if walk_truncated || !unchecked_files.is_empty() { all_complete = false; } @@ -400,7 +424,7 @@ fn handle_directory_mode( all_complete, unchecked_files, Some(walk_truncated), - Vec::new(), + pending_servers, ); Response::success(&req.id, response) } @@ -420,7 +444,31 @@ fn handle_global_mode( .cloned() .collect() }; - let response = build_response(&diagnostics, Vec::new(), true, Vec::new(), None, Vec::new()); + let pending_starts = ctx.lsp().pending_startup_files(); + let statuses = pending_starts + .iter() + .map(|(key, _)| ServerStatusEntry { + server_id: key.kind.id_str().to_string(), + scope: ServerScope::Workspace, + status: crate::lsp::manager::START_IN_PROGRESS_STATUS.into(), + }) + .collect(); + let pending = pending_starts + .iter() + .map(|(key, _)| key.kind.id_str().to_string()) + .collect(); + let unchecked = pending_starts + .iter() + .map(|(_, file)| file.display().to_string()) + .collect(); + let response = build_response( + &diagnostics, + statuses, + pending_starts.is_empty(), + unchecked, + None, + pending, + ); Response::success(&req.id, response) } @@ -815,12 +863,37 @@ struct ServerStatusEntry { status: String, } +pub(crate) fn startup_unavailable_response( + file: &Path, + outcomes: &EnsureServerOutcomes, +) -> serde_json::Value { + let statuses: Vec<_> = outcomes + .attempts + .iter() + .map(|attempt| ServerStatusEntry::from_attempt(attempt, ServerScope::File)) + .collect(); + let pending: Vec<_> = statuses + .iter() + .filter(|status| status.status == crate::lsp::manager::START_IN_PROGRESS_STATUS) + .map(|status| status.server_id.clone()) + .collect(); + let unchecked = if pending.is_empty() { + Vec::new() + } else { + vec![file.display().to_string()] + }; + build_response(&[], statuses, false, unchecked, None, pending) +} + impl ServerStatusEntry { fn from_attempt(attempt: &crate::lsp::manager::ServerAttempt, scope: ServerScope) -> Self { let status = match &attempt.result { ServerAttemptResult::ProjectClosed => { "project closed; reopen the project and retry".into() } + ServerAttemptResult::InProgress { .. } => { + crate::lsp::manager::START_IN_PROGRESS_STATUS.into() + } ServerAttemptResult::Ok { .. } => "ok".to_string(), ServerAttemptResult::NoRootMarker { looked_for } => { format!("no_root_marker (looked for: {})", looked_for.join(", ")) diff --git a/crates/aft/src/commands/lsp_inspect.rs b/crates/aft/src/commands/lsp_inspect.rs index cdfdf32cb9..bd5960fb4a 100644 --- a/crates/aft/src/commands/lsp_inspect.rs +++ b/crates/aft/src/commands/lsp_inspect.rs @@ -214,6 +214,9 @@ fn classify_binary_source( fn spawn_status(result: &ServerAttemptResult) -> String { match result { + ServerAttemptResult::InProgress { .. } => { + crate::lsp::manager::START_IN_PROGRESS_STATUS.into() + } ServerAttemptResult::ProjectClosed => "project closed; reopen the project and retry".into(), ServerAttemptResult::Ok { .. } => "ok".to_string(), ServerAttemptResult::NoRootMarker { .. } => "no_root_marker".to_string(), @@ -222,6 +225,25 @@ fn spawn_status(result: &ServerAttemptResult) -> String { } } +#[cfg(test)] +#[test] +fn startup_status_preserves_pending_and_existing_failure_text() { + use crate::lsp::{registry::ServerKind, roots::ServerKey}; + let pending = ServerAttemptResult::InProgress { + server_key: ServerKey { + kind: ServerKind::Rust, + root: PathBuf::from("workspace"), + }, + }; + assert_eq!(spawn_status(&pending), "start_in_progress"); + assert_eq!( + spawn_status(&ServerAttemptResult::NoRootMarker { + looked_for: Vec::new() + }), + "no_root_marker" + ); +} + fn pull_status(outcome: &PullFileOutcome) -> String { match outcome { PullFileOutcome::Full { diagnostic_count } => { diff --git a/crates/aft/src/context.rs b/crates/aft/src/context.rs index 7e116d4daf..c24fbdddaf 100644 --- a/crates/aft/src/context.rs +++ b/crates/aft/src/context.rs @@ -5558,38 +5558,24 @@ impl AppContext { context } - /// Current omission-preserving status values. Generation identities are - /// checked before project scoping or tsconfig-membership work, so a cache hit - /// faithfully reuses each category's presence or absence. + /// Explicit status reads wait for a coherent diagnostic snapshot. Publishers + /// use the nonblocking accessor so a busy manager never clears their bar. pub fn status_bar_count_values(&self) -> StatusBarCountValues { - self.try_status_bar_count_values() - .unwrap_or_else(|| { - // Explicit status reads may still report independent Tier-2 - // categories. Publishers use the try accessor and skip contention. - let tier2 = self - .status_bar_tier2 - .read() - .unwrap_or_else(std::sync::PoisonError::into_inner); - StatusBarCountValues { - errors: None, - warnings: None, - dead_code: tier2.dead_code, - unused_exports: tier2.unused_exports, - duplicates: tier2.duplicates, - todos: tier2.todos, - tier2_stale: tier2.stale, - disabled_categories: Vec::new(), - diagnostics: None, - diagnostics_reason: None, - diagnostics_gaps: Vec::new(), - } - }) + self.status_bar_count_values_inner(true) + .expect("a blocking status read obtains the language-server manager") .mask_disabled(&self.config().inspect) } /// A busy manager is not a missing diagnostic producer. Publishers must /// leave their previous bar untouched and retry on a later signal. pub(crate) fn try_status_bar_count_values(&self) -> Option { + self.status_bar_count_values_inner(false) + } + + fn status_bar_count_values_inner( + &self, + wait_for_manager: bool, + ) -> Option { #[cfg(test)] STATUS_COUNT_PROBES.with(|count| count.set(count.get() + 1)); let tier2 = self @@ -5598,7 +5584,11 @@ impl AppContext { .unwrap_or_else(std::sync::PoisonError::into_inner) .clone(); let tsconfig_generation = self.tsconfig_membership.lock().generation(); - let lsp = self.lsp_manager.try_lock()?; + let lsp = if wait_for_manager { + self.lsp_manager.lock() + } else { + self.lsp_manager.try_lock()? + }; let diagnostics_generation = lsp.diagnostics_generation(); let (authoritative, producer_gaps) = lsp.project_diagnostics_authority(); let pending_reason = if lsp.has_project_applicability() { diff --git a/crates/aft/src/lsp/client.rs b/crates/aft/src/lsp/client.rs index e1dbf53870..85803a862a 100644 --- a/crates/aft/src/lsp/client.rs +++ b/crates/aft/src/lsp/client.rs @@ -1974,14 +1974,21 @@ impl LspClient { /// published so far describe the files before the save; see /// [`Self::rust_check_state`]. pub(crate) fn record_save_sent(&mut self, uri: &lsp_types::Uri) { + self.record_save_sent_at( + uri, + Instant::now(), + self.rust_check_begins_read.load(Ordering::SeqCst), + ); + } + + fn record_save_sent_at(&mut self, uri: &lsp_types::Uri, at: Instant, begins_read: u64) { if self.rust_checks_on_save { - let now = Instant::now(); self.rust_save = Some(RustSaveRequest { uri: uri.clone(), due_at: None, - last_sent_at: Some(now), + last_sent_at: Some(at), sends: 1, - begins_read_at_send: Some(self.rust_check_begins_read.load(Ordering::SeqCst)), + begins_read_at_send: Some(begins_read), }); } } @@ -2298,6 +2305,38 @@ impl LspClient { }) } + /// Clone the installed writer and rust-analyzer check-begin counter for + /// startup replay. The worker captures each save's write time and current + /// counter value without waiting for the manager lock. + pub(crate) fn startup_notification_writer( + &self, + ) -> Result<(LspWriter, Arc), LspError> { + self.ensure_can_send()?; + Ok(( + self.writer.clone(), + Arc::clone(&self.rust_check_begins_read), + )) + } + + pub(crate) fn record_startup_sync( + &mut self, + uri: &lsp_types::Uri, + opened: bool, + saved: Option<(Instant, u64)>, + ) { + self.note_analysis_document( + if opened { + "textDocument/didOpen" + } else { + "textDocument/didChange" + }, + &json!({"textDocument": {"uri": uri.as_str()}}), + ); + if let Some((at, begins_read)) = saved { + self.record_save_sent_at(uri, at, begins_read); + } + } + /// Send a notification (fire-and-forget). pub fn send_notification(&mut self, params: N::Params) -> Result<(), LspError> where diff --git a/crates/aft/src/lsp/document.rs b/crates/aft/src/lsp/document.rs index 48ff268d65..2e163cfc9b 100644 --- a/crates/aft/src/lsp/document.rs +++ b/crates/aft/src/lsp/document.rs @@ -116,6 +116,15 @@ impl DocumentStore { self.entries.get(path).map(|e| e.version) } + /// Record an explicit version after startup coalesced several unsent edits. + pub(crate) fn sync_at_version(&mut self, path: PathBuf, version: i32, snapshot: &DiskSnapshot) { + self.open_with(path.clone(), snapshot); + self.entries + .get_mut(&path) + .expect("document just inserted") + .version = version; + } + /// Get the full document entry, or None if not open. pub fn entry(&self, path: &Path) -> Option<&DocumentEntry> { self.entries.get(path) diff --git a/crates/aft/src/lsp/instance_regression_tests.rs b/crates/aft/src/lsp/instance_regression_tests.rs index 3acc037972..909d6bd272 100644 --- a/crates/aft/src/lsp/instance_regression_tests.rs +++ b/crates/aft/src/lsp/instance_regression_tests.rs @@ -351,6 +351,11 @@ fn startup_replay_preserves_reader_instance() { key(dir.path()), StartReservation { instance, + definition: None, + source_file: None, + notification_versions: Default::default(), + pending_notifications: Default::default(), + replay_pid: None, deferred_events: Vec::new(), clients_generation: manager.clients_generation, signal: Arc::new(StartSignal::default()), diff --git a/crates/aft/src/lsp/manager.rs b/crates/aft/src/lsp/manager.rs index 45a316cd0a..7fdd3efcb9 100644 --- a/crates/aft/src/lsp/manager.rs +++ b/crates/aft/src/lsp/manager.rs @@ -204,6 +204,8 @@ fn server_key_sort(left: &ServerKey, right: &ServerKey) -> std::cmp::Ordering { /// server". #[derive(Debug, Clone)] pub enum ServerAttemptResult { + /// A reserved startup has not published its client yet. + InProgress { server_key: ServerKey }, /// The root has no live route; reopening it permits a lazy start again. ProjectClosed, /// Server is running and ready to serve requests for this file. @@ -434,15 +436,23 @@ impl ApplicableServerFailure { #[derive(Clone, Debug, Default)] pub struct ApplicableServerStartOutcomes { pub successful: Vec, + pub in_progress: Vec, pub failures: Vec, /// The first producer whose startup could not complete before the inspect /// request's shared work deadline. pub deadline_exceeded: Option, } +pub const START_IN_PROGRESS_STATUS: &str = "start_in_progress"; +pub const START_IN_PROGRESS_REASON: &str = "server startup is in progress; retry shortly"; +pub const STARTUP_NOTIFICATION_PATH_CAP: usize = 8_192; +pub const STARTUP_NOTIFICATION_QUEUE_FULL: &str = + "language-server startup notification queue is full; retry the file change"; + impl ServerAttemptResult { pub fn failure_reason(&self) -> String { match self { + Self::InProgress { .. } => START_IN_PROGRESS_REASON.into(), Self::ProjectClosed => "project closed; reopen the project and retry".into(), Self::BinaryNotInstalled { binary } => format!("{binary} is unavailable"), Self::SpawnFailed { reason, .. } => { @@ -909,7 +919,9 @@ pub struct DrainedLspEvents { pub events: Vec, /// Reader identities in the same order as `events`, retained for dispatch. pub(crate) instances: Vec>, - /// Channel receipts consumed, including events rejected as retired. + /// Events this drain worked through: channel receipts (including ones + /// rejected as retired or held back for a server still starting) plus + /// startup events released from the ready queue. pub processed: usize, pub diagnostics_changed: bool, /// Complete snapshots accepted from document-version-verified publishes @@ -1018,6 +1030,13 @@ pub struct LspShutdownAllOutcome { } pub struct LspManager { + #[cfg(test)] + spawn_hook: Option>, + #[cfg(test)] + spawned_hook: Option>, + #[cfg(test)] + startup_finished_hook: Option>, + startup_owner: Weak>, retired_events_dropped: u64, background_pull_owner: Weak>, background_pull_pending: HashSet<(ServerKey, PathBuf)>, @@ -1044,6 +1063,8 @@ pub struct LspManager { /// Already accepted observations consumed while ordering a pull response. /// The normal event drain delivers them once to status/alert consumers. pending_diagnostic_observations: Vec, + /// Events applied at startup release, awaiting outward delivery exactly once. + ready_startup_events: std::collections::VecDeque, /// Unified event channel — all server reader threads send here. event_tx: Sender, event_rx: Receiver, @@ -1064,7 +1085,7 @@ pub struct LspManager { /// Per-(kind,root) cache of start failures. Once a server fails to start /// for a workspace root, later requests report the remembered failure /// instead of starting it again. Without this, every file open or - /// didChange retries `spawn_server` and logs a fresh ERROR — visible as + /// didChange retries startup and logs a fresh ERROR — visible as /// repeated `failed to spawn TypeScript Language Server: Could not find a /// valid TypeScript installation` lines per edit. /// @@ -1083,6 +1104,7 @@ pub struct LspManager { /// Server/root pairs for which we already logged that watched-file /// notifications are skipped because the capability is absent. watched_file_skip_logged: HashSet, + pending_watched_changes: HashMap, /// rust-analyzer instances with a workspace reload running on a helper /// thread because the file watcher saw a Cargo manifest change (see /// [`spawn_watcher_rust_workspace_reload`]). The value collects the @@ -1329,10 +1351,17 @@ impl LspManager { pub fn new() -> Self { let (event_tx, event_rx) = unbounded(); Self { + #[cfg(test)] + spawn_hook: None, + #[cfg(test)] + spawned_hook: None, retired_events_dropped: 0, rust_cargo_disabled_for_worker: Arc::new(std::sync::atomic::AtomicBool::new(false)), quiesced: std::sync::atomic::AtomicBool::new(false), clients: HashMap::new(), + #[cfg(test)] + startup_finished_hook: None, + startup_owner: Weak::new(), background_pull_owner: Weak::new(), background_pull_pending: HashSet::new(), background_pull_running: false, @@ -1344,6 +1373,7 @@ impl LspManager { documents: HashMap::new(), diagnostics: DiagnosticsStore::new(), pending_diagnostic_observations: Vec::new(), + ready_startup_events: Default::default(), event_tx, event_rx, post_edit_waiters: HashMap::new(), @@ -1354,6 +1384,7 @@ impl LspManager { failed_spawns: HashMap::new(), transient_backoff: HashMap::new(), watched_file_skip_logged: HashSet::new(), + pending_watched_changes: HashMap::new(), watcher_rust_reloads: HashMap::new(), #[cfg(windows)] last_watched_file_notification_trace: "no watched-file notification attempted" @@ -1386,10 +1417,15 @@ impl LspManager { &mut self, owner: Weak>, ) { + self.startup_owner = owner.clone(); self.background_pull_owner = owner; } fn queue_background_document_pull(&mut self, key: &ServerKey, file: &Path) { + // Reserved document versions are not queryable until replay finishes. + if self.starting.contains_key(key) { + return; + } let eligible = self.clients.get(key).is_some_and(|client| { client.uses_analysis_progress() && client.state() == ServerState::Ready @@ -1529,9 +1565,12 @@ impl LspManager { result: ServerAttemptResult, durability: FailureDurability, ) -> ServerAttemptResult { - // Closing a route is not a failed executable. Reopening the project - // must permit an immediate start rather than replay a retry backoff. - if matches!(result, ServerAttemptResult::ProjectClosed) { + // Closing a route or joining a reserved start is not an executable + // failure and must not fabricate a retry backoff. + if matches!( + result, + ServerAttemptResult::ProjectClosed | ServerAttemptResult::InProgress { .. } + ) { return result; } let now = self.retry_clock.now(); @@ -1880,196 +1919,34 @@ impl LspManager { &mut self, snapshot: &ApplicableServerSnapshot, config: &Config, - deadline: Option, + deadline: Option, ) -> ApplicableServerStartOutcomes { let mut outcomes = ApplicableServerStartOutcomes { failures: snapshot.producer_failures.clone(), ..ApplicableServerStartOutcomes::default() }; for candidate in &snapshot.candidates { - let initialize_timeout = deadline - .map(|deadline| deadline.saturating_duration_since(std::time::Instant::now())); - if initialize_timeout.is_some_and(|remaining| remaining.is_zero()) { + if deadline.is_some_and(|deadline| Instant::now() >= deadline) { outcomes.deadline_exceeded = Some(candidate.key.clone()); break; } - if self.clients.contains_key(&candidate.key) { - outcomes.successful.push(candidate.key.clone()); - continue; - } - self.log_transient_retry(&candidate.key); - match self.spawn_server_with_timeout( + match self.start_server( &candidate.definition, - &candidate.key.root, + &candidate.key, &candidate.source_file, config, - initialize_timeout, ) { - Ok(client) => { - self.note_start_succeeded(&candidate.key); - self.clients.insert(candidate.key.clone(), client); - self.server_binaries - .insert(candidate.key.clone(), candidate.definition.binary.clone()); - self.documents.entry(candidate.key.clone()).or_default(); - outcomes.successful.push(candidate.key.clone()); + ServerAttemptResult::Ok { server_key } => outcomes.successful.push(server_key), + ServerAttemptResult::InProgress { server_key } => { + outcomes.in_progress.push(server_key) } - Err(failure) => { - if deadline.is_some_and(|deadline| std::time::Instant::now() >= deadline) { - // A request-owned timeout says nothing durable about the - // producer; a later call with a fresh budget may retry it. - outcomes.deadline_exceeded = Some(candidate.key.clone()); - break; - } - let result = classify_spawn_error(&candidate.definition.binary, &failure.error); - let result = - self.record_failed_spawn(&candidate.key, result, failure.durability); - outcomes.failures.push(ApplicableServerFailure { - server_key: candidate.key.clone(), - result, - }); - } - } - } - outcomes - } - - /// First step of [`start_applicable_server_unlocked`], under the lock. - /// Returns `None` once `outcomes` holds this server's result; otherwise - /// says whether to wait for another thread's start or spawn unlocked. A - /// start that follows a wait reports the other thread's cached failure - /// instead of retrying it. - fn begin_unlocked_start( - &mut self, - candidate: &ApplicableServerCandidate, - config: &Config, - after_wait: bool, - deadline: Instant, - outcomes: &mut ApplicableServerStartOutcomes, - ) -> Option { - let key = &candidate.key; - if self.is_quiesced() { - outcomes.failures.push(ApplicableServerFailure { - server_key: key.clone(), - result: ServerAttemptResult::ProjectClosed, - }); - return None; - } - if self.clients.contains_key(key) { - outcomes.successful.push(key.clone()); - return None; - } - if let Some(reservation) = self.starting.get(key) { - return Some(StartNext::Wait(Arc::clone(&reservation.signal))); - } - if after_wait { - if let Some(result) = self.failure_to_replay(key) { - outcomes.failures.push(ApplicableServerFailure { - server_key: key.clone(), + result => outcomes.failures.push(ApplicableServerFailure { + server_key: candidate.key.clone(), result, - }); - return None; + }), } } - match self.prepare_spawn( - &candidate.definition, - &key.root, - &candidate.source_file, - config, - ) { - Ok(prepared) => { - self.log_transient_retry(key); - self.starting.insert( - key.clone(), - StartReservation { - instance: prepared.instance, - deferred_events: Vec::new(), - clients_generation: self.clients_generation, - signal: Arc::new(StartSignal::default()), - }, - ); - Some(StartNext::Spawn(prepared)) - } - Err(error) => { - self.record_start_error(candidate, &prepare_failure(error), deadline, outcomes); - None - } - } - } - - /// Last step of [`start_applicable_server_unlocked`], under the lock: - /// publish the client or record the failure, then release the reservation. - fn finish_unlocked_start( - &mut self, - candidate: &ApplicableServerCandidate, - result: Result, - deadline: Instant, - outcomes: &mut ApplicableServerStartOutcomes, - ) { - let key = &candidate.key; - let started_generation = self - .starting - .get(key) - .map(|reservation| reservation.clients_generation); - match result { - Ok(client) - if self.rust_cargo_disabled_for_worker() - && key.kind == ServerKind::Rust - && !client.rust_cargo_disabled_for_worker => - { - outcomes.failures.push(ApplicableServerFailure { - server_key: key.clone(), - result: ServerAttemptResult::SpawnFailed { - binary: candidate.definition.binary.clone(), - reason: RUST_WORKER_POLICY_CHANGED.into(), - }, - }); - } - // A start through another path (an edit's diagnostics, say) won - // the race; keep that client and let this one's drop stop its - // process. - Ok(_) if self.clients.contains_key(key) => { - slog_info!( - "lsp start discarded server={} root={} reason=duplicate start; stopping redundant client", - key.kind.id_str(), - key.root.display() - ); - outcomes.successful.push(key.clone()); - } - // Every client was taken away while this one was starting - // (shutdown, idle reap, unbind); do not revive the manager. - Ok(_) if started_generation != Some(self.clients_generation) => { - slog_info!( - "lsp start discarded server={} root={} reason=shutdown during initialize; stopping client", - key.kind.id_str(), - key.root.display() - ); - outcomes.failures.push(ApplicableServerFailure { - server_key: key.clone(), - result: ServerAttemptResult::SpawnFailed { - binary: candidate.definition.binary.clone(), - reason: "language servers were shut down while this one was starting" - .to_string(), - }, - }); - } - Ok(client) => { - self.note_start_succeeded(key); - self.clients.insert(key.clone(), client); - self.server_binaries - .insert(key.clone(), candidate.definition.binary.clone()); - self.documents.entry(key.clone()).or_default(); - outcomes.successful.push(key.clone()); - } - Err(failure) => { - let failure = self.absorb_spawn_failure(failure); - if self.clients.contains_key(key) { - outcomes.successful.push(key.clone()); - } else { - self.record_start_error(candidate, &failure, deadline, outcomes); - } - } - } - self.release_start_reservation(key); + outcomes } /// Drop a start reservation, handle the events held back for it, and wake @@ -2078,32 +1955,44 @@ impl LspManager { let Some(reservation) = self.starting.remove(key) else { return; }; - for event in &reservation.deferred_events { - let _ = self.handle_event(event); - } - reservation.signal.finish(); - } - - /// Report a start that produced no client, as the locked start path does: - /// a failure caused by the request deadline is not cached, any other one - /// is, so later file events skip the server. - fn record_start_error( - &mut self, - candidate: &ApplicableServerCandidate, - failure: &StartFailure, - deadline: Instant, - outcomes: &mut ApplicableServerStartOutcomes, - ) { - if Instant::now() >= deadline { - outcomes.deadline_exceeded = Some(candidate.key.clone()); + if reservation + .signal + .cancelled + .load(std::sync::atomic::Ordering::Acquire) + { + self.pending_watched_changes.remove(key); + reservation.signal.finish(); + self.wake_post_edit_waiters(); return; } - let result = classify_spawn_error(&candidate.definition.binary, &failure.error); - let result = self.record_failed_spawn(&candidate.key, result, failure.durability); - outcomes.failures.push(ApplicableServerFailure { - server_key: candidate.key.clone(), - result, - }); + for event in reservation.deferred_events { + // A receipt from a reader that no longer owns the key was dropped + // by `handle_event`; like the normal drain, it is not delivered. + let Ok(published_file) = self.handle_event(&event) else { + continue; + }; + let published = published_file.map(|file| { + let version = self + .documents + .get(key) + .and_then(|store| store.version(&file)); + let epoch = self.diagnostic_epoch(key, &file); + let pid = self.clients.get(key).map(LspClient::child_pid); + StartupPublished { + key: key.clone(), + file, + version, + epoch, + pid, + } + }); + self.ready_startup_events + .push_back(StartupEventDelivery { event, published }); + } + reservation.signal.finish(); + // A post-edit wait parked on this start must re-check: the server + // is now either running or gone. + self.wake_post_edit_waiters(); } /// First step of [`ensure_server_for_file_detailed_unlocked`] for one @@ -2117,15 +2006,16 @@ impl LspManager { file_path: &Path, config: &Config, ) -> FileServerStart { + self.publish_completed_starts(); if self.is_quiesced() { return FileServerStart::Failed(ServerAttemptResult::ProjectClosed); } + if self.starting.contains_key(key) { + return FileServerStart::Wait; + } if self.clients.contains_key(key) { return FileServerStart::Running; } - if let Some(reservation) = self.starting.get(key) { - return FileServerStart::Wait(Arc::clone(&reservation.signal)); - } if let Some(cached) = self.failure_to_replay(key) { return FileServerStart::Failed(cached); } @@ -2136,10 +2026,15 @@ impl LspManager { self.starting.insert( key.clone(), StartReservation { + definition: Some(def.clone()), + source_file: Some(file_path.to_path_buf()), instance: prepared.instance, deferred_events: Vec::new(), clients_generation: self.clients_generation, signal: Arc::new(StartSignal::default()), + notification_versions: HashMap::new(), + pending_notifications: HashSet::new(), + replay_pid: None, }, ); FileServerStart::Spawn(Box::new(prepared)) @@ -2152,6 +2047,350 @@ impl LspManager { } } + /// Reserve under mutable manager access, but create the process only on a + /// startup worker. This method cannot release a caller's MutexGuard over + /// LspManager, so it must not run the spawn or handshake inline. + fn start_server( + &mut self, + def: &ServerDef, + key: &ServerKey, + file: &Path, + config: &Config, + ) -> ServerAttemptResult { + match self.begin_file_server_start(def, key, file, config) { + FileServerStart::Running => ServerAttemptResult::Ok { + server_key: key.clone(), + }, + FileServerStart::Failed(result) => result, + FileServerStart::Wait => ServerAttemptResult::InProgress { + server_key: key.clone(), + }, + FileServerStart::Spawn(mut prepared) => { + let signal = Arc::clone(&self.starting[key].signal); + prepared.start_signal = Some(Arc::clone(&signal)); + let owner = self.startup_owner.clone(); + #[cfg(test)] + let finished_hook = self.startup_finished_hook.clone(); + let worker_signal = Arc::clone(&signal); + let worker = std::thread::Builder::new() + .name("lsp-startup".into()) + .spawn(move || { + let guard = StartupWorkerGuard::new(Arc::clone(&worker_signal)); + let timeout = prepared.inspect_initialize_timeout(); + let result = prepared.run(Some(timeout)); + guard.publish(|signal| signal.complete(result)); + if let Some(manager) = owner.upgrade() { + manager.lock().publish_completed_starts(); + } + #[cfg(test)] + if let Some(hook) = finished_hook { + hook(); + } + }); + if let Err(error) = worker { + signal.complete(Err(SpawnFailure::NotStarted { + exit_log_line: error.to_string(), + error: error.into(), + })); + } + ServerAttemptResult::InProgress { + server_key: key.clone(), + } + } + } + } + + /// Publish each completed startup phase under the manager lock. Notification + /// replay uses a cloned writer on its own worker, never this lock. + fn publish_completed_starts(&mut self) { + let keys: Vec<_> = self.starting.keys().cloned().collect(); + for key in keys { + let Some(reservation) = self.starting.get(&key) else { + continue; + }; + let signal = Arc::clone(&reservation.signal); + if !signal.wait_until(Instant::now()) { + continue; + } + let cancelled = self.is_quiesced() + || reservation.clients_generation != self.clients_generation + || signal.cancelled.load(std::sync::atomic::Ordering::Acquire); + let replay_pid = reservation.replay_pid; + let definition = reservation.definition.clone(); + if cancelled { + signal.cancel(); + if replay_pid.is_some_and(|pid| { + self.clients + .get(&key) + .is_some_and(|client| client.child_pid() == pid) + }) { + self.clients.remove(&key); + self.forget_server_state(Some(&key)); + } + self.release_start_reservation(&key); + } else if replay_pid.is_some() { + let completed = signal.replay_completion.lock().take(); + if let Some(result) = completed { + self.finish_notification_replay(&key, result); + } + } else { + let completed = signal.completion.lock().take(); + if let (Some(result), Some(definition)) = (completed, definition) { + self.finish_file_server_start(&definition, &key, result); + } + } + } + self.flush_pending_watched_changes(); + } + + /// Wait for starts admitted by a locked call, including queued document + /// replay. A standalone manager publishes its own completed worker phases. + /// Shared-manager callers should prefer the unlocked startup helpers. + pub fn wait_for_startups(&mut self, deadline: Instant) -> bool { + let signals: Vec<_> = self + .starting + .values() + .map(|r| Arc::clone(&r.signal)) + .collect(); + self.wait_for_startup_signals(signals, deadline) + } + + fn wait_for_file_startups(&mut self, file: &Path, config: &Config, deadline: Instant) -> bool { + // Accepted notifications identify their owner even if a later root or + // definition lookup no longer finds the same reservation. + let expected = expected_server_keys_for_file(file, config); + let signals = self + .starting + .iter() + .filter(|(key, reservation)| { + expected.contains(key) || reservation.notification_versions.contains_key(file) + }) + .map(|(_, reservation)| Arc::clone(&reservation.signal)) + .collect(); + self.wait_for_startup_signals(signals, deadline) + } + + fn wait_for_startup_signals( + &mut self, + signals: Vec>, + deadline: Instant, + ) -> bool { + for signal in signals { + loop { + self.publish_completed_starts(); + if signal.outcome.lock().is_some() { + break; + } + if !signal.wait_until(deadline) { + return false; + } + } + } + true + } + + pub fn has_pending_startups(&self) -> bool { + !self.starting.is_empty() + } + + fn queue_startup_notification( + &mut self, + key: &ServerKey, + path: &Path, + ) -> Result { + self.mark_pulled_report_stale(key, path); + let reservation = self + .starting + .get_mut(key) + .expect("startup reservation exists"); + if !reservation.notification_versions.contains_key(path) + && reservation.notification_versions.len() >= STARTUP_NOTIFICATION_PATH_CAP + { + return Err(LspError::ServerNotReady( + STARTUP_NOTIFICATION_QUEUE_FULL.into(), + )); + } + let version = reservation + .notification_versions + .get(path) + .copied() + .or_else(|| { + self.documents + .get(key) + .and_then(|store| store.version(path)) + }) + .map_or(0, |version| version + 1); + reservation + .notification_versions + .insert(path.to_path_buf(), version); + reservation.pending_notifications.insert(path.to_path_buf()); + Ok(version) + } + + /// Snapshot paths and reserve their versions under the manager lock. The + /// worker reads current disk contents and sends through this client's writer. + fn start_notification_replay(&mut self, key: &ServerKey) { + let sender = self + .clients + .get(key) + .ok_or_else(|| LspError::ServerNotReady("server not found".into())) + .and_then(|client| { + Ok(( + client.startup_notification_writer()?, + client.save_notification(), + client.child_pid(), + )) + }); + let ((writer, begins_read), save, pid) = match sender { + Ok(sender) => sender, + Err(error) => { + self.complete_startup_outcome( + key, + Some(ServerAttemptResult::SpawnFailed { + binary: self.server_binaries.get(key).cloned().unwrap_or_default(), + reason: error.to_string(), + }), + ); + return; + } + }; + let reservation = self + .starting + .get_mut(key) + .expect("startup reservation exists"); + let paths = std::mem::take(&mut reservation.pending_notifications); + let mut jobs: Vec<_> = paths + .into_iter() + .map(|path| { + let version = reservation.notification_versions[&path]; + let opened = !self + .documents + .get(key) + .is_some_and(|store| store.is_open(&path)); + (path, version, opened) + }) + .collect(); + jobs.sort_by(|left, right| left.1.cmp(&right.1).then_with(|| left.0.cmp(&right.0))); + reservation.replay_pid = Some(pid); + let signal = Arc::clone(&reservation.signal); + *signal + .finished + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) = false; + let owner = self.startup_owner.clone(); + let worker_signal = Arc::clone(&signal); + let worker = std::thread::Builder::new() + .name("lsp-startup-sync".into()) + .spawn(move || { + let guard = StartupWorkerGuard::new(Arc::clone(&worker_signal)); + let result = replay_startup_notifications( + writer, + save, + begins_read, + pid, + &worker_signal, + jobs, + ); + guard.publish(|signal| { + *signal.replay_completion.lock() = Some(result); + signal.finish(); + }); + if let Some(manager) = owner.upgrade() { + manager.lock().publish_completed_starts(); + } + }); + if let Err(error) = worker { + *signal.replay_completion.lock() = Some(StartupReplayResult { + pid, + documents: Vec::new(), + error: Some(error.into()), + }); + signal.finish(); + } + } + + fn finish_notification_replay(&mut self, key: &ServerKey, result: StartupReplayResult) { + if !self + .clients + .get(key) + .is_some_and(|client| client.child_pid() == result.pid) + { + self.complete_startup_outcome( + key, + Some(ServerAttemptResult::SpawnFailed { + binary: self.server_binaries.get(key).cloned().unwrap_or_default(), + reason: "server not found".into(), + }), + ); + return; + } + for document in result.documents { + self.documents + .entry(key.clone()) + .or_default() + .sync_at_version(document.path.clone(), document.version, &document.disk); + if let Ok(uri) = uri_for_path(&document.path) { + if let Some(client) = self.clients.get_mut(key) { + client.record_startup_sync(&uri, document.opened, document.saved); + } + } + } + if let Some(error) = result.error { + self.complete_startup_outcome( + key, + Some(ServerAttemptResult::SpawnFailed { + binary: self.server_binaries.get(key).cloned().unwrap_or_default(), + reason: error.to_string(), + }), + ); + } else if self + .starting + .get(key) + .is_some_and(|r| !r.pending_notifications.is_empty()) + { + self.start_notification_replay(key); + } else { + self.complete_startup_outcome(key, None); + } + } + + /// Publish the final startup outcome for all waiters before releasing the reservation. + fn complete_startup_outcome(&mut self, key: &ServerKey, outcome: Option) { + let ready = outcome.is_none(); + if !ready { + self.pending_watched_changes.remove(key); + } + if let Some(reservation) = self.starting.get(key) { + *reservation.signal.outcome.lock() = + Some(outcome.unwrap_or_else(|| ServerAttemptResult::Ok { + server_key: key.clone(), + })); + } + self.release_start_reservation(key); + if ready { + self.queue_open_document_pulls(key); + } + } + + pub(crate) fn pending_startup_files(&self) -> Vec<(ServerKey, PathBuf)> { + let mut pending: Vec<_> = self + .starting + .iter() + .filter(|(_, r)| { + !r.signal + .cancelled + .load(std::sync::atomic::Ordering::Acquire) + }) + .filter_map(|(key, r)| { + r.source_file + .as_ref() + .map(|file| (key.clone(), file.clone())) + }) + .collect(); + pending.sort_by(|(left, _), (right, _)| server_key_sort(left, right)); + pending + } + /// Remember a failed start the way [`Self::ensure_server_for_file_detailed`] /// does, and return the result to report. fn record_file_start_failure( @@ -2192,8 +2431,7 @@ impl LspManager { reason: RUST_WORKER_POLICY_CHANGED.into(), }) } - // Another path started the same server meanwhile; keep that - // client and let this one's drop stop its process. + // Keep an already-published client if a completion is replayed. Ok(_) if self.clients.contains_key(key) => { slog_info!( "lsp start discarded server={} root={} reason=duplicate start; stopping redundant client", @@ -2228,11 +2466,30 @@ impl LspManager { if self.clients.contains_key(key) { None } else { - Some(self.record_file_start_failure(def, key, failure)) + Some( + if self.starting.get(key).is_some_and(|r| { + r.signal + .request_deadline_exceeded + .load(std::sync::atomic::Ordering::Acquire) + }) { + classify_spawn_error(&def.binary, &failure.error) + } else { + self.record_file_start_failure(def, key, failure) + }, + ) } } }; - self.release_start_reservation(key); + if outcome.is_none() + && self + .starting + .get(key) + .is_some_and(|r| !r.pending_notifications.is_empty()) + { + self.start_notification_replay(key); + } else { + self.complete_startup_outcome(key, outcome.clone()); + } outcome } @@ -2251,7 +2508,7 @@ impl LspManager { servers_for_file(file_path, config) .into_iter() .filter_map(|def| server_key_for_definition(&def, file_path, config)) - .filter(|key| self.clients.contains_key(key)) + .filter(|key| self.clients.contains_key(key) || self.starting.contains_key(key)) .collect() } @@ -2274,6 +2531,9 @@ impl LspManager { Some((definition, key)) }) .any(|(definition, key)| { + if self.starting.contains_key(&key) { + return true; + } if let Some(client) = self.clients.get(&key) { return client.state() != ServerState::Ready; } @@ -2282,95 +2542,32 @@ impl LspManager { }) } - /// Detailed version of [`ensure_server_for_file`] that records every - /// matching server's outcome (`Ok` / `NoRootMarker` / `BinaryNotInstalled` - /// / `SpawnFailed`). - /// - /// Use this when the caller wants to honestly report _why_ a file has no - /// active server (e.g., to surface "bash-language-server not on PATH" to - /// the agent instead of silently returning `total: 0`). + /// Ensure matching servers are admitted without blocking the caller's + /// manager guard. A cold start returns `InProgress`; use + /// [`ensure_server_for_file_detailed_unlocked`] to wait without holding a + /// shared manager mutex. pub fn ensure_server_for_file_detailed( &mut self, file_path: &Path, config: &Config, ) -> EnsureServerOutcomes { - let defs = servers_for_file(file_path, config); let mut outcomes = EnsureServerOutcomes::default(); - - for def in defs { - let server_id = def.kind.id_str().to_string(); - let server_name = def.name.to_string(); - - let Some(key) = server_key_for_definition(&def, file_path, config) else { - outcomes.attempts.push(ServerAttempt { - server_id, - server_name, - result: ServerAttemptResult::NoRootMarker { - looked_for: def.root_markers.iter().map(|s| s.to_string()).collect(), - }, - }); - continue; + for def in servers_for_file(file_path, config) { + let result = match server_key_for_definition(&def, file_path, config) { + Some(key) => self.start_server(&def, &key, file_path, config), + None => ServerAttemptResult::NoRootMarker { + looked_for: def.root_markers.iter().map(|s| s.to_string()).collect(), + }, }; - - if !self.clients.contains_key(&key) { - // If this server already failed to start for this root, - // return the remembered failure without retrying or - // re-logging. This prevents per-edit ERROR spam when the - // user's environment is missing a dependency the LSP needs - // (the typescript-language-server "Could not find a valid - // TypeScript installation" case is the canonical example). - // A transient failure is retried once its backoff window has - // passed. - if let Some(cached) = self.failure_to_replay(&key) { - outcomes.attempts.push(ServerAttempt { - server_id, - server_name, - result: cached, - }); - continue; - } - - // A spurious ServerExited can drop the client without killing - // the child. Reap any still-tracked processes for this pair - // before spawning a replacement, otherwise leaked servers - // accumulate on a live worktree. - self.reap_unreferenced_children_for(&key); - self.log_transient_retry(&key); - - match self.spawn_server(&def, &key.root, file_path, config) { - Ok(client) => { - self.note_start_succeeded(&key); - self.clients.insert(key.clone(), client); - self.server_binaries.insert(key.clone(), def.binary.clone()); - self.documents.entry(key.clone()).or_default(); - } - Err(failure) => { - slog_error!("failed to spawn {}: {}", def.name, failure.error); - let result = classify_spawn_error(&def.binary, &failure.error); - // Remember the failure so subsequent file events skip - // this (kind, root) pair instead of producing a fresh - // spawn attempt + ERROR log per request. - let result = self.record_failed_spawn(&key, result, failure.durability); - outcomes.attempts.push(ServerAttempt { - server_id, - server_name, - result, - }); - continue; - } - } + if let ServerAttemptResult::Ok { server_key } = &result { + outcomes.successful.push(server_key.clone()); } - outcomes.attempts.push(ServerAttempt { - server_id, - server_name, - result: ServerAttemptResult::Ok { - server_key: key.clone(), - }, + server_id: def.kind.id_str().to_string(), + server_name: def.name.to_string(), + result, }); - outcomes.successful.push(key); } - outcomes } @@ -2559,12 +2756,14 @@ impl LspManager { /// per server so the post-edit waiter can match `publishDiagnostics` /// against the exact version that this notification carried. /// - /// Returns: `Vec<(ServerKey, target_version)>`. `target_version` is the - /// `version` field on the `VersionedTextDocumentIdentifier` we just sent - /// (post-bump). For freshly-opened documents (`didOpen`) the version is - /// `0`. Servers that do not echo a document version cannot prove a report - /// describes this edit, so post-edit observation leaves them pending rather - /// than accepting a report merely because it arrived after the edit. + /// Returns `Vec<(ServerKey, target_version)>` for sent notifications or + /// versions reserved for startup replay. New documents start at version 0; + /// coalesced startup edits can make the first didOpen carry a later version. + /// A reserved version is not evidence of analysis: post-edit observation + /// still requires diagnostics matching that version. Servers that do not + /// echo a document version cannot prove a report describes this edit, so + /// observation leaves them pending rather than accepting a report merely + /// because it arrived after the edit. pub fn notify_file_changed_versioned( &mut self, file_path: &Path, @@ -2572,7 +2771,16 @@ impl LspManager { config: &Config, ) -> Result, LspError> { let canonical_path = canonicalize_for_lsp(file_path)?; - let server_keys = self.ensure_server_for_file(&canonical_path, config); + let outcomes = self.ensure_server_for_file_detailed(&canonical_path, config); + let server_keys = outcomes + .attempts + .into_iter() + .filter_map(|attempt| match attempt.result { + ServerAttemptResult::Ok { server_key } + | ServerAttemptResult::InProgress { server_key } => Some(server_key), + _ => None, + }) + .collect(); self.notify_file_changed_for_server_keys(canonical_path, content, server_keys) } @@ -2630,6 +2838,11 @@ impl LspManager { let disk = DiskSnapshot::read(&canonical_path); for key in server_keys { + if self.starting.contains_key(&key) { + let version = self.queue_startup_notification(&key, &canonical_path)?; + versions.push((key, version)); + continue; + } // The server is about to hold text its pulled report may not // describe. A pull-only server publishes nothing after the // change, so without this a scoped inspect would keep certifying @@ -2814,16 +3027,133 @@ impl LspManager { self.notify_file_changed(file_path, content, &Config::default()) } - /// Notify every active server whose workspace contains at least one changed - /// path that watched files changed. This is intentionally workspace-scoped - /// rather than extension-scoped: configuration edits such as `package.json` - /// or `tsconfig.json` affect a server's project graph even though those - /// files may not be documents handled by the server itself. + /// Keep only the latest event for each path. Explicit LSP change kinds are + /// retained; inferring them again would turn CREATED into CHANGED. + fn queue_watched_change( + &mut self, + key: &ServerKey, + path: PathBuf, + kind: FileChangeType, + raw: Option, + extra_config_markers: Vec, + ) -> Result<(), LspError> { + let pid = self.clients.get(key).map(LspClient::child_pid); + let pending = self + .pending_watched_changes + .entry(key.clone()) + .or_insert_with(|| PendingWatchedChanges { + generation: self.clients_generation, + pid, + changes: HashMap::new(), + }); + if pending.generation != self.clients_generation + || pending.pid.is_some_and(|old| Some(old) != pid) + { + *pending = PendingWatchedChanges { + generation: self.clients_generation, + pid, + changes: HashMap::new(), + }; + } + if !pending.changes.contains_key(&path) + && pending.changes.len() >= STARTUP_NOTIFICATION_PATH_CAP + { + return Err(LspError::ServerNotReady( + STARTUP_NOTIFICATION_QUEUE_FULL.into(), + )); + } + pending.changes.insert( + path, + PendingWatchedChange { + kind, + raw, + extra_config_markers, + }, + ); + Ok(()) + } + + /// Replay through the existing watched-file send paths only after the + /// client and its dynamic subscription are ready. Each batch stays bound + /// to its workspace, generation and client rather than broadcasting twice. + fn flush_pending_watched_changes(&mut self) { + if self.is_quiesced() { + return; + } + let keys: Vec<_> = self + .pending_watched_changes + .keys() + .filter(|key| { + !self.starting.contains_key(*key) + && self + .clients + .get(*key) + .is_some_and(LspClient::has_watched_file_registration) + }) + .cloned() + .collect(); + let mut reloads = Vec::new(); + for key in keys { + let Some(pending) = self.pending_watched_changes.remove(&key) else { + continue; + }; + let pid = self.clients.get(&key).map(LspClient::child_pid); + if pending.generation != self.clients_generation + || pending.pid.is_some_and(|old| Some(old) != pid) + { + continue; + } + let mut direct = Vec::new(); + let mut forwarded: HashMap, Vec<(PathBuf, FileChangeType)>> = + HashMap::new(); + for (path, change) in pending.changes { + if let Some(raw) = change.raw { + forwarded + .entry(change.extra_config_markers) + .or_default() + .push((raw, change.kind)); + } else { + direct.push((path, change.kind)); + } + } + if !direct.is_empty() { + if let Err(error) = + self.notify_files_watched_changed_for_server(&direct, Some(&key)) + { + crate::slog_warn!("queued watched-file sync failed for {key:?}: {error}"); + } + } + for (markers, changes) in forwarded { + reloads.extend(self.forward_watcher_file_events_for_server( + &changes, + &markers, + Some(&key), + )); + } + } + if let Some(manager) = self.startup_owner.upgrade() { + spawn_watcher_rust_workspace_reloads(&manager, reloads); + } + } + + /// Notify every active or starting server whose workspace contains a changed + /// path. Pending startup or registration retains the event's explicit kind. pub fn notify_files_watched_changed( &mut self, paths: &[(PathBuf, FileChangeType)], _config: &Config, ) -> Result<(), LspError> { + self.notify_files_watched_changed_for_server(paths, None) + } + + fn notify_files_watched_changed_for_server( + &mut self, + paths: &[(PathBuf, FileChangeType)], + only_server: Option<&ServerKey>, + ) -> Result<(), LspError> { + if self.is_quiesced() { + return Err(LspError::ProjectClosed); + } #[cfg(windows)] let mut trace = vec![format!( "input_paths={paths:?}; active_keys={:?}", @@ -2848,12 +3178,41 @@ impl LspManager { trace.push(format!("resolved_events={canonical_events:?}")); let mut first_error = None; - let keys: Vec = self.clients.keys().cloned().collect(); + let keys: Vec = self + .clients + .keys() + .chain(self.starting.keys()) + .filter(|key| only_server.is_none_or(|only| only == *key)) + .cloned() + .collect::>() + .into_iter() + .collect(); #[cfg(windows)] if keys.is_empty() { trace.push("outcome=no-active-client".to_string()); } for key in keys { + if self.starting.contains_key(&key) + || !self + .clients + .get(&key) + .is_some_and(LspClient::has_watched_file_registration) + { + for (path, typ) in &canonical_events { + if path.starts_with(&key.root) { + if let Err(error) = + self.queue_watched_change(&key, path.clone(), *typ, None, Vec::new()) + { + first_error.get_or_insert(error); + } + } + } + #[cfg(windows)] + trace.push(format!( + "key={key:?}; outcome=queued-until-ready-registration" + )); + continue; + } let mut changes = Vec::new(); for (path, typ) in &canonical_events { if !path.starts_with(&key.root) { @@ -2955,9 +3314,21 @@ impl LspManager { &mut self, events: &[(PathBuf, FileChangeType)], extra_config_markers: &[String], + ) -> Vec<(ServerKey, Vec)> { + self.forward_watcher_file_events_for_server(events, extra_config_markers, None) + } + + fn forward_watcher_file_events_for_server( + &mut self, + events: &[(PathBuf, FileChangeType)], + extra_config_markers: &[String], + only_server: Option<&ServerKey>, ) -> Vec<(ServerKey, Vec)> { let mut rust_manifest_changes = Vec::new(); - if events.is_empty() || self.clients.is_empty() { + if self.is_quiesced() { + return rust_manifest_changes; + } + if events.is_empty() || (self.clients.is_empty() && self.starting.is_empty()) { return rust_manifest_changes; } // A server's root and globs may spell a path either the way the @@ -2971,7 +3342,15 @@ impl LspManager { (path.as_path(), resolve_for_lsp_uri(path), *typ) }) .collect(); - let keys: Vec = self.clients.keys().cloned().collect(); + let keys: Vec = self + .clients + .keys() + .chain(self.starting.keys()) + .filter(|key| only_server.is_none_or(|only| only == *key)) + .cloned() + .collect::>() + .into_iter() + .collect(); for key in keys { let in_root: Vec<&(&Path, PathBuf, FileChangeType)> = events .iter() @@ -2994,7 +3373,28 @@ impl LspManager { let mut overflowed = false; let mut sent_to_registered_watcher: Vec = Vec::new(); - if let Some(client) = self.clients.get(&key) { + let waiting = self.starting.contains_key(&key) + || !self + .clients + .get(&key) + .is_some_and(LspClient::has_watched_file_registration); + if waiting { + for (raw, resolved, typ) in &in_root { + if let Err(error) = self.queue_watched_change( + &key, + resolved.clone(), + *typ, + Some(raw.to_path_buf()), + extra_config_markers.to_vec(), + ) { + crate::slog_warn!( + "queued watched-file forward failed for {key:?}: {error}" + ); + } + } + } + + if let Some(client) = self.clients.get(&key).filter(|_| !waiting) { if client.has_watched_file_registration() { let watchers = client.registered_file_watchers(); let is_config = @@ -3494,7 +3894,7 @@ impl LspManager { .find_map(|(key, entry)| (key == server_key).then_some(entry.epoch)) } - /// Whether this server is a running client. + /// Whether this server has a running client, including pending startup replay. pub(crate) fn has_client(&self, server_key: &ServerKey) -> bool { self.clients.contains_key(server_key) } @@ -3661,6 +4061,9 @@ impl LspManager { /// Get an active client for a file path, if one exists. pub fn client_for_file(&self, file_path: &Path, config: &Config) -> Option<&LspClient> { let key = self.server_key_for_file(file_path, config)?; + if self.starting.contains_key(&key) { + return None; + } self.clients.get(&key) } @@ -3675,6 +4078,9 @@ impl LspManager { config: &Config, ) -> Option<&mut LspClient> { let key = self.server_key_for_file(file_path, config)?; + if self.starting.contains_key(&key) { + return None; + } self.clients.get_mut(&key) } @@ -3695,7 +4101,17 @@ impl LspManager { /// Whether LSP events are waiting to be drained. Cheap channel peek for /// the maintenance scheduler's skip probe. pub fn has_pending_events(&self) -> bool { - !self.event_rx.is_empty() || !self.pending_diagnostic_observations.is_empty() + !self.event_rx.is_empty() + || !self.pending_diagnostic_observations.is_empty() + || !self.ready_startup_events.is_empty() + || (!self.is_quiesced() + && self.pending_watched_changes.keys().any(|key| { + !self.starting.contains_key(key) + && self + .clients + .get(key) + .is_some_and(LspClient::has_watched_file_registration) + })) } pub fn drain_events_bounded(&mut self, max_events: usize) -> DrainedLspEvents { @@ -3707,6 +4123,7 @@ impl LspManager { max_events: usize, preserve_pull_observations: bool, ) -> DrainedLspEvents { + self.publish_completed_starts(); let mut events = Vec::new(); let mut instances = Vec::new(); let mut processed = 0; @@ -3718,6 +4135,24 @@ impl LspManager { }; let mut diagnostics_changed = !accepted_snapshots.is_empty(); while processed < max_events { + if !preserve_pull_observations { + if let Some(delivery) = self.ready_startup_events.pop_front() { + processed += 1; + diagnostics_changed |= delivery.published.is_some(); + if let Some(ref published) = delivery.published { + if self.startup_publish_is_current(published) { + if let Some(snapshot) = + self.accepted_snapshot_for_file(&published.key, &published.file) + { + accepted_snapshots.push(snapshot); + } + } + } + instances.push(delivery.event.instance); + events.push(delivery.event.event); + continue; + } + } let Ok(event) = self.event_rx.try_recv() else { break; }; @@ -3730,12 +4165,15 @@ impl LspManager { } continue; } - // `handle_event` returns the file of a stored publish; the - // snapshot reuses it instead of parsing the payload again. - let Ok(published_file) = self.handle_event(&event) else { + if !self.admit_event(&event) { + continue; + } + if self.defer_startup_event(&event) { continue; - }; - if let Some(file) = published_file { + } + // `handle_ready_event` returns the file of a stored publish; the + // snapshot reuses it instead of parsing the payload again. + if let Some(file) = self.handle_ready_event(&event) { diagnostics_changed = true; if let Some(snapshot) = self.accepted_live_publish_snapshot(&event.event, &file) { accepted_snapshots.push(snapshot); @@ -3747,7 +4185,9 @@ impl LspManager { for event in deferred_pull_events { let _ = self.event_tx.send(event); } - let has_more = processed >= max_events && !self.event_rx.is_empty(); + let has_more = processed >= max_events + && (!self.event_rx.is_empty() + || (!preserve_pull_observations && !self.ready_startup_events.is_empty())); self.send_due_rust_saves(); let mut promoted_reports = Vec::new(); for (key, client) in &mut self.clients { @@ -3847,6 +4287,15 @@ impl LspManager { config: &Config, timeout: std::time::Duration, ) -> Vec { + // Notification startup used to finish before this relative observation + // timeout began. Preserve that separation when startup is asynchronous; + // callers with an absolute deadline still use wait_for_file_diagnostics. + let lookup_path = normalize_lookup_path(file_path); + self.wait_for_file_startups( + &lookup_path, + config, + Instant::now() + super::client::HANDSHAKE_REQUEST_TIMEOUT + INITIALIZE_EXIT_WAIT, + ); let deadline = std::time::Instant::now() + timeout; self.wait_for_file_diagnostics(file_path, config, deadline) } @@ -3944,7 +4393,7 @@ impl LspManager { #[doc(hidden)] pub fn pending_event_count_for_test(&self) -> usize { - self.event_rx.len() + self.event_rx.len() + self.ready_startup_events.len() } #[doc(hidden)] @@ -4141,7 +4590,11 @@ impl LspManager { continue; } if !self.clients.contains_key(key) { - wait.exited.push(key.clone()); + // A start still in flight has no client yet. It is pending, + // not exited; releasing its reservation wakes this waiter. + if !self.starting.contains_key(key) { + wait.exited.push(key.clone()); + } continue; } if self @@ -4366,7 +4819,23 @@ impl LspManager { ) -> Vec { let lookup_path = normalize_lookup_path(file_path); - if self.server_key_for_file(&lookup_path, config).is_none() { + // A locked notification may have admitted startup without waiting. An + // explicit diagnostics wait must drive both startup and replay phases. + self.wait_for_file_startups(&lookup_path, config, deadline); + + if self.server_key_for_file(&lookup_path, config).is_none() + && !servers_for_file(&lookup_path, config) + .iter() + .any(|definition| { + self.clients.keys().any(|key| { + key.kind == definition.kind + && self + .documents + .get(key) + .is_some_and(|store| store.is_open(&lookup_path)) + }) + }) + { return Vec::new(); } @@ -4453,6 +4922,14 @@ impl LspManager { config: &Config, deadline: Option, ) -> Result, LspError> { + self.ensure_server_for_file_detailed(file_path, config); + self.wait_for_file_startups( + file_path, + config, + deadline.unwrap_or_else(|| { + Instant::now() + super::client::HANDSHAKE_REQUEST_TIMEOUT + INITIALIZE_EXIT_WAIT + }), + ); let pulls = self.begin_file_pulls(file_path, config, deadline)?; Ok(pulls .into_iter() @@ -4544,6 +5021,12 @@ impl LspManager { opened_for_pull: false, state: DocumentPullState::Done(PullFileOutcome::PullNotSupported), }; + if self.starting.contains_key(&pull.key) { + pull.state = DocumentPullState::Replied(Err(LspError::ServerNotReady( + START_IN_PROGRESS_REASON.into(), + ))); + return pull; + } let supports_pull = self .clients .get(&pull.key) @@ -4613,6 +5096,9 @@ impl LspManager { file: &Path, deadline: Instant, ) -> Result { + if self.starting.contains_key(key) { + return Err(LspError::ServerNotReady(START_IN_PROGRESS_REASON.into())); + } let file = canonicalize_for_lsp(file)?; let uri = uri_for_path(&file)?; let version = self @@ -4790,6 +5276,9 @@ impl LspManager { &mut self, server_key: &ServerKey, ) -> Result, LspError> { + if self.starting.contains_key(server_key) { + return Err(LspError::ServerNotReady(START_IN_PROGRESS_REASON.into())); + } let supports_workspace = self .clients .get(server_key) @@ -5098,6 +5587,10 @@ impl LspManager { /// waiting on child processes. The caller owns graceful shutdown so the /// manager lock is not held across a Shutdown handshake. pub fn take_all_clients(&mut self) -> Vec<(ServerKey, LspClient)> { + self.cancel_startups(); + self.starting.clear(); + self.ready_startup_events.clear(); + self.pending_diagnostic_observations.clear(); let clients: Vec<_> = self.clients.drain().collect(); self.clients_generation = self.clients_generation.wrapping_add(1); self.forget_server_state(None); @@ -5112,6 +5605,8 @@ impl LspManager { self.server_binaries.retain(|server, _| keep(server)); self.documents.retain(|server, _| keep(server)); self.watched_file_skip_logged.retain(keep); + self.pending_watched_changes + .retain(|server, _| keep(server)); self.latest_push_for_pull_servers .retain(|(server, _), _| keep(server)); self.latest_pull_for_rust @@ -5144,6 +5639,15 @@ impl LspManager { pub(crate) fn close_start_admission(&self) { self.quiesced .store(true, std::sync::atomic::Ordering::Release); + self.cancel_startups(); + } + + fn cancel_startups(&self) { + for (key, reservation) in &self.starting { + reservation.signal.cancel(); + let pids = self.child_registry.pids_for_server(&key.root, &key.kind); + self.child_registry.force_kill_pids(&pids); + } } pub(crate) fn resume(&self) { @@ -5363,10 +5867,14 @@ impl LspManager { notes } - /// Active server keys (running clients). Used by `lsp_diagnostics` + /// Server keys whose startup synchronization has finished. Used by `lsp_diagnostics` /// directory mode to know which servers to ask for workspace pull. pub fn active_server_keys(&self) -> Vec { - self.clients.keys().cloned().collect() + self.clients + .keys() + .filter(|key| !self.starting.contains_key(*key)) + .cloned() + .collect() } /// Return the last watched-file routing decision for Windows CI timeout @@ -5633,6 +6141,9 @@ impl LspManager { /// diagnostics freshness gate both use this predicate so a complete /// producer set cannot be judged incomplete by a second, stricter check. pub fn producer_has_settled(&self, server: &ServerKey) -> bool { + if self.starting.contains_key(server) { + return false; + } if self .clients .get(server) @@ -5670,7 +6181,11 @@ impl LspManager { } fn drain_events_for_file(&mut self, file_path: &Path) -> bool { - let mut saw_file_diagnostics = false; + let mut saw_file_diagnostics = self.ready_startup_events.iter().any(|delivery| { + delivery.published.as_ref().is_some_and(|published| { + published.file.as_path() == file_path && self.startup_publish_is_current(published) + }) + }); while let Ok(event) = self.event_rx.try_recv() { if matches!( self.handle_event(&event).ok().flatten(), @@ -5682,6 +6197,21 @@ impl LspManager { saw_file_diagnostics } + // Outward delivery may follow another edit or restart. Only the same + // client's still-current document and report can contribute an observation. + fn startup_publish_is_current(&self, published: &StartupPublished) -> bool { + !self.is_quiesced() + && published.pid.is_some() + && self.clients.get(&published.key).map(LspClient::child_pid) == published.pid + && self + .documents + .get(&published.key) + .and_then(|store| store.version(&published.file)) + == published.version + && published.epoch.is_some() + && self.diagnostic_epoch(&published.key, &published.file) == published.epoch + } + /// The snapshot of a publish `handle_event` just stored for `file`, when /// it is an accepted live report. fn accepted_live_publish_snapshot( @@ -5839,12 +6369,17 @@ impl LspManager { accepted } - fn handle_event(&mut self, receipt: &ReceivedLspEvent) -> Result, ()> { - if !self.accept_event_instance(&receipt.event, receipt.instance) { - self.consume_pending_exit_report(receipt); - return Err(()); + /// Reject a receipt from a reader that no longer owns its key, keeping + /// only the exit report that receipt was meant to log. + fn admit_event(&mut self, receipt: &ReceivedLspEvent) -> bool { + if self.accept_event_instance(&receipt.event, receipt.instance) { + return true; } - let event = &receipt.event; + self.consume_pending_exit_report(receipt); + false + } + + fn defer_startup_event(&mut self, receipt: &ReceivedLspEvent) -> bool { let (LspEvent::Notification { server_kind, root, .. } @@ -5856,17 +6391,37 @@ impl LspManager { } | LspEvent::DocumentDiagnosticsPulled { server_kind, root, .. - }) = event; + }) = &receipt.event; let event_key = ServerKey { kind: server_kind.clone(), root: root.clone(), }; - if !self.clients.contains_key(&event_key) { - if let Some(reservation) = self.starting.get_mut(&event_key) { - reservation.deferred_events.push(receipt.clone()); - return Ok(None); - } + // Replay sends run on a worker outside the manager lock. Defer LSP + // events until replay commits document versions, save write times, + // and rust-analyzer check-begin counts. + if let Some(reservation) = self.starting.get_mut(&event_key) { + reservation.deferred_events.push(receipt.clone()); + true + } else { + false + } + } + + /// `Err` means the receipt came from a retired reader and was dropped; + /// `Ok(None)` covers both a deferred startup event and one that stored + /// no publish. + fn handle_event(&mut self, receipt: &ReceivedLspEvent) -> Result, ()> { + if !self.admit_event(receipt) { + return Err(()); + } + if self.defer_startup_event(receipt) { + return Ok(None); } + Ok(self.handle_ready_event(receipt)) + } + + fn handle_ready_event(&mut self, receipt: &ReceivedLspEvent) -> Option { + let event = &receipt.event; let published_file = match event { LspEvent::DocumentDiagnosticsPulled { server_kind, @@ -6010,8 +6565,12 @@ impl LspManager { } _ => None, }; + if matches!(event, LspEvent::ServerRequest { method, .. } if method == "client/registerCapability") + { + self.flush_pending_watched_changes(); + } self.wake_post_edit_waiters(); - Ok(published_file) + published_file } fn wake_post_edit_waiters(&mut self) { @@ -6525,32 +7084,6 @@ impl LspManager { } } - fn spawn_server( - &mut self, - def: &ServerDef, - root: &Path, - source_file: &Path, - config: &Config, - ) -> Result { - self.spawn_server_with_timeout(def, root, source_file, config, None) - } - - fn spawn_server_with_timeout( - &mut self, - def: &ServerDef, - root: &Path, - source_file: &Path, - config: &Config, - initialize_timeout: Option, - ) -> Result { - let prepared = self - .prepare_spawn(def, root, source_file, config) - .map_err(prepare_failure)?; - prepared - .run(initialize_timeout) - .map_err(|failure| self.absorb_spawn_failure(failure)) - } - /// Gather everything a spawn needs from manager state, so the spawn and /// its `initialize` handshake can run without the manager lock. fn prepare_spawn( @@ -6653,6 +7186,11 @@ impl LspManager { .unwrap_or_else(|| root.to_path_buf()); Ok(PreparedSpawn { + start_signal: None, + #[cfg(test)] + spawn_hook: self.spawn_hook.clone(), + #[cfg(test)] + spawned_hook: self.spawned_hook.clone(), instance: ServerInstanceId::next(), rust_worker_policy: self.rust_worker_policy(), kind: def.kind.clone(), @@ -6781,7 +7319,10 @@ impl LspManager { fn server_key_for_file(&self, file_path: &Path, config: &Config) -> Option { for def in servers_for_file(file_path, config) { - let key = server_key_for_definition(&def, file_path, config)?; + // One inapplicable definition must not hide another live server. + let Some(key) = server_key_for_definition(&def, file_path, config) else { + continue; + }; if self.clients.contains_key(&key) { return Some(key); } @@ -6790,6 +7331,12 @@ impl LspManager { } } +impl Drop for LspManager { + fn drop(&mut self) { + self.cancel_startups(); + } +} + impl Default for LspManager { fn default() -> Self { Self::new() @@ -7093,6 +7640,7 @@ fn recoverable_pull_rejection(err: &LspError) -> bool { fn server_attempt_result_reason(result: &ServerAttemptResult) -> String { match result { + ServerAttemptResult::InProgress { .. } => START_IN_PROGRESS_STATUS.into(), ServerAttemptResult::ProjectClosed => "project closed; reopen the project and retry".into(), ServerAttemptResult::SpawnFailed { binary, reason } => { format!("spawn_failed: {binary} ({reason})") @@ -7400,6 +7948,11 @@ fn diagnostic_path_candidates(file: &Path) -> Vec { /// Everything needed to spawn and initialize one language server, gathered /// from `LspManager` state so the slow part can run without the manager lock. struct PreparedSpawn { + start_signal: Option>, + #[cfg(test)] + spawn_hook: Option>, + #[cfg(test)] + spawned_hook: Option>, instance: ServerInstanceId, rust_worker_policy: Arc, kind: ServerKind, @@ -7469,6 +8022,20 @@ impl PreparedSpawn { /// Start the process and run the `initialize` handshake. Touches no /// manager state, so it runs without the manager lock. fn run(mut self, initialize_timeout: Option) -> Result { + #[cfg(test)] + if let Some(hook) = &self.spawn_hook { + hook(); + } + if self + .start_signal + .as_ref() + .is_some_and(|s| s.cancelled.load(std::sync::atomic::Ordering::Acquire)) + { + return Err(SpawnFailure::NotStarted { + error: LspError::ProjectClosed, + exit_log_line: String::new(), + }); + } let initialize_timeout = initialize_timeout.or_else(|| self.test_initialize_timeout()); let worker = self.kind == ServerKind::Rust && self @@ -7535,6 +8102,21 @@ impl PreparedSpawn { }); } }; + #[cfg(test)] + if let Some(hook) = &self.spawned_hook { + hook(client.child_pid()); + } + if self + .start_signal + .as_ref() + .is_some_and(|s| s.cancelled.load(std::sync::atomic::Ordering::Acquire)) + { + drop(client); + return Err(SpawnFailure::NotStarted { + error: LspError::ProjectClosed, + exit_log_line: String::new(), + }); + } client.runtime_note = self.runtime_note; client.completed_rust_check = completed_rust_check; // A worker can arrive while spawn runs without the manager lock. @@ -7652,15 +8234,156 @@ impl PreparedSpawn { } } +struct StartupReplayDocument { + path: PathBuf, + version: i32, + opened: bool, + disk: DiskSnapshot, + /// Save-notification write time and rust-analyzer check-begin count. + saved: Option<(Instant, u64)>, +} + +struct StartupEventDelivery { + event: ReceivedLspEvent, + published: Option, +} + +struct StartupPublished { + key: ServerKey, + file: PathBuf, + version: Option, + epoch: Option, + pid: Option, +} + +struct StartupReplayResult { + pid: u32, + documents: Vec, + error: Option, +} + +/// Read each queued path only when its client is installed. The writer belongs +/// to that exact client. Cancellation prevents later replay sends from starting; +/// the child registry terminates the process for already queued writer jobs. +fn replay_startup_notifications( + writer: super::writer::LspWriter, + save: Option, + begins_read: Arc, + pid: u32, + signal: &StartSignal, + jobs: Vec<(PathBuf, i32, bool)>, +) -> StartupReplayResult { + let mut result = StartupReplayResult { + pid, + documents: Vec::new(), + error: None, + }; + for (path, version, opened) in jobs { + let mut write_attempted = false; + let mut document_written = false; + let sent = (|| -> Result<(DiskSnapshot, Option<(Instant, u64)>), LspError> { + let disk = DiskSnapshot::read(&path); + let content = std::fs::read_to_string(&path)?; + let uri = uri_for_path(&path)?; + #[cfg(test)] + if let Some(hook) = signal.replay_hook.lock().clone() { + hook(); + } + let _send = signal.replay_gate.lock(); + if signal.cancelled.load(std::sync::atomic::Ordering::Acquire) { + return Err(LspError::ProjectClosed); + } + let payload = if opened { + let language = language_id_for_extension( + path.extension() + .and_then(|ext| ext.to_str()) + .unwrap_or_default(), + ); + serde_json::to_string( + &serde_json::json!({"jsonrpc": "2.0", "method": "textDocument/didOpen", "params": {"textDocument": {"uri": uri.as_str(), "languageId": language, "version": version, "text": content}}}), + )? + } else { + super::client::full_did_change_message(&uri, version, &content)? + }; + write_attempted = true; + writer.send(payload)?; + document_written = true; + let saved = if let Some(save) = save { + writer.send(super::client::did_save_message( + &uri, + (save == SaveNotification::IncludeText).then_some(content.as_str()), + )?)?; + Some(( + Instant::now(), + begins_read.load(std::sync::atomic::Ordering::SeqCst), + )) + } else { + None + }; + Ok((disk, saved)) + })(); + match sent { + Ok((disk, saved)) => result.documents.push(StartupReplayDocument { + path, + version, + opened, + disk, + saved, + }), + Err(error) => { + if document_written + || (write_attempted && (!opened || matches!(error, LspError::Timeout(_)))) + { + result.documents.push(StartupReplayDocument { + path, + version, + opened, + disk: DiskSnapshot::default(), + saved: None, + }); + } + result.error = Some(error); + break; + } + } + } + result +} + /// Completion flag for a server start running without the manager lock. A /// second start of the same server waits on it instead of spawning a twin. #[derive(Default)] struct StartSignal { finished: std::sync::Mutex, changed: std::sync::Condvar, + completion: parking_lot::Mutex>>, + outcome: parking_lot::Mutex>, + replay_completion: parking_lot::Mutex>, + replay_gate: parking_lot::Mutex<()>, + cancelled: std::sync::atomic::AtomicBool, + request_deadline_exceeded: std::sync::atomic::AtomicBool, + #[cfg(test)] + replay_hook: parking_lot::Mutex>>, } impl StartSignal { + fn complete(&self, result: Result) { + let mut completion = self.completion.lock(); + if !self.cancelled.load(std::sync::atomic::Ordering::Acquire) { + *completion = Some(result); + } + drop(completion); + self.finish(); + } + + fn cancel(&self) { + let _replay = self.replay_gate.lock(); + self.cancelled + .store(true, std::sync::atomic::Ordering::Release); + *self.outcome.lock() = Some(ServerAttemptResult::ProjectClosed); + self.completion.lock().take(); + self.finish(); + } fn finish(&self) { *self .finished @@ -7694,6 +8417,8 @@ impl StartSignal { /// A server whose start is running without the manager lock. struct StartReservation { + definition: Option, + source_file: Option, instance: ServerInstanceId, /// Events from this server that arrived before its client was published. /// Handling them earlier would find no client for the key, so a @@ -7703,28 +8428,76 @@ struct StartReservation { /// `LspManager::clients_generation` when the start began. clients_generation: u64, signal: Arc, + /// Keep assigned versions until replay ends, including paths already sent, + /// so an edit arriving during replay cannot reset the document version. + notification_versions: HashMap, + /// Paths only: replay reads current disk text rather than retaining edits. + /// New distinct paths are rejected at the cap, never silently evicted. + pending_notifications: HashSet, + /// The installed client instance whose writer is replaying these paths. + replay_pid: Option, +} + +/// Watched changes wait for an initialized client and its subscription without +/// starting a server or changing an already-running client's readiness state. +struct PendingWatchedChanges { + generation: u64, + pid: Option, + changes: HashMap, +} + +struct PendingWatchedChange { + kind: FileChangeType, + raw: Option, + extra_config_markers: Vec, } -/// What an unlocked start does next, decided under the manager lock. -enum StartNext { - /// Another thread is starting this server; wait for it, then look again. - Wait(Arc), - /// This thread reserved the server and spawns it without the lock. - Spawn(PreparedSpawn), +/// Wait through spawn and replay until a final startup outcome or the deadline. +fn wait_for_startup_outcome( + lock: impl Fn() -> G, + signal: &StartSignal, + deadline: Instant, +) -> Option +where + G: std::ops::DerefMut, +{ + loop { + lock().publish_completed_starts(); + if let Some(outcome) = signal.outcome.lock().clone() { + return Some(outcome); + } + if !signal.wait_until(deadline) { + return None; + } + } } -/// Start one inspect producer without holding the manager lock across the -/// spawn and the `initialize` handshake. -/// -/// The handshake can take seconds (rust-analyzer on a large workspace), and -/// while it held the lock every other manager user waited, including the -/// standalone request loop, which drains LSP events and renders the status -/// bar under that lock; a sibling `read` then waited for the handshake. The -/// start reserves the server under the lock, spawns and initializes it -/// unlocked, and publishes the client under the lock. A concurrent start of -/// the same server waits for the reservation instead of spawning a second -/// process. Outcomes match [`LspManager::start_applicable_servers`] for one -/// server under a request deadline. +/// Join existing startups for a workspace diagnostics request without +/// starting servers for files the request has not opened. +pub(crate) fn wait_for_pending_startups_unlocked( + lock: impl Fn() -> G, + scope: Option<&Path>, +) -> bool +where + G: std::ops::DerefMut, +{ + let deadline = Instant::now() + super::client::HANDSHAKE_REQUEST_TIMEOUT + INITIALIZE_EXIT_WAIT; + let signals: Vec<_> = lock() + .starting + .iter() + .filter(|(key, _)| { + scope.is_none_or(|scope| key.root.starts_with(scope) || scope.starts_with(&key.root)) + }) + .map(|(_, reservation)| Arc::clone(&reservation.signal)) + .collect(); + signals + .iter() + .all(|signal| wait_for_startup_outcome(&lock, signal, deadline).is_some()) +} + +/// Start or join one producer's reserved startup without holding the manager +/// mutex across process creation, initialization, or waiting for completion. +/// The request deadline limits waiting, not the worker's initialize handshake. pub fn start_applicable_server_unlocked( manager: &Arc>, snapshot: &ApplicableServerSnapshot, @@ -7736,87 +8509,109 @@ pub fn start_applicable_server_unlocked( failures: snapshot .producer_failures .iter() - .filter(|failure| failure.server_key == *server) + .filter(|f| f.server_key == *server) .cloned() .collect(), ..ApplicableServerStartOutcomes::default() }; - let Some(candidate) = snapshot - .candidates - .iter() - .find(|candidate| candidate.key == *server) - else { + let Some(candidate) = snapshot.candidates.iter().find(|c| c.key == *server) else { return outcomes; }; - - let mut waited = false; - loop { + let mut retried_abandoned = false; + let result = loop { if Instant::now() >= deadline { - outcomes.deadline_exceeded = Some(candidate.key.clone()); + outcomes.deadline_exceeded = Some(server.clone()); return outcomes; } - let next = - manager - .lock() - .begin_unlocked_start(candidate, config, waited, deadline, &mut outcomes); - match next { - None => return outcomes, - Some(StartNext::Wait(signal)) => { - if !signal.wait_until(deadline) { - outcomes.deadline_exceeded = Some(candidate.key.clone()); - return outcomes; - } - waited = true; - } - Some(StartNext::Spawn(prepared)) => { - // The inspect wait budget is not an initialize cancellation. Keep - // the reserved start alive after the caller's deadline, so a - // slow handshake can finish without being killed by AFT. - let (tx, rx) = std::sync::mpsc::sync_channel(1); - let initialize_timeout = prepared.inspect_initialize_timeout(); - let manager = Arc::clone(manager); - let candidate = candidate.clone(); - std::thread::spawn(move || { - let mut reservation = ReservationGuard { - manager: &manager, - key: Some(candidate.key.clone()), - }; - let result = prepared.run(Some(initialize_timeout)); - let mut completed = ApplicableServerStartOutcomes::default(); - manager.lock().finish_unlocked_start( - &candidate, - result, - deadline, - &mut completed, - ); - reservation.key = None; - let _ = tx.send(completed); - }); - match rx.recv_timeout(deadline.saturating_duration_since(Instant::now())) { - Ok(completed) => { - outcomes.successful.extend(completed.successful); - outcomes.failures.extend(completed.failures); - outcomes.deadline_exceeded = completed.deadline_exceeded; - } - Err(_) => outcomes.deadline_exceeded = Some(server.clone()), - } - return outcomes; + let (initial, signal) = { + let mut lsp = manager.lock(); + lsp.startup_owner = Arc::downgrade(manager); + let result = lsp.start_server( + &candidate.definition, + server, + &candidate.source_file, + config, + ); + let signal = lsp.starting.get(server).map(|r| Arc::clone(&r.signal)); + (result, signal) + }; + let abandoned_before_join = signal.as_ref().is_some_and(|signal| { + signal + .request_deadline_exceeded + .load(std::sync::atomic::Ordering::Acquire) + }); + let result = if let (ServerAttemptResult::InProgress { .. }, Some(signal)) = + (&initial, signal) + { + wait_for_startup_outcome(|| manager.lock(), &signal, deadline).unwrap_or_else(|| { + signal + .request_deadline_exceeded + .store(true, std::sync::atomic::Ordering::Release); + initial + }) + } else { + initial + }; + // A new request can overlap cleanup of an earlier timed-out request. + // Observe its result, then retry once with the new request's budget and + // configuration instead of returning that abandoned attempt's failure. + if abandoned_before_join + && !retried_abandoned + && matches!( + result, + ServerAttemptResult::SpawnFailed { .. } + | ServerAttemptResult::BinaryNotInstalled { .. } + ) + && Instant::now() < deadline + { + let lsp = manager.lock(); + if !lsp.is_quiesced() && lsp.failure_to_replay(server).is_none() { + retried_abandoned = true; + continue; } } + break result; + }; + match result { + ServerAttemptResult::Ok { server_key } => outcomes.successful.push(server_key), + ServerAttemptResult::InProgress { server_key } => { + outcomes.in_progress.push(server_key.clone()); + outcomes.deadline_exceeded = Some(server_key); + } + result => outcomes.failures.push(ApplicableServerFailure { + server_key: server.clone(), + result, + }), } + outcomes } -/// Releases a reservation whose start unwound before publishing, so waiters -/// are not left blocked on a start that will never finish. -struct ReservationGuard<'a> { - manager: &'a parking_lot::Mutex, - key: Option, +/// Wake waiters if a startup worker unwinds before publishing a result. +struct StartupWorkerGuard { + signal: Arc, + armed: bool, } -impl Drop for ReservationGuard<'_> { +impl StartupWorkerGuard { + fn new(signal: Arc) -> Self { + Self { + signal, + armed: true, + } + } + + fn publish(mut self, publish: impl FnOnce(&StartSignal)) { + // Waking a waiter can advance this signal to the next replay phase. + // Disarm before publication instead of inferring ownership from that + // phase's shared completion flag when this worker is dropped. + self.armed = false; + publish(&self.signal); + } +} +impl Drop for StartupWorkerGuard { fn drop(&mut self) { - if let Some(key) = self.key.take() { - self.manager.lock().release_start_reservation(&key); + if self.armed { + self.signal.cancel(); } } } @@ -7829,47 +8624,13 @@ enum FileServerStart { /// The server cannot start; report this. Failed(ServerAttemptResult), /// Another thread is starting this server; wait for it, then look again. - Wait(Arc), + Wait, /// This thread reserved the server and starts it without the lock. Spawn(Box), } -/// How long one wait for another thread's start of the same server lasts -/// before the waiter looks at the manager again. A start ends on its own -/// within the `initialize` budget; this only bounds a wait whose signal was -/// lost to a start that unwound. -const FILE_START_WAIT_SLICE: Duration = Duration::from_secs(5); - -/// Releases a reservation taken by [`ensure_server_for_file_detailed_unlocked`] -/// if its start unwinds before publishing, so waiters are not left blocked. -struct FileStartGuard<'a, L, G> -where - L: Fn() -> G, - G: std::ops::DerefMut, -{ - lock: &'a L, - key: Option, -} - -impl Drop for FileStartGuard<'_, L, G> -where - L: Fn() -> G, - G: std::ops::DerefMut, -{ - fn drop(&mut self) { - if let Some(key) = self.key.take() { - (self.lock)().release_start_reservation(&key); - } - } -} - -/// [`LspManager::ensure_server_for_file_detailed`] with the server spawn and -/// its `initialize` handshake (up to 30 seconds) run without the manager -/// lock. Each server to start is reserved under the lock; a concurrent caller -/// for the same server waits on that one reservation rather than on the -/// whole manager, and callers for other servers or for no server at all are -/// not held up. Outcomes, failure caching and the retry backoff match the -/// locked method. `lock` acquires the manager (for example `|| ctx.lsp()`). +/// Start or join matching servers, releasing the manager mutex while waiting +/// for the shared startup completion. Cold locked callers use the same worker. pub fn ensure_server_for_file_detailed_unlocked( lock: impl Fn() -> G, file_path: &Path, @@ -7880,54 +8641,38 @@ where { let mut outcomes = EnsureServerOutcomes::default(); for def in servers_for_file(file_path, config) { - let server_id = def.kind.id_str().to_string(); - let server_name = def.name.to_string(); - let Some(key) = server_key_for_definition(&def, file_path, config) else { - outcomes.attempts.push(ServerAttempt { - server_id, - server_name, - result: ServerAttemptResult::NoRootMarker { - looked_for: def.root_markers.iter().map(|s| s.to_string()).collect(), - }, - }); - continue; - }; - let failure = loop { - let next = lock().begin_file_server_start(&def, &key, file_path, config); - match next { - FileServerStart::Running => break None, - FileServerStart::Failed(result) => break Some(result), - FileServerStart::Wait(signal) => { - signal.wait_until(Instant::now() + FILE_START_WAIT_SLICE); - } - FileServerStart::Spawn(prepared) => { - let mut guard = FileStartGuard { - lock: &lock, - key: Some(key.clone()), - }; - let result = prepared.run(None); - guard.key = None; - break lock().finish_file_server_start(&def, &key, result); - } + let result = if let Some(key) = server_key_for_definition(&def, file_path, config) { + let (initial, signal) = { + let mut manager = lock(); + let result = manager.start_server(&def, &key, file_path, config); + let signal = manager.starting.get(&key).map(|r| Arc::clone(&r.signal)); + (result, signal) + }; + if let (ServerAttemptResult::InProgress { .. }, Some(signal)) = (&initial, signal) { + wait_for_startup_outcome( + &lock, + &signal, + Instant::now() + + super::client::HANDSHAKE_REQUEST_TIMEOUT + + INITIALIZE_EXIT_WAIT, + ) + .unwrap_or(initial) + } else { + initial } - }; - match failure { - Some(result) => outcomes.attempts.push(ServerAttempt { - server_id, - server_name, - result, - }), - None => { - outcomes.attempts.push(ServerAttempt { - server_id, - server_name, - result: ServerAttemptResult::Ok { - server_key: key.clone(), - }, - }); - outcomes.successful.push(key); + } else { + ServerAttemptResult::NoRootMarker { + looked_for: def.root_markers.iter().map(|s| s.to_string()).collect(), } + }; + if let ServerAttemptResult::Ok { server_key } = &result { + outcomes.successful.push(server_key.clone()); } + outcomes.attempts.push(ServerAttempt { + server_id: def.kind.id_str().to_string(), + server_name: def.name.to_string(), + result, + }); } outcomes } @@ -7970,7 +8715,14 @@ where G: std::ops::DerefMut, { let canonical_path = canonicalize_for_lsp(file_path)?; - ensure_server_for_file_detailed_unlocked(&lock, &canonical_path, config); + let outcomes = ensure_server_for_file_detailed_unlocked(&lock, &canonical_path, config); + if outcomes + .attempts + .iter() + .any(|attempt| matches!(attempt.result, ServerAttemptResult::InProgress { .. })) + { + return Err(LspError::ServerNotReady(START_IN_PROGRESS_REASON.into())); + } // Every server is now running or has a remembered failure, so this // starts nothing under the lock unless a client vanished in between. lock().ensure_file_open(&canonical_path, config) @@ -8618,7 +9370,7 @@ fn delay_applicability_walk_for_test() { } } -/// Classify an error returned by `spawn_server` into a structured +/// Classify a startup error into a structured /// `ServerAttemptResult`. The two interesting cases for callers are: /// - `BinaryNotInstalled` — the server's binary couldn't be resolved on PATH /// or via override. The agent can be told "install bash-language-server". @@ -8727,6 +9479,10 @@ fn env_binary_override_from( #[path = "manager_regression_tests.rs"] mod regression_tests; +#[cfg(all(test, unix))] +#[path = "startup_regression_tests.rs"] +mod startup_regression_tests; + #[cfg(all(test, unix))] #[path = "instance_regression_tests.rs"] pub(crate) mod instance_regression_tests; @@ -9456,6 +10212,11 @@ mod unlocked_start_tests { manager.starting.insert( key.clone(), StartReservation { + definition: None, + source_file: None, + notification_versions: Default::default(), + pending_notifications: Default::default(), + replay_pid: None, instance: super::ServerInstanceId::next(), deferred_events: Vec::new(), clients_generation: manager.clients_generation, @@ -10064,7 +10825,9 @@ mod server_exit_reap_tests { 2, "both orphans must be registered under the spawn key" ); - let _ = manager.ensure_server_for_file(&main_rs, &config); + let _ = crate::lsp::manager::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file(&main_rs, &config) + }); let deadline = Instant::now() + Duration::from_secs(5); loop { @@ -10270,7 +11033,9 @@ exit 1 let snapshot = manager .resolve_applicable_servers_for_root(&root, &config) .unwrap(); - let outcomes = manager.start_applicable_servers(&snapshot, &config); + let outcomes = crate::lsp::manager::settled_lsp(&mut manager, |startup_manager| { + startup_manager.start_applicable_servers(&snapshot, &config) + }); let root_failure = outcomes .failures .iter() @@ -10710,3 +11475,48 @@ mod document_pull_certificate_tests { assert_eq!(entry.diagnostics[0].message, "checked error"); } } + +#[cfg(test)] +pub(crate) fn settled_lsp( + manager: &mut LspManager, + operation: impl Fn(&mut LspManager) -> R, +) -> R { + let initial = operation(manager); + if !manager.has_pending_startups() { + return initial; + } + assert!(manager.wait_for_startups(Instant::now() + Duration::from_secs(30))); + operation(manager) +} + +#[cfg(test)] +mod startup_worker_guard_tests { + use super::*; + + #[test] + fn published_worker_cannot_cancel_the_next_replay_phase() { + let signal = Arc::new(StartSignal::default()); + let guard = StartupWorkerGuard::new(Arc::clone(&signal)); + guard.publish(|signal| { + signal.finish(); + // A woken manager can consume completion and start another batch + // before the publishing worker's scope ends. + *signal.finished.lock().unwrap() = false; + }); + assert!(!signal.cancelled.load(std::sync::atomic::Ordering::Acquire)); + assert!(signal.outcome.lock().is_none()); + assert!(!signal.wait_until(Instant::now())); + } + + #[test] + fn unpublished_worker_releases_its_reservation_on_drop() { + let signal = Arc::new(StartSignal::default()); + drop(StartupWorkerGuard::new(Arc::clone(&signal))); + assert!(signal.cancelled.load(std::sync::atomic::Ordering::Acquire)); + assert!(signal.wait_until(Instant::now())); + assert!(matches!( + *signal.outcome.lock(), + Some(ServerAttemptResult::ProjectClosed) + )); + } +} diff --git a/crates/aft/src/lsp/startup_regression_tests.rs b/crates/aft/src/lsp/startup_regression_tests.rs new file mode 100644 index 0000000000..da4c12f8fd --- /dev/null +++ b/crates/aft/src/lsp/startup_regression_tests.rs @@ -0,0 +1,867 @@ +use super::*; +use crate::config::UserServerDef; +use std::os::unix::fs::PermissionsExt; +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; + +const WAIT: Duration = Duration::from_secs(30); + +thread_local! { + static MANAGER_LOCK_HELD: std::cell::Cell = const { std::cell::Cell::new(false) }; +} + +fn locked(f: &Fixture, operation: impl FnOnce(&mut LspManager) -> R) -> R { + let mut manager = f.manager.lock(); + MANAGER_LOCK_HELD.set(true); + let result = operation(&mut manager); + MANAGER_LOCK_HELD.set(false); + result +} + +struct Fixture { + _root: tempfile::TempDir, + config: Config, + file: PathBuf, + manager: Arc>, + snapshot: ApplicableServerSnapshot, + key: ServerKey, +} + +fn fixture() -> Fixture { + let root = tempfile::tempdir().unwrap(); + std::fs::write(root.path().join("marker"), "marker").unwrap(); + let file = root.path().join("source.reserved"); + std::fs::write(&file, "content").unwrap(); + let mut frame = Vec::new(); + super::super::transport::write_message( + &mut frame, + r#"{"jsonrpc":"2.0","id":1,"result":{"capabilities":{}}}"#, + ) + .unwrap(); + let script = root.path().join("server.sh"); + std::fs::write( + &script, + format!( + "#!/bin/sh\nIFS= read -r header\nprintf '%s' '{}'\nwhile IFS= read -r line; do :; done\n", + String::from_utf8(frame).unwrap(), + ), + ) + .unwrap(); + std::fs::set_permissions(&script, std::fs::Permissions::from_mode(0o755)).unwrap(); + let config = Config { + project_root: Some(root.path().to_path_buf()), + lsp_servers: vec![UserServerDef { + id: "reserved".into(), + extensions: vec!["reserved".into()], + binary: "reserved".into(), + args: Vec::new(), + root_markers: vec!["marker".into()], + env: HashMap::new(), + initialization_options: None, + disabled: false, + }], + ..Config::default() + }; + let mut manager = LspManager::new(); + manager.override_binary(ServerKind::Custom(Arc::from("reserved")), script); + let definition = servers_for_file(&file, &config).remove(0); + let key = server_key_for_definition(&definition, &file, &config).unwrap(); + let snapshot = ApplicableServerSnapshot { + server_keys: vec![key.clone()], + candidates: vec![ApplicableServerCandidate { + key: key.clone(), + definition, + source_file: file.clone(), + }], + ..ApplicableServerSnapshot::default() + }; + let manager = Arc::new(parking_lot::Mutex::new(manager)); + manager.lock().startup_owner = Arc::downgrade(&manager); + Fixture { + _root: root, + config, + file, + manager, + snapshot, + key, + } +} + +struct Pause { + entered: Receiver<()>, + resume: Sender<()>, + spawns: Arc, + under_lock: Arc, +} + +fn pause_first_spawn(f: &Fixture) -> Pause { + let (entered_tx, entered) = unbounded(); + let (resume, resume_rx) = unbounded(); + let spawns = Arc::new(AtomicUsize::new(0)); + let under_lock = Arc::new(AtomicBool::new(false)); + let count = Arc::clone(&spawns); + let locked = Arc::clone(&under_lock); + f.manager.lock().spawn_hook = Some(Arc::new(move || { + // Only a guard held by the spawning thread violates the lock boundary; + // another caller is allowed to inspect the manager during startup. + if MANAGER_LOCK_HELD.get() { + locked.store(true, Ordering::SeqCst); + } + if count.fetch_add(1, Ordering::SeqCst) == 0 { + entered_tx.send(()).unwrap(); + resume_rx.recv_timeout(WAIT).unwrap(); + } + })); + Pause { + entered, + resume, + spawns, + under_lock, + } +} + +#[test] +fn locked_and_unlocked_starts_share_one_reservation() { + let f = fixture(); + let pause = pause_first_spawn(&f); + std::thread::scope(|scope| { + let running = scope.spawn(|| { + ensure_server_for_file_detailed_unlocked(|| f.manager.lock(), &f.file, &f.config) + }); + pause.entered.recv_timeout(WAIT).unwrap(); + let file = locked(&f, |manager| { + manager.ensure_server_for_file_detailed(&f.file, &f.config) + }); + let area = locked(&f, |manager| { + manager.start_applicable_servers(&f.snapshot, &f.config) + }); + pause.resume.send(()).unwrap(); + let completed = running.join().unwrap(); + assert_eq!(pause.spawns.load(Ordering::SeqCst), 1, "duplicate spawn"); + assert_eq!( + server_attempt_result_reason(&file.attempts[0].result), + "start_in_progress" + ); + assert!(file.successful.is_empty()); + assert!(area.successful.is_empty()); + assert!(area.failures.is_empty()); + assert_eq!(completed.successful, vec![f.key.clone()]); + assert!( + !pause.under_lock.load(Ordering::SeqCst), + "spawn held manager lock" + ); + }); +} + +#[test] +fn locked_cold_start_never_spawns_under_manager_lock() { + for area in [false, true] { + let f = fixture(); + let pause = pause_first_spawn(&f); + pause.resume.send(()).unwrap(); + if area { + locked(&f, |manager| { + manager.start_applicable_servers(&f.snapshot, &f.config) + }); + } else { + locked(&f, |manager| { + manager.ensure_server_for_file_detailed(&f.file, &f.config) + }); + } + pause.entered.recv_timeout(WAIT).unwrap(); + assert!( + !pause.under_lock.load(Ordering::SeqCst), + "spawn held manager lock" + ); + let completed = + ensure_server_for_file_detailed_unlocked(|| f.manager.lock(), &f.file, &f.config); + assert_eq!(completed.successful, vec![f.key.clone()]); + } +} + +#[test] +fn waiter_sees_pending_then_the_real_result() { + let f = fixture(); + let pause = pause_first_spawn(&f); + let initial = locked(&f, |manager| { + manager.ensure_server_for_file_detailed(&f.file, &f.config) + }); + pause.entered.recv_timeout(WAIT).unwrap(); + let pending = locked(&f, |manager| { + manager.ensure_server_for_file_detailed(&f.file, &f.config) + }); + assert!( + matches!(&initial.attempts[0].result, ServerAttemptResult::InProgress { server_key } if server_key == &f.key) + ); + assert_eq!(START_IN_PROGRESS_STATUS, "start_in_progress"); + assert_eq!( + START_IN_PROGRESS_REASON, + "server startup is in progress; retry shortly" + ); + assert_eq!( + pending.attempts[0].result.failure_reason(), + START_IN_PROGRESS_REASON + ); + let response = + crate::commands::lsp_diagnostics::startup_unavailable_response(&f.file, &pending); + assert_eq!(response["complete"], false); + assert_eq!(response["pending_servers"], serde_json::json!(["reserved"])); + assert_eq!( + response["unchecked_files"], + serde_json::json!([f.file.display().to_string()]) + ); + assert_eq!( + response["lsp_servers_used"][0]["status"], + "start_in_progress" + ); + let area = locked(&f, |manager| { + manager.start_applicable_servers(&f.snapshot, &f.config) + }); + assert_eq!(area.in_progress, vec![f.key.clone()]); + assert!(area.successful.is_empty()); + assert!(!f.manager.lock().failed_spawns.contains_key(&f.key)); + assert!(!f.manager.lock().transient_backoff.contains_key(&f.key)); + pause.resume.send(()).unwrap(); + let ready = ensure_server_for_file_detailed_unlocked(|| f.manager.lock(), &f.file, &f.config); + assert_eq!(ready.successful, vec![f.key.clone()]); + assert_eq!( + server_attempt_result_reason(&ready.attempts[0].result), + "ok" + ); + assert_eq!( + ServerAttemptResult::BinaryNotInstalled { + binary: "example-ls".into() + } + .failure_reason(), + "example-ls is unavailable" + ); + assert_eq!(pause.spawns.load(Ordering::SeqCst), 1); +} + +#[test] +fn failed_spawn_releases_reservation_for_retry_and_all_waiters() { + let f = fixture(); + let binary = f.manager.lock().binary_overrides[&f.key.kind].clone(); + let original = std::fs::read(&binary).unwrap(); + let clock_offset = Arc::new(std::sync::atomic::AtomicU64::new(0)); + let clock = Arc::clone(&clock_offset); + let now = Instant::now(); + f.manager + .lock() + .set_retry_clock(RetryClock::from_fn(move || { + now + Duration::from_secs(clock.load(Ordering::SeqCst)) + })); + let pause = pause_first_spawn(&f); + locked(&f, |manager| { + manager.ensure_server_for_file_detailed(&f.file, &f.config) + }); + pause.entered.recv_timeout(WAIT).unwrap(); + // An executable image error occurs after binary resolution and is transient. + std::fs::write(&binary, "invalid executable image\n").unwrap(); + std::thread::scope(|scope| { + let one = scope.spawn(|| { + ensure_server_for_file_detailed_unlocked(|| f.manager.lock(), &f.file, &f.config) + }); + let two = scope.spawn(|| { + ensure_server_for_file_detailed_unlocked(|| f.manager.lock(), &f.file, &f.config) + }); + let mut manager = f.manager.lock(); + let signal = Arc::clone(&manager.starting[&f.key].signal); + pause.resume.send(()).unwrap(); + assert!(signal.wait_until(Instant::now() + WAIT)); + manager.publish_completed_starts(); + assert!(manager.starting.is_empty()); + drop(manager); + let one = one.join().unwrap(); + let two = two.join().unwrap(); + assert!(matches!( + one.attempts[0].result, + ServerAttemptResult::SpawnFailed { .. } + )); + assert_eq!( + server_attempt_result_reason(&one.attempts[0].result), + server_attempt_result_reason(&two.attempts[0].result) + ); + }); + assert!(f.manager.lock().starting.is_empty()); + assert_eq!(pause.spawns.load(Ordering::SeqCst), 1); + std::fs::write(&binary, original).unwrap(); + clock_offset.store(3600, Ordering::SeqCst); + let retry = ensure_server_for_file_detailed_unlocked(|| f.manager.lock(), &f.file, &f.config); + assert_eq!(retry.successful, vec![f.key.clone()]); + assert_eq!(pause.spawns.load(Ordering::SeqCst), 2); +} + +#[test] +fn closing_project_during_startup_reaps_child_and_never_installs_it() { + for shutdown in [false, true] { + let f = fixture(); + let (spawned_tx, spawned_rx) = unbounded(); + let (resume_tx, resume_rx) = unbounded(); + f.manager.lock().spawned_hook = Some(Arc::new(move |pid| { + spawned_tx.send(pid).unwrap(); + resume_rx.recv_timeout(WAIT).unwrap(); + })); + locked(&f, |manager| { + manager.ensure_server_for_file_detailed(&f.file, &f.config) + }); + let pid = spawned_rx.recv_timeout(WAIT).unwrap(); + let signal = Arc::clone(&f.manager.lock().starting[&f.key].signal); + if shutdown { + f.manager.lock().shutdown_all(); + } else { + f.manager.lock().quiesce(); + } + resume_tx.send(()).unwrap(); + assert!(signal.cancelled.load(Ordering::SeqCst)); + assert!(signal.wait_until(Instant::now() + WAIT)); + // waitpid blocks in the kernel, not a timing loop; the client's reaper + // may already have consumed the exit status, which also proves exit. + let waited = unsafe { libc::waitpid(pid as i32, std::ptr::null_mut(), 0) }; + assert!( + waited == pid as i32 + || (waited == -1 + && std::io::Error::last_os_error().raw_os_error() == Some(libc::ECHILD)) + ); + assert_eq!(unsafe { libc::kill(pid as i32, 0) }, -1); + assert_eq!( + std::io::Error::last_os_error().raw_os_error(), + Some(libc::ESRCH) + ); + f.manager.lock().publish_completed_starts(); + assert_eq!(f.manager.lock().server_count(), 0); + assert!(f.manager.lock().child_registry.pids().is_empty()); + if !shutdown { + let result = locked(&f, |manager| { + manager.ensure_server_for_file_detailed(&f.file, &f.config) + }); + assert!(matches!( + result.attempts[0].result, + ServerAttemptResult::ProjectClosed + )); + } + } +} + +#[test] +fn cancelled_start_cannot_publish_over_a_new_reservation() { + let f = fixture(); + let (finished_tx, finished_rx) = unbounded(); + f.manager.lock().startup_finished_hook = Some(Arc::new(move || { + finished_tx.send(()).unwrap(); + })); + let pause = pause_first_spawn(&f); + let spawned = Arc::new(AtomicUsize::new(0)); + let count = Arc::clone(&spawned); + f.manager.lock().spawned_hook = Some(Arc::new(move |_| { + count.fetch_add(1, Ordering::SeqCst); + })); + locked(&f, |manager| { + manager.ensure_server_for_file_detailed(&f.file, &f.config) + }); + pause.entered.recv_timeout(WAIT).unwrap(); + let old = Arc::clone(&f.manager.lock().starting[&f.key].signal); + f.manager.lock().quiesce(); + assert!(f.manager.lock().starting.is_empty()); + f.manager.lock().resume(); + let ready = ensure_server_for_file_detailed_unlocked(|| f.manager.lock(), &f.file, &f.config); + assert_eq!(ready.successful, vec![f.key.clone()]); + let pid = f.manager.lock().clients[&f.key].child_pid(); + finished_rx.recv_timeout(WAIT).unwrap(); + pause.resume.send(()).unwrap(); + finished_rx.recv_timeout(WAIT).unwrap(); + assert!(old.cancelled.load(Ordering::SeqCst)); + assert!(matches!( + *old.outcome.lock(), + Some(ServerAttemptResult::ProjectClosed) + )); + f.manager.lock().publish_completed_starts(); + assert_eq!(f.manager.lock().clients[&f.key].child_pid(), pid); + assert_eq!(spawned.load(Ordering::SeqCst), 1); +} + +#[test] +fn in_progress_is_not_a_cached_executable_failure() { + let f = fixture(); + let result = f.manager.lock().record_failed_spawn( + &f.key, + ServerAttemptResult::InProgress { + server_key: f.key.clone(), + }, + FailureDurability::Transient, + ); + assert!(matches!(result, ServerAttemptResult::InProgress { .. })); + assert!(f.manager.lock().failed_spawns.is_empty()); + assert!(f.manager.lock().transient_backoff.is_empty()); +} + +fn notification_fixture() -> Fixture { + let f = fixture(); + let binary = f.manager.lock().binary_overrides[&f.key.kind].clone(); + std::fs::write(&binary, r#"#!/usr/bin/env python3 +import json, os, sys +seen = [] +def write_message(message): + body = json.dumps(message).encode() + sys.stdout.buffer.write(('Content-Length: %d\r\n\r\n' % len(body)).encode() + body) + sys.stdout.buffer.flush() +def register_watched_files(): + write_message({'jsonrpc': '2.0', 'id': 'startup-watches', 'method': 'client/registerCapability', 'params': { + 'registrations': [{'id': 'startup-watches', 'method': 'workspace/didChangeWatchedFiles', + 'registerOptions': {'watchers': [{'globPattern': '**/*'}]}}]}}) +while True: + headers = {} + while True: + line = sys.stdin.buffer.readline() + if not line: + sys.exit(0) + if line in (b'\r\n', b'\n'): + break + key, value = line.decode().split(':', 1) + headers[key.lower()] = value.strip() + message = json.loads(sys.stdin.buffer.read(int(headers['content-length']))) + method = message.get('method') + if method == 'initialized' and os.environ.get('AFT_STARTUP_WATCHES') == '1': + register_watched_files() + if method == 'test/notifications' and message.get('params', {}).get('register_watches'): + register_watched_files() + if method in ('textDocument/didOpen', 'textDocument/didChange', 'workspace/didChangeWatchedFiles'): + seen.append(message) + if method != 'workspace/didChangeWatchedFiles' and os.environ.get('AFT_STARTUP_PUSH') == '1': + document = message['params']['textDocument'] + write_message({'jsonrpc': '2.0', 'method': 'textDocument/publishDiagnostics', 'params': { + 'uri': document['uri'], 'version': document['version'], 'diagnostics': [{ + 'range': {'start': {'line': 0, 'character': 0}, 'end': {'line': 0, 'character': 1}}, + 'severity': 1, 'message': 'startup replay diagnostic'}]}}) + if 'id' in message and method is not None: + result = {'capabilities': {}} if method == 'initialize' else seen if method == 'test/notifications' else None + write_message({'jsonrpc': '2.0', 'id': message['id'], 'result': result}) + if method == 'exit': + break +"#).unwrap(); + f +} + +enum RecordedNotifications {} +impl lsp_types::request::Request for RecordedNotifications { + type Params = serde_json::Value; + type Result = Vec; + const METHOD: &'static str = "test/notifications"; +} + +fn recorded_notifications(f: &Fixture) -> Vec { + recorded_notifications_with(f, serde_json::json!({})) +} + +fn recorded_notifications_with(f: &Fixture, params: serde_json::Value) -> Vec { + let request = f + .manager + .lock() + .clients + .get_mut(&f.key) + .unwrap() + .start_request::(params) + .unwrap(); + serde_json::from_value(request.wait(WAIT).unwrap()).unwrap() +} + +#[test] +fn file_change_during_startup_replays_latest_did_open() { + for changes in [1, 2] { + let f = notification_fixture(); + let pause = pause_first_spawn(&f); + locked(&f, |manager| { + manager.ensure_server_for_file_detailed(&f.file, &f.config) + }); + pause.entered.recv_timeout(WAIT).unwrap(); + let mut expected = Vec::new(); + for version in 0..changes { + let content = format!("content version {version}"); + std::fs::write(&f.file, &content).unwrap(); + expected = locked(&f, |manager| { + manager.notify_file_changed_versioned(&f.file, &content, &f.config) + }) + .unwrap(); + } + pause.resume.send(()).unwrap(); + let ready = + ensure_server_for_file_detailed_unlocked(|| f.manager.lock(), &f.file, &f.config); + assert_eq!(ready.successful, vec![f.key.clone()]); + let messages = recorded_notifications(&f); + assert_eq!( + messages.len(), + 1, + "startup changes must coalesce into one didOpen: {messages:?}" + ); + assert_eq!(messages[0]["method"], "textDocument/didOpen"); + assert_eq!( + messages[0]["params"]["textDocument"]["text"], + format!("content version {}", changes - 1) + ); + assert_eq!( + messages[0]["params"]["textDocument"]["version"], + changes - 1 + ); + assert_eq!(expected, vec![(f.key.clone(), changes - 1)]); + } +} + +#[test] +fn file_change_during_cancelled_startup_is_not_replayed() { + let f = notification_fixture(); + let pause = pause_first_spawn(&f); + let (done_tx, done_rx) = unbounded(); + f.manager.lock().startup_finished_hook = Some(Arc::new(move || { + done_tx.send(()).unwrap(); + })); + locked(&f, |manager| { + manager.ensure_server_for_file_detailed(&f.file, &f.config) + }); + pause.entered.recv_timeout(WAIT).unwrap(); + let queued = locked(&f, |manager| { + manager.notify_file_changed_versioned(&f.file, "never replay", &f.config) + }) + .unwrap(); + assert_eq!(queued, vec![(f.key.clone(), 0)]); + assert!(f.manager.lock().starting[&f.key] + .pending_notifications + .contains(&canonicalize_for_lsp(&f.file).unwrap())); + f.manager.lock().quiesce(); + pause.resume.send(()).unwrap(); + done_rx.recv_timeout(WAIT).unwrap(); + f.manager.lock().resume(); + f.manager.lock().spawn_hook = None; + f.manager.lock().startup_finished_hook = None; + let ready = ensure_server_for_file_detailed_unlocked(|| f.manager.lock(), &f.file, &f.config); + assert_eq!(ready.successful, vec![f.key.clone()]); + assert!( + recorded_notifications(&f).is_empty(), + "closed reservation leaked a notification into its replacement" + ); +} + +#[test] +fn unlocked_request_waits_until_startup_notification_replay_finishes() { + let f = notification_fixture(); + let pause = pause_first_spawn(&f); + locked(&f, |manager| { + manager.ensure_server_for_file_detailed(&f.file, &f.config) + }); + pause.entered.recv_timeout(WAIT).unwrap(); + std::fs::write(&f.file, "latest disk content").unwrap(); + locked(&f, |manager| { + manager.notify_file_changed_versioned(&f.file, "not retained", &f.config) + }) + .unwrap(); + let (entered_tx, entered_rx) = unbounded(); + let (resume_tx, resume_rx) = unbounded(); + let signal = Arc::clone(&f.manager.lock().starting[&f.key].signal); + *signal.replay_hook.lock() = Some(Arc::new(move || { + assert!(!MANAGER_LOCK_HELD.get()); + entered_tx.send(()).unwrap(); + resume_rx.recv_timeout(WAIT).unwrap(); + })); + let (done_tx, done_rx) = unbounded(); + std::thread::scope(|scope| { + let waiting = scope.spawn(|| { + let result = + ensure_server_for_file_detailed_unlocked(|| f.manager.lock(), &f.file, &f.config); + done_tx.send(()).unwrap(); + result + }); + let existing = scope.spawn(|| { + let joined = wait_for_pending_startups_unlocked(|| f.manager.lock(), Some(&f.key.root)); + done_tx.send(()).unwrap(); + joined + }); + pause.resume.send(()).unwrap(); + entered_rx.recv_timeout(WAIT).unwrap(); + let mut lock_available = false; + let mut event_deferred = false; + let mut readiness_blocked = false; + if let Some(mut manager) = f.manager.try_lock_for(Duration::from_secs(5)) { + lock_available = true; + let client_hidden = manager.client_for_file_mut(&f.file, &f.config).is_none(); + let uri = uri_for_path(&canonicalize_for_lsp(&f.file).unwrap()).unwrap(); + let pull = manager.begin_open_document_pull(f.key.clone(), &f.file, &uri, None); + readiness_blocked = client_hidden + && manager.has_client(&f.key) + && manager.active_server_keys().is_empty() + && !manager.producer_has_settled(&f.key) + && manager.navigation_requires_deferred_execution(&f.file, &f.config) + && matches!( + pull.state, + DocumentPullState::Replied(Err(LspError::ServerNotReady(_))) + ) + && manager.begin_workspace_pull(&f.key).is_err(); + let before = manager.starting[&f.key].deferred_events.len(); + let event = LspEvent::Notification { + server_kind: f.key.kind.clone(), + root: f.key.root.clone(), + method: "custom/producerProgress".into(), + params: None, + }; + manager.event_tx.send(event.into()).unwrap(); + let drained = manager.drain_events_bounded(1); + event_deferred = manager.clients.contains_key(&f.key) + && manager.starting[&f.key].deferred_events.len() == before + 1 + && drained.events.is_empty() + && drained.processed == 1; + } + let returned_early = done_rx.recv_timeout(Duration::from_secs(5)).is_ok(); + resume_tx.send(()).unwrap(); + assert_eq!(waiting.join().unwrap().successful, vec![f.key.clone()]); + assert!(existing.join().unwrap()); + assert!(lock_available, "replay held the manager mutex"); + assert!( + readiness_blocked, + "uncommitted replay versions were exposed to requests" + ); + assert!( + event_deferred, + "producer event passed uncommitted replay state" + ); + assert!(!returned_early, "request returned before replay completed"); + }); + let delivered = f.manager.lock().drain_events_bounded(1); + assert_eq!(delivered.processed, 1); + assert!( + matches!(&delivered.events[..], [LspEvent::Notification { method, .. }] if method == "custom/producerProgress") + ); + assert!(f.manager.lock().drain_events().events.is_empty()); + let messages = recorded_notifications(&f); + assert_eq!( + messages[0]["params"]["textDocument"]["text"], + "latest disk content" + ); +} + +#[test] +fn startup_notification_path_bound_rejects_instead_of_dropping() { + let f = notification_fixture(); + let pause = pause_first_spawn(&f); + locked(&f, |manager| { + manager.ensure_server_for_file_detailed(&f.file, &f.config) + }); + pause.entered.recv_timeout(WAIT).unwrap(); + { + let mut manager = f.manager.lock(); + let reservation = manager.starting.get_mut(&f.key).unwrap(); + reservation.notification_versions.insert(f.file.clone(), 0); + for index in 1..STARTUP_NOTIFICATION_PATH_CAP { + reservation + .notification_versions + .insert(f.file.with_file_name(format!("{index}.reserved")), 0); + } + assert_eq!( + manager.queue_startup_notification(&f.key, &f.file).unwrap(), + 1 + ); + let extra = f.file.with_file_name("over-cap.reserved"); + let error = manager + .queue_startup_notification(&f.key, &extra) + .unwrap_err(); + assert_eq!( + STARTUP_NOTIFICATION_QUEUE_FULL, + "language-server startup notification queue is full; retry the file change" + ); + assert!( + matches!(error, LspError::ServerNotReady(ref reason) if reason == STARTUP_NOTIFICATION_QUEUE_FULL) + ); + assert_eq!( + manager.starting[&f.key].notification_versions.len(), + STARTUP_NOTIFICATION_PATH_CAP + ); + assert!(!manager.starting[&f.key] + .pending_notifications + .contains(&extra)); + } + f.manager.lock().quiesce(); + pause.resume.send(()).unwrap(); +} + +#[test] +fn relative_diagnostics_wait_keeps_its_observation_budget_after_startup() { + let f = notification_fixture(); + f.manager.lock().set_extra_env("AFT_STARTUP_PUSH", "1"); + let pause = pause_first_spawn(&f); + locked(&f, |manager| { + manager.notify_file_changed_versioned(&f.file, "content", &f.config) + }) + .unwrap(); + pause.entered.recv_timeout(WAIT).unwrap(); + let (entered_tx, entered_rx) = unbounded(); + let (done_tx, done_rx) = unbounded(); + std::thread::scope(|scope| { + let waiting = scope.spawn(|| { + let mut manager = f.manager.lock(); + entered_tx.send(()).unwrap(); + let diagnostics = + manager.wait_for_diagnostics(&f.file, &f.config, Duration::from_millis(250)); + done_tx.send(()).unwrap(); + diagnostics + }); + entered_rx.recv_timeout(WAIT).unwrap(); + let returned_during_startup = done_rx.recv_timeout(Duration::from_secs(1)).is_ok(); + pause.resume.send(()).unwrap(); + let diagnostics = waiting.join().unwrap(); + assert!( + !returned_during_startup, + "startup consumed the observation timeout" + ); + assert_eq!(diagnostics.len(), 1); + assert_eq!(diagnostics[0].message, "startup replay diagnostic"); + }); +} + +#[test] +fn watched_changes_during_startup_replay_latest_explicit_types_once() { + let f = notification_fixture(); + f.manager.lock().set_extra_env("AFT_STARTUP_WATCHES", "1"); + let pause = pause_first_spawn(&f); + locked(&f, |manager| { + manager.notify_file_changed_versioned(&f.file, "content", &f.config) + }) + .unwrap(); + pause.entered.recv_timeout(WAIT).unwrap(); + let existing = f.file.with_file_name("existing.json"); + let created = f.file.with_file_name("created.json"); + let deleted = f.file.with_file_name("deleted.json"); + for path in [&existing, &created] { + std::fs::write(path, "{}").unwrap(); + } + locked(&f, |manager| { + manager.notify_files_watched_changed( + &[ + (existing.clone(), FileChangeType::CHANGED), + (created.clone(), FileChangeType::CREATED), + (deleted.clone(), FileChangeType::CREATED), + ], + &f.config, + ) + }) + .unwrap(); + locked(&f, |manager| { + manager + .notify_files_watched_changed(&[(deleted.clone(), FileChangeType::DELETED)], &f.config) + }) + .unwrap(); + assert_eq!( + f.manager.lock().pending_watched_changes[&f.key] + .changes + .len(), + 3 + ); + pause.resume.send(()).unwrap(); + let ready = ensure_server_for_file_detailed_unlocked(|| f.manager.lock(), &f.file, &f.config); + assert_eq!(ready.successful, vec![f.key.clone()]); + recorded_notifications(&f); + f.manager.lock().drain_events(); + let messages = recorded_notifications(&f); + let watched: Vec<_> = messages + .iter() + .filter(|message| message["method"] == "workspace/didChangeWatchedFiles") + .collect(); + assert_eq!(watched.len(), 1, "{messages:?}"); + let changes = watched[0]["params"]["changes"].as_array().unwrap(); + assert_eq!(changes.len(), 3); + for (name, kind) in [ + ("existing.json", 2), + ("created.json", 1), + ("deleted.json", 3), + ] { + let change = changes + .iter() + .find(|change| change["uri"].as_str().unwrap().ends_with(name)) + .unwrap(); + assert_eq!(change["type"], kind); + } + f.manager.lock().drain_events(); + assert_eq!( + recorded_notifications(&f) + .iter() + .filter(|message| message["method"] == "workspace/didChangeWatchedFiles") + .count(), + 1 + ); +} + +#[test] +fn watched_changes_wait_for_late_registration_without_restarting_server() { + let f = notification_fixture(); + let ready = ensure_server_for_file_detailed_unlocked(|| f.manager.lock(), &f.file, &f.config); + assert_eq!(ready.successful, vec![f.key.clone()]); + let pid = f.manager.lock().clients[&f.key].child_pid(); + let path = f.file.with_file_name("late.json"); + std::fs::write(&path, "{}").unwrap(); + f.manager + .lock() + .notify_files_watched_changed(&[(path, FileChangeType::CREATED)], &f.config) + .unwrap(); + assert!(recorded_notifications(&f).is_empty()); + recorded_notifications_with(&f, serde_json::json!({"register_watches": true})); + f.manager.lock().drain_events(); + let messages = recorded_notifications(&f); + assert_eq!(messages.len(), 1, "{messages:?}"); + assert_eq!(messages[0]["method"], "workspace/didChangeWatchedFiles"); + assert_eq!(messages[0]["params"]["changes"][0]["type"], 1); + assert_eq!(f.manager.lock().clients[&f.key].child_pid(), pid); +} + +#[test] +fn cancelled_startup_drops_queued_watched_changes() { + let f = notification_fixture(); + f.manager.lock().set_extra_env("AFT_STARTUP_WATCHES", "1"); + let pause = pause_first_spawn(&f); + locked(&f, |manager| { + manager.ensure_server_for_file_detailed(&f.file, &f.config) + }); + pause.entered.recv_timeout(WAIT).unwrap(); + let path = f.file.with_file_name("cancelled.json"); + std::fs::write(&path, "{}").unwrap(); + f.manager + .lock() + .notify_files_watched_changed(&[(path, FileChangeType::CHANGED)], &f.config) + .unwrap(); + assert!(!f.manager.lock().pending_watched_changes.is_empty()); + f.manager.lock().quiesce(); + assert!(f.manager.lock().pending_watched_changes.is_empty()); + pause.resume.send(()).unwrap(); + f.manager.lock().resume(); + f.manager.lock().spawn_hook = None; + let ready = ensure_server_for_file_detailed_unlocked(|| f.manager.lock(), &f.file, &f.config); + assert_eq!(ready.successful, vec![f.key.clone()]); + recorded_notifications(&f); + f.manager.lock().drain_events(); + assert!(recorded_notifications(&f).is_empty()); +} + +#[test] +fn watcher_changes_during_startup_keep_kind_and_registration_filtering() { + let f = notification_fixture(); + f.manager.lock().set_extra_env("AFT_STARTUP_WATCHES", "1"); + let pause = pause_first_spawn(&f); + locked(&f, |manager| { + manager.ensure_server_for_file_detailed(&f.file, &f.config) + }); + pause.entered.recv_timeout(WAIT).unwrap(); + let path = f.file.with_file_name("watcher.json"); + std::fs::write(&path, "{}").unwrap(); + f.manager + .lock() + .forward_watcher_file_events(&[(path, FileChangeType::CREATED)], &[]); + assert_eq!( + f.manager.lock().pending_watched_changes[&f.key] + .changes + .len(), + 1 + ); + pause.resume.send(()).unwrap(); + let ready = ensure_server_for_file_detailed_unlocked(|| f.manager.lock(), &f.file, &f.config); + assert_eq!(ready.successful, vec![f.key.clone()]); + recorded_notifications(&f); + f.manager.lock().drain_events(); + let messages = recorded_notifications(&f); + assert_eq!(messages.len(), 1, "{messages:?}"); + assert_eq!(messages[0]["method"], "workspace/didChangeWatchedFiles"); + assert_eq!(messages[0]["params"]["changes"][0]["type"], 1); +} diff --git a/crates/aft/src/lsp/typescript_diagnostics.rs b/crates/aft/src/lsp/typescript_diagnostics.rs index d12e2e59d9..7e10019e90 100644 --- a/crates/aft/src/lsp/typescript_diagnostics.rs +++ b/crates/aft/src/lsp/typescript_diagnostics.rs @@ -313,7 +313,10 @@ mod tests { ServerKind::TypeScript, modules.join(".bin/typescript-language-server"), ); - let opened = manager.ensure_file_open_default(&file).unwrap(); + let opened = crate::lsp::manager::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_file_open_default(&file) + }) + .unwrap(); let key = opened .server_keys .iter() diff --git a/crates/aft/src/subc/mod.rs b/crates/aft/src/subc/mod.rs index 1f83372394..0e3ed407c6 100644 --- a/crates/aft/src/subc/mod.rs +++ b/crates/aft/src/subc/mod.rs @@ -16936,7 +16936,12 @@ mod tests { ); ctx.lsp() .set_extra_env("AFT_FAKE_LSP_IGNORE_SHUTDOWN".into(), "1".into()); - assert_eq!(ctx.lsp().ensure_server_for_file(&file, &config).len(), 1); + assert_eq!( + crate::lsp::manager::settled_lsp(&mut ctx.lsp(), |startup_manager| startup_manager + .ensure_server_for_file(&file, &config)) + .len(), + 1 + ); assert_eq!(ctx.lsp().active_client_count(), 1); let executor = Arc::new(Executor::new()); assert!(executor.register_actor(root.clone(), Arc::clone(&ctx))); @@ -16963,7 +16968,9 @@ mod tests { .any(|failure| matches!(failure.result, ServerAttemptResult::ProjectClosed))); assert_eq!(ctx.lsp().active_client_count(), 0); - let file_denied = ctx.lsp().ensure_server_for_file_detailed(&file, &config); + let file_denied = crate::lsp::manager::settled_lsp(&mut ctx.lsp(), |startup_manager| { + startup_manager.ensure_server_for_file_detailed(&file, &config) + }); assert!(file_denied .attempts .iter() @@ -16993,7 +17000,12 @@ mod tests { ctx.mark_subc_bound(); ctx.lsp() .set_extra_env("AFT_FAKE_LSP_IGNORE_SHUTDOWN".into(), "0".into()); - assert_eq!(ctx.lsp().ensure_server_for_file(&file, &config).len(), 1); + assert_eq!( + crate::lsp::manager::settled_lsp(&mut ctx.lsp(), |startup_manager| startup_manager + .ensure_server_for_file(&file, &config)) + .len(), + 1 + ); let deadline = Instant::now() + Duration::from_secs(10); while ctx.lsp_child_registry().health_snapshot().children_total > 1 { assert!(Instant::now() < deadline, "old client must be reaped"); @@ -17043,7 +17055,12 @@ mod tests { "AFT_FAKE_LSP_CWD", &root.as_path().parent().unwrap().to_string_lossy(), ); - assert_eq!(ctx.lsp().ensure_server_for_file(&file, &config).len(), 1); + assert_eq!( + crate::lsp::manager::settled_lsp(&mut ctx.lsp(), |startup_manager| startup_manager + .ensure_server_for_file(&file, &config)) + .len(), + 1 + ); assert_eq!(ctx.lsp().active_client_count(), 1); let pids = ctx.lsp_child_registry().pids(); assert_eq!(pids.len(), 1, "one tracked server child: {pids:?}"); diff --git a/crates/aft/tests/helpers/mod.rs b/crates/aft/tests/helpers/mod.rs index a548a62f42..6dd97c9c79 100644 --- a/crates/aft/tests/helpers/mod.rs +++ b/crates/aft/tests/helpers/mod.rs @@ -1828,3 +1828,18 @@ pub fn canonicalize_like_product(path: &std::path::Path) -> std::path::PathBuf { } pub(crate) mod aft_binary; +/// Cold locked starts now return pending. Fixtures that need a ready server +/// wait through the same public completion path as an owned manager. +pub fn settled_lsp( + manager: &mut aft::lsp::manager::LspManager, + operation: impl Fn(&mut aft::lsp::manager::LspManager) -> R, +) -> R { + let initial = operation(manager); + if !manager.has_pending_startups() { + return initial; + } + assert!( + manager.wait_for_startups(std::time::Instant::now() + std::time::Duration::from_secs(30)) + ); + operation(manager) +} diff --git a/crates/aft/tests/integration/inspect_command_test.rs b/crates/aft/tests/integration/inspect_command_test.rs index 5342e54ec7..3a5ec46239 100644 --- a/crates/aft/tests/integration/inspect_command_test.rs +++ b/crates/aft/tests/integration/inspect_command_test.rs @@ -4480,9 +4480,9 @@ fn rust_worker_inspect_capacity_refusal_is_partial_and_names_cap() { manager.override_binary(ServerKind::Rust, fake_server_path()); manager.disable_rust_cargo_for_worker(); assert_eq!( - manager - .ensure_server_for_file(file, &Config::default()) - .len(), + crate::test_helpers::settled_lsp(&mut manager, |startup_manager| startup_manager + .ensure_server_for_file(file, &Config::default())) + .len(), 1 ); occupied.push(manager); @@ -4947,14 +4947,14 @@ fn review_python_background_pull_preserves_queued_rust_alert_once() { std::fs::write(&rust, "pub fn broken() {}\n").unwrap(); configure_fake_rust_lsp(&ctx); let config = ctx.config(); - let rust_key = ctx - .lsp() - .ensure_file_open(&rust, &config) - .unwrap() - .server_keys - .into_iter() - .find(|key| key.kind == ServerKind::Rust) - .unwrap(); + let rust_key = crate::test_helpers::settled_lsp(&mut ctx.lsp(), |manager| { + manager.ensure_file_open(&rust, &config) + }) + .unwrap() + .server_keys + .into_iter() + .find(|key| key.kind == ServerKind::Rust) + .unwrap(); let deadline = Instant::now() + Duration::from_secs(15); loop { ctx.lsp().drain_events(); @@ -4969,7 +4969,10 @@ fn review_python_background_pull_preserves_queued_rust_alert_once() { .set_extra_env("AFT_FAKE_LSP_PULL_DELAY_SIGNAL", signal.to_str().unwrap()); ctx.lsp().set_extra_env("AFT_FAKE_LSP_PULL_DELAY_MS", "500"); let python = root.join("broken.py"); - ctx.lsp().ensure_file_open(&python, &config).unwrap(); + crate::test_helpers::settled_lsp(&mut ctx.lsp(), |startup_manager| { + startup_manager.ensure_file_open(&python, &config) + }) + .unwrap(); let deadline = Instant::now() + Duration::from_secs(15); while !signal.exists() { ctx.lsp().drain_events(); @@ -5046,7 +5049,10 @@ fn python_dynamic_pull_populates_status_and_alert_source_without_inspect() { let (dir, ctx, log) = dynamic_python_pull_fixture(); let file = std::fs::canonicalize(dir.path()).unwrap().join("broken.py"); let config = ctx.config(); - ctx.lsp().ensure_file_open(&file, &config).unwrap(); + crate::test_helpers::settled_lsp(&mut ctx.lsp(), |startup_manager| { + startup_manager.ensure_file_open(&file, &config) + }) + .unwrap(); let deadline = Instant::now() + Duration::from_secs(15); let mut snapshots = Vec::new(); loop { @@ -5113,7 +5119,10 @@ fn open_dynamic_python( expected: usize, ) -> Vec { let config = ctx.config(); - ctx.lsp().ensure_file_open(file, &config).unwrap(); + crate::test_helpers::settled_lsp(&mut ctx.lsp(), |startup_manager| { + startup_manager.ensure_file_open(file, &config) + }) + .unwrap(); let deadline = Instant::now() + Duration::from_secs(15); let mut accepted = Vec::new(); loop { @@ -5421,7 +5430,10 @@ fn scoped_python_progress_second_file_after_idle_without_new_run_settles() { std::fs::write(dir.path().join("second.py"), "value: int = 'wrong'\n").unwrap(); let first = std::fs::canonicalize(dir.path()).unwrap().join("broken.py"); let config = ctx.config(); - ctx.lsp().ensure_file_open(&first, &config).unwrap(); + crate::test_helpers::settled_lsp(&mut ctx.lsp(), |startup_manager| { + startup_manager.ensure_file_open(&first, &config) + }) + .unwrap(); let deadline = Instant::now() + Duration::from_secs(15); let mut ended = false; loop { @@ -5616,7 +5628,10 @@ fn scoped_python_progress_reanalysis_keeps_warm_counts_and_alert_episode() { .diagnostics_store_mut_for_test() .mark_stale_for_file(&file); let config = ctx.config(); - ctx.lsp().ensure_file_open(&file, &config).unwrap(); + crate::test_helpers::settled_lsp(&mut ctx.lsp(), |startup_manager| { + startup_manager.ensure_file_open(&file, &config) + }) + .unwrap(); let deadline = Instant::now() + Duration::from_secs(15); loop { let drained = ctx.lsp().drain_events(); @@ -7733,10 +7748,10 @@ fn scoped_diagnostics_open_no_documents_and_close_none() { ctx.lsp().set_extra_env("AFT_FAKE_LSP_PULL", "1"); let config = ctx.config().clone(); - let open_result = ctx - .lsp() - .ensure_file_open(&pre_opened, &config) - .expect("pre-open document"); + let open_result = crate::test_helpers::settled_lsp(&mut ctx.lsp(), |manager| { + manager.ensure_file_open(&pre_opened, &config) + }) + .expect("pre-open document"); assert_eq!(open_result.newly_opened.len(), 1); let pre_open_events = collect_lsp_notifications(&ctx, "custom/documentOpened", 1); assert_eq!(pre_open_events[0]["uri"], file_uri(&pre_opened)); @@ -7796,9 +7811,10 @@ fn scoped_inspect_without_diagnostics_does_no_producer_work() { ctx.lsp() .override_binary(ServerKind::TypeScript, fake_server_path()); ctx.lsp().set_extra_env("AFT_FAKE_LSP_PULL", "1"); - ctx.lsp() - .ensure_file_open(&pre_opened, &ctx.config()) - .unwrap(); + crate::test_helpers::settled_lsp(&mut ctx.lsp(), |startup_manager| { + startup_manager.ensure_file_open(&pre_opened, &ctx.config()) + }) + .unwrap(); collect_lsp_notifications(&ctx, "custom/documentOpened", 1); ctx.update_status_bar_tier2(Some(11), Some(12), Some(13), Some(14), false); @@ -9797,7 +9813,9 @@ fn rust_inspect_restores_completed_check_after_module_restart_with_real_rust_ana let ctx = new_context(); let config = ctx.config(); let file = root.join("src/lib.rs"); - ctx.lsp().ensure_server_for_file(&file, &config); + crate::test_helpers::settled_lsp(&mut ctx.lsp(), |startup_manager| { + startup_manager.ensure_server_for_file(&file, &config) + }); let deadline = std::time::Instant::now() + std::time::Duration::from_secs(180); loop { ctx.lsp().drain_events(); @@ -10099,7 +10117,9 @@ fn scoped_rust_inspect_does_not_take_a_check_announced_before_a_save_for_the_sav // a request round trip prove it ran and the reader queued its events, // without draining those events through the manager before the save. let config = ctx.config().clone(); - let started = ctx.lsp().ensure_server_for_file(&lib, &config); + let started = crate::test_helpers::settled_lsp(&mut ctx.lsp(), |startup_manager| { + startup_manager.ensure_server_for_file(&lib, &config) + }); assert!(!started.is_empty(), "the fake rust server must start"); wait_for_path_event(&completed, "initial check completed"); ctx.lsp() diff --git a/crates/aft/tests/integration/lsp_commands_test.rs b/crates/aft/tests/integration/lsp_commands_test.rs index a8ed43c8a9..69b2920030 100644 --- a/crates/aft/tests/integration/lsp_commands_test.rs +++ b/crates/aft/tests/integration/lsp_commands_test.rs @@ -489,10 +489,10 @@ fn edit_notification_reaches_the_server_after_a_busy_manager_frees() { let ctx = app_context_with_fake_lsp(); let canonical = fs::canonicalize(&main_rs).expect("canonical main.rs"); let config = ctx.config(); - let opened = ctx - .lsp() - .ensure_file_open(&canonical, &config) - .expect("open main.rs"); + let opened = crate::test_helpers::settled_lsp(&mut ctx.lsp(), |manager| { + manager.ensure_file_open(&canonical, &config) + }) + .expect("open main.rs"); assert_eq!( opened.server_keys.len(), 1, diff --git a/crates/aft/tests/integration/lsp_diagnostics_test.rs b/crates/aft/tests/integration/lsp_diagnostics_test.rs index 4b361fb153..8ed3001a57 100644 --- a/crates/aft/tests/integration/lsp_diagnostics_test.rs +++ b/crates/aft/tests/integration/lsp_diagnostics_test.rs @@ -132,12 +132,19 @@ fn collect_watched_file_events(manager: &mut LspManager) -> serde_json::Value { } fn collect_watched_file_events_from_ctx(ctx: &AppContext) -> serde_json::Value { - let event = collect_event(&mut ctx.lsp(), |event| { - matches!( - event, - LspEvent::Notification { method, .. } if method == "custom/watchedFilesChanged" - ) - }) + let deadline = Instant::now() + Duration::from_secs(15); + let event = loop { + // An asynchronous post-write hook may need this manager to send its + // queued changes. Do not retain the guard while waiting for its notification. + let drained = ctx.lsp().drain_events(); + if let Some(event) = drained.into_iter().find(|event| matches!( + event, LspEvent::Notification { method, .. } if method == "custom/watchedFilesChanged" + )) { break Some(event); } + if Instant::now() >= deadline { + break None; + } + thread::sleep(Duration::from_millis(25)); + } .unwrap_or_else(|| { #[cfg(windows)] panic!( @@ -260,9 +267,9 @@ fn pyright_diagnostics_for( // Start the server first so its startup does not use up the edit's // diagnostics budget, which production caps at 10 seconds. assert!( - !ctx.lsp() - .ensure_server_for_file(file, &ctx.config()) - .is_empty(), + !crate::test_helpers::settled_lsp(&mut ctx.lsp(), |startup_manager| startup_manager + .ensure_server_for_file(file, &ctx.config())) + .is_empty(), "pyright should start for the fixture before diagnostics can be checked" ); @@ -484,9 +491,9 @@ fn pyright_edit_error_reaches_status_bar_alert_source_and_inline_diagnostics() { "configure failed: {configured:#}" ); assert!( - !ctx.lsp() - .ensure_server_for_file(&file, &ctx.config()) - .is_empty(), + !crate::test_helpers::settled_lsp(&mut ctx.lsp(), |startup_manager| startup_manager + .ensure_server_for_file(&file, &ctx.config())) + .is_empty(), "pyright should start for the fixture" ); @@ -2079,7 +2086,10 @@ fn test_ensure_file_open_detects_disk_drift() { let mut manager = manager_with_fake_server(); // Open the file the first time. - manager.ensure_file_open(file, &config).expect("first open"); + crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_file_open(file, &config) + }) + .expect("first open"); let original_mtime = fs::metadata(file).unwrap().modified().unwrap(); @@ -2096,9 +2106,10 @@ fn test_ensure_file_open_detects_disk_drift() { // Drain anything queued and then re-open. Should re-sync (didChange). let _ = manager.drain_events(); - manager - .ensure_file_open(file, &config) - .expect("re-open after drift"); + crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_file_open(file, &config) + }) + .expect("re-open after drift"); // The fake server emits "custom/documentChanged" on didChange. If // ensure_file_open detected drift, that notification arrives. @@ -3497,7 +3508,9 @@ fn workspace_unchanged_reply_does_not_relabel_an_interleaved_report() { ] { manager.set_extra_env(name, "1"); } - let keys = manager.ensure_server_for_file(&files[0], &config); + let keys = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file(&files[0], &config) + }); assert_eq!(keys.len(), 1, "workspace producer must start"); let key = &keys[0]; let file = key.root.join("src/main.rs"); diff --git a/crates/aft/tests/integration/lsp_manager_test.rs b/crates/aft/tests/integration/lsp_manager_test.rs index 9c7c514951..c39167a548 100644 --- a/crates/aft/tests/integration/lsp_manager_test.rs +++ b/crates/aft/tests/integration/lsp_manager_test.rs @@ -49,7 +49,12 @@ fn rust_worker_config_disables_all_compilation_and_head_config_is_unchanged() { let log = temp.path().join("initialization.jsonl"); let mut manager = rust_worker_logged_manager(&log); let config = Config::default(); - assert_eq!(manager.ensure_server_for_file(&file, &config).len(), 1); + assert_eq!( + crate::test_helpers::settled_lsp(&mut manager, |startup_manager| startup_manager + .ensure_server_for_file(&file, &config)) + .len(), + 1 + ); manager.shutdown_all(); manager.disable_rust_cargo_for_worker(); let mut worker_config = config.clone(); @@ -65,7 +70,9 @@ fn rust_worker_config_disables_all_compilation_and_head_config_is_unchanged() { ..Default::default() }); assert_eq!( - manager.ensure_server_for_file(&file, &worker_config).len(), + crate::test_helpers::settled_lsp(&mut manager, |startup_manager| startup_manager + .ensure_server_for_file(&file, &worker_config)) + .len(), 1 ); let rows: Vec = fs::read_to_string(&log) @@ -105,18 +112,33 @@ fn rust_worker_shared_root_restarts_head_analyzer_and_never_reenables_cargo() { let log = temp.path().join("initialization.jsonl"); let mut manager = rust_worker_logged_manager(&log); let config = Config::default(); - assert_eq!(manager.ensure_server_for_file(&file, &config).len(), 1); + assert_eq!( + crate::test_helpers::settled_lsp(&mut manager, |startup_manager| startup_manager + .ensure_server_for_file(&file, &config)) + .len(), + 1 + ); manager.disable_rust_cargo_for_worker(); assert_eq!( manager.server_count(), 0, "worker admission must retire the head's analyzer" ); - assert_eq!(manager.ensure_server_for_file(&file, &config).len(), 1); + assert_eq!( + crate::test_helpers::settled_lsp(&mut manager, |startup_manager| startup_manager + .ensure_server_for_file(&file, &config)) + .len(), + 1 + ); // Simulate a primary session reading another file after a delegated // session arrived. Both share the root's analyzer, so the primary read // must not turn compilation back on for the delegated session. - assert_eq!(manager.ensure_server_for_file(&other, &config).len(), 1); + assert_eq!( + crate::test_helpers::settled_lsp(&mut manager, |startup_manager| startup_manager + .ensure_server_for_file(&other, &config)) + .len(), + 1 + ); manager.disable_rust_cargo_for_worker(); assert_eq!(manager.server_count(), 1, "worker admission is idempotent"); let rows: Vec = fs::read_to_string(&log) @@ -147,16 +169,24 @@ fn rust_worker_cap_refuses_n_plus_one_without_spawn() { let config = Config::default(); for index in 0..2 { assert_eq!( - managers[index] - .ensure_server_for_file(&fixtures[index].1, &config) - .len(), + crate::test_helpers::settled_lsp(&mut managers[index], |startup_manager| { + startup_manager.ensure_server_for_file(&fixtures[index].1, &config) + }) + .len(), 1 ); } let pid_dir = fixtures[2].0.path().join("pids"); fs::create_dir(&pid_dir).unwrap(); managers[2].set_extra_env("AFT_FAKE_LSP_PID_DIR", &pid_dir.display().to_string()); - let result = managers[2].ensure_server_for_file_detailed(&fixtures[2].1, &config); + let result = { + let manager = std::cell::RefCell::new(&mut managers[2]); + aft::lsp::manager::ensure_server_for_file_detailed_unlocked( + || std::cell::RefMut::map(manager.borrow_mut(), |manager| &mut **manager), + &fixtures[2].1, + &config, + ) + }; assert!( result.successful.is_empty(), "the third analyzer must not be admitted" @@ -189,13 +219,21 @@ fn rust_worker_cap_admits_next_request_after_slot_release_without_restart() { let config = Config::default(); for index in 0..2 { assert_eq!( - managers[index] - .ensure_server_for_file(&fixtures[index].1, &config) - .len(), + crate::test_helpers::settled_lsp(&mut managers[index], |startup_manager| { + startup_manager.ensure_server_for_file(&fixtures[index].1, &config) + }) + .len(), 1 ); } - let refused = managers[2].ensure_server_for_file_detailed(&fixtures[2].1, &config); + let refused = { + let manager = std::cell::RefCell::new(&mut managers[2]); + aft::lsp::manager::ensure_server_for_file_detailed_unlocked( + || std::cell::RefMut::map(manager.borrow_mut(), |manager| &mut **manager), + &fixtures[2].1, + &config, + ) + }; assert!(refused.successful.is_empty()); assert!(refused.attempts[0] .result @@ -203,8 +241,7 @@ fn rust_worker_cap_admits_next_request_after_slot_release_without_restart() { .contains("rust_analyzer_capacity_exhausted")); drop(managers.remove(0)); assert_eq!( - managers[1] - .ensure_server_for_file(&fixtures[2].1, &config) + crate::test_helpers::settled_lsp(&mut managers[1], |startup_manager| startup_manager.ensure_server_for_file(&fixtures[2].1, &config)) .len(), 1, "the very next request must start after slot release without clearing failures or restarting" @@ -226,9 +263,10 @@ fn rust_worker_cap_never_counts_or_refuses_head_analyzers() { // after both worker slots are full. Neither may consume or need a slot. for index in 0..4 { assert_eq!( - managers[index] - .ensure_server_for_file(&fixtures[index].1, &Config::default()) - .len(), + crate::test_helpers::settled_lsp(&mut managers[index], |startup_manager| { + startup_manager.ensure_server_for_file(&fixtures[index].1, &Config::default()) + }) + .len(), 1, "head analyzers must not consume or be refused worker capacity (start {index})" ); @@ -711,7 +749,9 @@ fn test_manager_spawns_server_on_first_touch() { let mut manager = LspManager::new(); manager.override_binary(ServerKind::Rust, fake_server_path()); - let keys = manager.ensure_server_for_file_default(&main_rs); + let keys = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_default(&main_rs) + }); assert_eq!(keys.len(), 1); assert_eq!( @@ -733,8 +773,12 @@ fn test_manager_reuses_existing_server() { let mut manager = LspManager::new(); manager.override_binary(ServerKind::Rust, fake_server_path()); - let first = manager.ensure_server_for_file_default(&main_rs); - let second = manager.ensure_server_for_file_default(&lib_rs); + let first = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_default(&main_rs) + }); + let second = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_default(&lib_rs) + }); assert_eq!(first.len(), 1); assert_eq!(second.len(), 1); @@ -748,8 +792,12 @@ fn rust_workspace_members_share_one_analyzer_root() { let mut manager = LspManager::new(); manager.override_binary(ServerKind::Rust, fake_server_path()); - let first = manager.ensure_server_for_file_default(&first_source); - let second = manager.ensure_server_for_file_default(&second_source); + let first = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_default(&first_source) + }); + let second = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_default(&second_source) + }); let expected_root = canonicalize_lsp_root(&workspace); assert_eq!(first.len(), 1); @@ -765,7 +813,9 @@ fn test_manager_shutdown_all() { let mut manager = LspManager::new(); manager.override_binary(ServerKind::Rust, fake_server_path()); - manager.ensure_server_for_file_default(&main_rs); + crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_default(&main_rs) + }); assert_eq!(manager.active_client_count(), 1); manager.shutdown_all(); @@ -785,7 +835,9 @@ fn shutdown_taken_clients_shares_deadline_across_roots_and_reaps_every_server() manager.set_child_registry(registry.clone()); manager.override_binary(ServerKind::Rust, fake_server_path()); manager.set_extra_env("AFT_FAKE_LSP_IGNORE_SHUTDOWN", "1"); - manager.ensure_server_for_file_default(main_rs); + crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_default(main_rs) + }); managers.push(manager); } let pids = registry.pids(); @@ -828,7 +880,9 @@ fn shutdown_all_bounds_unresponsive_servers_and_reaps_them() { manager.set_extra_env("AFT_FAKE_LSP_IGNORE_SHUTDOWN", "1"); for (_, main_rs, _) in &fixtures { - manager.ensure_server_for_file_default(main_rs); + crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_default(main_rs) + }); } let pids = registry.pids(); assert_eq!(pids.len(), 6, "each fixture must have its own LSP child"); @@ -877,7 +931,9 @@ fn shutdown_all_force_kills_servers_ignoring_sigterm_within_the_budget_with_dela manager.set_extra_env("AFT_FAKE_LSP_IGNORE_SIGTERM", "1"); manager.set_extra_env("AFT_FAKE_LSP_EXIT_DELAY_MS", "30000"); for (_, main_rs, _) in &fixtures { - manager.ensure_server_for_file_default(main_rs); + crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_default(main_rs) + }); } let pids = registry.pids(); assert_eq!(pids.len(), 6, "each fixture must have its own LSP child"); @@ -920,7 +976,9 @@ fn shutdown_all_reports_graceful_servers_within_the_budget() { manager.override_binary(ServerKind::Rust, fake_server_path()); for (_, main_rs, _) in &fixtures { - manager.ensure_server_for_file_default(main_rs); + crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_default(main_rs) + }); } assert_eq!(manager.active_client_count(), 2); @@ -947,7 +1005,9 @@ fn test_server_lifecycle_states() { let mut manager = LspManager::new(); manager.override_binary(ServerKind::Rust, fake_server_path()); - manager.ensure_server_for_file_default(&main_rs); + crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_default(&main_rs) + }); let client = manager .client_for_file_default(&main_rs) @@ -964,7 +1024,9 @@ fn test_manager_handles_missing_binary() { PathBuf::from("/definitely/missing/fake-lsp-server"), ); - let keys = manager.ensure_server_for_file_default(&main_rs); + let keys = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_default(&main_rs) + }); assert!(keys.is_empty()); assert_eq!(manager.active_client_count(), 0); @@ -1010,7 +1072,9 @@ fn test_custom_server_env_and_initialization_options_reach_spawned_server() { fake_server_path(), ); - let keys = manager.ensure_server_for_file(&main_typ, &config); + let keys = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file(&main_typ, &config) + }); assert_eq!(keys.len(), 1); assert_eq!(manager.active_client_count(), 1); @@ -1045,7 +1109,9 @@ fn astro_uses_the_nearest_project_typescript_sdk() { let mut manager = LspManager::new(); manager.override_binary(ServerKind::Astro, fake_server_path()); - let outcomes = manager.ensure_server_for_file_detailed(&page, &config); + let outcomes = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_detailed(&page, &config) + }); assert_eq!(outcomes.successful.len(), 1, "outcomes: {outcomes:?}"); let initialized = collect_notification(&mut manager, "custom/initialized"); // Compare with normalized separators: the product joins the tsdk tail with @@ -1090,7 +1156,9 @@ fn astro_user_initialization_options_override_the_computed_tsdk() { let mut manager = LspManager::new(); manager.override_binary(ServerKind::Astro, fake_server_path()); - let outcomes = manager.ensure_server_for_file_detailed(&page, &config); + let outcomes = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_detailed(&page, &config) + }); assert_eq!(outcomes.successful.len(), 1, "outcomes: {outcomes:?}"); let initialized = collect_notification(&mut manager, "custom/initialized"); assert_eq!( @@ -1123,7 +1191,9 @@ fn astro_without_project_typescript_is_unavailable_without_spawning() { &started_signal.to_string_lossy(), ); - let outcomes = manager.ensure_server_for_file_detailed(&page, &config); + let outcomes = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_detailed(&page, &config) + }); assert!(outcomes.successful.is_empty()); let astro_attempt = outcomes .attempts @@ -1154,7 +1224,9 @@ fn watched_file_capability_defaults_false_when_initialize_has_no_field() { // initialize result so the client's capability tracker defaults to false. manager.set_extra_env("AFT_FAKE_LSP_NO_WATCHED_FILES", "1"); - let keys = manager.ensure_server_for_file(&main_rs, &config); + let keys = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file(&main_rs, &config) + }); assert_eq!(keys.len(), 1); let client = manager.client_for_file(&main_rs, &config).expect("client"); @@ -1207,7 +1279,9 @@ fn watched_file_batch_continues_after_a_server_rejects_send() { } let mut sources_by_key = HashMap::new(); for source in sources { - let started = manager.ensure_server_for_file(&source, &config); + let started = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file(&source, &config) + }); assert_eq!(started.len(), 1, "the fixture must start one producer"); let key = started .into_iter() @@ -1300,7 +1374,9 @@ fn echoed_client_watched_file_capability_does_not_enable_notifications() { manager.override_binary(server_kind, protocol_server); manager.set_extra_env("AFT_PROTOCOL_LSP_MODE", "static-watch"); - let keys = manager.ensure_server_for_file(&source, &config); + let keys = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file(&source, &config) + }); assert_eq!(keys.len(), 1); assert!( !manager @@ -1369,7 +1445,9 @@ fn watched_file_notifications_require_dynamic_registration_and_stop_after_unregi manager.override_binary(server_kind, protocol_server); manager.set_extra_env("AFT_PROTOCOL_LSP_MODE", "watch"); - let keys = manager.ensure_server_for_file(&source, &config); + let keys = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file(&source, &config) + }); assert_eq!(keys.len(), 1); let registered = collect_event( @@ -1461,7 +1539,9 @@ fn workspace_pull_timeout_sends_cancel_request() { manager.override_binary(server_kind, protocol_server); manager.set_extra_env("AFT_PROTOCOL_LSP_MODE", "workspace-timeout"); - let keys = manager.ensure_server_for_file(&source, &config); + let keys = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file(&source, &config) + }); assert_eq!(keys.len(), 1); let started = Instant::now(); @@ -1508,7 +1588,9 @@ fn failed_spawn_is_cached_and_not_retried_on_second_call() { ); // First call — must produce a BinaryNotInstalled attempt. - let first = manager.ensure_server_for_file_detailed(&main_rs, &Config::default()); + let first = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_detailed(&main_rs, &Config::default()) + }); assert_eq!(first.attempts.len(), 1); let first_result = &first.attempts[0].result; assert!( @@ -1519,7 +1601,9 @@ fn failed_spawn_is_cached_and_not_retried_on_second_call() { assert_eq!(manager.active_client_count(), 0); // Second call — must return the SAME cached classification, no new spawn. - let second = manager.ensure_server_for_file_detailed(&main_rs, &Config::default()); + let second = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_detailed(&main_rs, &Config::default()) + }); assert_eq!(second.attempts.len(), 1); let second_result = &second.attempts[0].result; // The cached result is cloned, so the variant must match the first call's. @@ -1543,7 +1627,9 @@ fn failed_spawn_dedup_persists_across_many_calls() { // Simulate the production case: many file events in a row. Without dedup, // each one would log a fresh ERROR and try to spawn the missing binary. for _ in 0..10 { - let outcome = manager.ensure_server_for_file_detailed(&main_rs, &Config::default()); + let outcome = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_detailed(&main_rs, &Config::default()) + }); assert_eq!(outcome.attempts.len(), 1); assert_eq!(manager.active_client_count(), 0); } @@ -1563,7 +1649,9 @@ fn failed_spawn_for_one_root_does_not_block_a_different_root() { ServerKind::Rust, PathBuf::from("/definitely/missing/fake-lsp-server-XYZZY"), ); - let outcome_a = manager.ensure_server_for_file_detailed(&main_rs_a, &Config::default()); + let outcome_a = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_detailed(&main_rs_a, &Config::default()) + }); assert_eq!(outcome_a.successful.len(), 0); assert_eq!(manager.active_client_count(), 0); @@ -1575,11 +1663,15 @@ fn failed_spawn_for_one_root_does_not_block_a_different_root() { // Workspace A still returns the cached failure (no retry on a different // binary path — the cache deliberately survives override changes mid-session // because runtime overrides are a test-only feature). - let outcome_a_again = manager.ensure_server_for_file_detailed(&main_rs_a, &Config::default()); + let outcome_a_again = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_detailed(&main_rs_a, &Config::default()) + }); assert_eq!(outcome_a_again.successful.len(), 0); // Workspace B is a fresh (kind, root) pair → not in the failed cache → spawns. - let outcome_b = manager.ensure_server_for_file_detailed(&main_rs_b, &Config::default()); + let outcome_b = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_detailed(&main_rs_b, &Config::default()) + }); assert_eq!(outcome_b.successful.len(), 1); assert_eq!(manager.active_client_count(), 1); } @@ -1728,7 +1820,9 @@ fn transient_handshake_timeout_on_file_events_is_retried_after_backoff_window() ); manager.set_extra_env("AFT_FAKE_LSP_PID_DIR", &pid_dir.display().to_string()); - let first = manager.ensure_server_for_file_detailed(&main_rs, &Config::default()); + let first = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_detailed(&main_rs, &Config::default()) + }); assert!(first.successful.is_empty(), "{first:?}"); assert_eq!(fake_server_spawns(&pid_dir), 1); let ServerAttemptResult::SpawnFailed { reason, .. } = &first.attempts[0].result else { @@ -1742,7 +1836,9 @@ fn transient_handshake_timeout_on_file_events_is_retried_after_backoff_window() // Every file event inside the window replays the failure. for _ in 0..5 { offset.fetch_add(5_000, Ordering::SeqCst); - let replay = manager.ensure_server_for_file_detailed(&lib_rs, &Config::default()); + let replay = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_detailed(&lib_rs, &Config::default()) + }); assert!(replay.successful.is_empty(), "{replay:?}"); } assert_eq!( @@ -1752,7 +1848,9 @@ fn transient_handshake_timeout_on_file_events_is_retried_after_backoff_window() ); offset.fetch_add(5_000, Ordering::SeqCst); - let retried = manager.ensure_server_for_file_detailed(&main_rs, &Config::default()); + let retried = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_detailed(&main_rs, &Config::default()) + }); assert_eq!(fake_server_spawns(&pid_dir), 2, "{retried:?}"); assert_eq!(retried.successful.len(), 1, "{retried:?}"); assert_eq!(manager.active_client_count(), 1); @@ -1775,7 +1873,9 @@ fn permanent_missing_binary_failure_is_replayed_after_backoff_window() { ServerKind::Rust, PathBuf::from("/definitely/missing/fake-lsp-server-XYZZY"), ); - let first = manager.ensure_server_for_file_detailed(&main_rs, &Config::default()); + let first = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_detailed(&main_rs, &Config::default()) + }); assert!( matches!( first.attempts[0].result, @@ -1787,7 +1887,9 @@ fn permanent_missing_binary_failure_is_replayed_after_backoff_window() { // Make the binary available, then step past the longest backoff. manager.override_binary(ServerKind::Rust, fake_server_path()); offset.fetch_add(60 * 60 * 1_000, Ordering::SeqCst); - let later = manager.ensure_server_for_file_detailed(&main_rs, &Config::default()); + let later = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_detailed(&main_rs, &Config::default()) + }); assert!( matches!( later.attempts[0].result, @@ -1827,7 +1929,9 @@ fn spawned_lsp_child_is_tracked_in_registry() { "registry should start empty before any spawn" ); - let keys = manager.ensure_server_for_file_default(&main_rs); + let keys = crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_default(&main_rs) + }); assert_eq!(keys.len(), 1); assert_eq!(manager.active_client_count(), 1); @@ -1847,7 +1951,9 @@ fn shutdown_all_untracks_pids_from_registry() { manager.set_child_registry(registry.clone()); manager.override_binary(ServerKind::Rust, fake_server_path()); - manager.ensure_server_for_file_default(&main_rs); + crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_default(&main_rs) + }); assert_eq!(registry.pids().len(), 1); manager.shutdown_all(); @@ -1869,7 +1975,9 @@ fn dropping_manager_untracks_pids_from_registry() { let mut manager = LspManager::new(); manager.set_child_registry(registry.clone()); manager.override_binary(ServerKind::Rust, fake_server_path()); - manager.ensure_server_for_file_default(&main_rs); + crate::test_helpers::settled_lsp(&mut manager, |startup_manager| { + startup_manager.ensure_server_for_file_default(&main_rs) + }); assert_eq!(registry.pids().len(), 1); } // manager and its LspClients drop here