From 8d0cdf8d04bbb770f558abed1c4180fd8720ea74 Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Sat, 24 Oct 2026 10:37:44 -0700 Subject: [PATCH 01/12] fix(lsp): retain admitted diagnostic snapshots for pull Transfer completed diagnostic work into bounded session-owned reports so push publication and repeated pulls share the same admitted evidence. Fence retained reports to document and scope identity, and retire exact operations on invalidation without claiming analyzer work has settled. Keep incomplete results truthful and retryable, and retry publication only for outbound backpressure rather than duplicate or oversized pushes. Fixes #4404 --- .../src/lsp_support/factory.rs | 11 + crates/tracedecay-lsp/src/operation_table.rs | 22 + crates/tracedecay-lsp/src/overlay.rs | 16 + crates/tracedecay-lsp/src/protocol.rs | 4 + .../src/protocol/diagnostic_snapshots.rs | 293 +++++++ .../src/protocol/diagnostics_controller.rs | 230 ++--- .../src/protocol/lifecycle_controller.rs | 19 +- .../src/protocol/outbound_controller.rs | 47 +- crates/tracedecay-lsp/src/protocol/tests.rs | 1 + .../protocol/tests/diagnostic_publication.rs | 6 +- .../protocol/tests/diagnostic_snapshots.rs | 826 ++++++++++++++++++ crates/tracedecay-lsp/src/provider.rs | 22 +- 12 files changed, 1338 insertions(+), 159 deletions(-) create mode 100644 crates/tracedecay-lsp/src/protocol/diagnostic_snapshots.rs create mode 100644 crates/tracedecay-lsp/src/protocol/tests/diagnostic_snapshots.rs diff --git a/crates/tracedecay-application/src/lsp_support/factory.rs b/crates/tracedecay-application/src/lsp_support/factory.rs index b352e5c66c..2e0193b905 100644 --- a/crates/tracedecay-application/src/lsp_support/factory.rs +++ b/crates/tracedecay-application/src/lsp_support/factory.rs @@ -486,6 +486,17 @@ impl DiagnosticSnapshotPort for FederatedDiagnostics { ) } + fn retire_document_refresh( + &self, + root: &AdmittedRoot, + document_uri: &str, + operation_id: &str, + ) -> bool { + self.roots + .get(root.uri()) + .is_some_and(|port| port.retire_document_refresh(root, document_uri, operation_id)) + } + fn supports_workspace_diagnostics(&self) -> bool { !self.roots.is_empty() && self diff --git a/crates/tracedecay-lsp/src/operation_table.rs b/crates/tracedecay-lsp/src/operation_table.rs index 3aef0a46a4..efbce9425b 100644 --- a/crates/tracedecay-lsp/src/operation_table.rs +++ b/crates/tracedecay-lsp/src/operation_table.rs @@ -178,6 +178,28 @@ where false } } + + /// Retires one exact operation without requiring a caller to reconstruct + /// the input key after that document's lifetime has already changed. + pub(crate) fn cancel_matching(&self, matches: impl Fn(&K, &M) -> bool) -> bool + where + K: Clone, + { + let pending = { + let mut in_flight = self.in_flight(); + let key = in_flight + .iter() + .find(|(key, pending)| matches(key, &pending.metadata)) + .map(|(key, _)| key.clone()); + key.and_then(|key| in_flight.remove(&key)) + }; + if let Some(pending) = pending { + pending.task.abort(); + true + } else { + false + } + } } impl Drop for BoundedOperationTable { diff --git a/crates/tracedecay-lsp/src/overlay.rs b/crates/tracedecay-lsp/src/overlay.rs index 7d6893b09f..0a7f0d6f47 100644 --- a/crates/tracedecay-lsp/src/overlay.rs +++ b/crates/tracedecay-lsp/src/overlay.rs @@ -394,6 +394,7 @@ pub const MAX_DIAGNOSTIC_OPERATIONS: usize = 128; #[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)] struct DiagnosticOperationKey { root_uri: String, + scope_digest: Option, document_uri: String, overlay_version: i64, overlay_language_id: Option, @@ -435,6 +436,7 @@ impl DiagnosticSnapshotAdapter { ) -> DiagnosticOperationKey { DiagnosticOperationKey { root_uri: root.uri().to_owned(), + scope_digest: root.scope_digest().cloned(), document_uri: document_uri.to_owned(), overlay_version: overlay.map_or(0, |overlay| overlay.version), overlay_language_id: overlay.map(|overlay| overlay.language_id.clone()), @@ -534,6 +536,20 @@ impl DiagnosticSnapshotPort for DiagnosticSnapshotAdapter { } } + fn retire_document_refresh( + &self, + root: &AdmittedRoot, + document_uri: &str, + operation_id: &str, + ) -> bool { + self.operations.cancel_matching(|key, identity| { + key.root_uri == root.uri() + && key.scope_digest.as_ref() == root.scope_digest() + && key.document_uri == document_uri + && identity.operation_id == operation_id + }) + } + fn supports_workspace_diagnostics(&self) -> bool { self.workspace.supports() } diff --git a/crates/tracedecay-lsp/src/protocol.rs b/crates/tracedecay-lsp/src/protocol.rs index 0da33a001d..b76392ba09 100644 --- a/crates/tracedecay-lsp/src/protocol.rs +++ b/crates/tracedecay-lsp/src/protocol.rs @@ -76,6 +76,7 @@ pub const MAX_QUEUED_OUTBOUND_MESSAGES: usize = 64; pub(crate) const TRACEDECAY_NATIVE_DIAGNOSTICS_METHOD: &str = "tracedecay/nativeDiagnostics"; mod context_controller; +mod diagnostic_snapshots; mod diagnostics_controller; mod dynamic_diagnostics_controller; mod lifecycle_controller; @@ -284,6 +285,8 @@ where .cloned() .collect::>(); self.clear_removed_workspace_root_state(&removed_roots); + self.clear_document_diagnostics(); + self.diagnostics.published.clear(); self.diagnostics.workspace_snapshots.clear(); self.diagnostics.workspace_failures.clear(); self.lifecycle.gateway.replace_workspace(workspace); @@ -491,6 +494,7 @@ where } fn invalidate_document(&mut self, uri: &str) { + self.invalidate_document_diagnostics(uri); self.diagnostics.workspace_snapshots.clear(); self.diagnostics.workspace_failures.clear(); self.discard_document_context(uri); diff --git a/crates/tracedecay-lsp/src/protocol/diagnostic_snapshots.rs b/crates/tracedecay-lsp/src/protocol/diagnostic_snapshots.rs new file mode 100644 index 0000000000..a7f8751fd1 --- /dev/null +++ b/crates/tracedecay-lsp/src/protocol/diagnostic_snapshots.rs @@ -0,0 +1,293 @@ +use tracedecay_domain::{ContentDigest, ManifestDigest}; + +use crate::diagnostics::DiagnosticMerge; +use crate::gateway::{AdmittedRoot, FeedbackCyclePort, SemanticProviderPort}; +use crate::overlay::{OverlayLifetime, OverlaySnapshot}; +use crate::provider::{ + DiagnosticRefreshIdentity, DiagnosticSnapshotOutcome, DiagnosticSnapshotPort, + GenerationDiagnostics, +}; +use crate::rpc::{DiagnosticSerializationCapabilities, diagnostic_value}; +use crate::workspace_diagnostics::{ + MAX_WORKSPACE_DIAGNOSTIC_BYTES, MAX_WORKSPACE_DIAGNOSTIC_RESULTS, +}; + +use super::DaemonLspProtocolSession; +use super::diagnostics_controller::PublishedDiagnostic; + +#[derive(Clone, Debug, Eq, PartialEq)] +pub(super) struct DiagnosticDocumentIdentity { + pub(super) root: AdmittedRoot, + scope_set_digest: Option, + version: i64, + language_id: Option, + content_digest: Option, + lifetime: Option, +} + +impl DiagnosticDocumentIdentity { + pub(super) fn new( + root: &AdmittedRoot, + scope_set_digest: Option<&ManifestDigest>, + overlay: Option<&OverlaySnapshot>, + ) -> Self { + Self { + root: root.clone(), + scope_set_digest: scope_set_digest.cloned(), + version: overlay.map_or(0, |overlay| overlay.version), + language_id: overlay.map(|overlay| overlay.language_id.clone()), + content_digest: overlay.map(|overlay| overlay.content_digest.clone()), + lifetime: overlay.map(|overlay| overlay.lifetime.clone()), + } + } +} + +#[derive(Clone)] +pub(super) struct PendingDiagnosticRefresh { + pub(super) identity: DiagnosticRefreshIdentity, + pub(super) document: DiagnosticDocumentIdentity, +} + +pub(super) struct RetainedDiagnosticSnapshot { + document: DiagnosticDocumentIdentity, + outcome: DiagnosticSnapshotOutcome, +} + +impl DaemonLspProtocolSession +where + P: FeedbackCyclePort, + S: SemanticProviderPort, + D: DiagnosticSnapshotPort, +{ + /// The adapter transfers a completion once. The session admits it once, + /// then owns the bounded report used by both push and pull presentation. + pub(super) fn document_diagnostic_snapshot( + &mut self, + root: &AdmittedRoot, + uri: &str, + overlay: Option<&OverlaySnapshot>, + ) -> DiagnosticSnapshotOutcome { + let document = DiagnosticDocumentIdentity::new( + root, + self.lifecycle.gateway.workspace().scope_set_digest(), + overlay, + ); + if self + .diagnostics + .active_refreshes + .get(uri) + .is_some_and(|pending| pending.document != document) + { + self.invalidate_document_diagnostics(uri); + } + if self + .diagnostics + .document_snapshots + .get(uri) + .is_some_and(|retained| retained.document != document) + { + self.diagnostics.document_snapshots.remove(uri); + } + + let outcome = self + .diagnostics + .provider + .document_diagnostics(root, uri, overlay); + let outcome = match outcome { + DiagnosticSnapshotOutcome::Partial { ref coverage, .. } + if coverage == "refresh-required" => + { + return self + .diagnostics + .document_snapshots + .get(uri) + .map_or(outcome, |retained| retained.outcome.clone()); + } + DiagnosticSnapshotOutcome::Ready { + mut diagnostics, + completed_operation_id, + } => { + if let Some(operation_id) = completed_operation_id.as_deref() { + let pending = self.diagnostics.active_refreshes.get(uri); + if !pending.is_some_and(|pending| { + pending.document == document + && pending.identity.operation_id == operation_id + }) { + return pending.map_or_else( + || DiagnosticSnapshotOutcome::Partial { + source_generation: None, + coverage: "refresh-required".to_owned(), + }, + |pending| { + DiagnosticSnapshotOutcome::Refreshing(pending.identity.clone()) + }, + ); + } + } + let invalid_generation = self + .diagnostics + .active_refreshes + .get(uri) + .filter(|_| completed_operation_id.is_some()) + .is_some_and(|pending| { + pending + .identity + .target_generation + .is_some_and(|target| target != diagnostics.generation) + || pending + .identity + .source_generation + .is_some_and(|source| diagnostics.generation < source) + }); + let superseded = self + .diagnostics + .published + .get(uri) + .is_some_and(|published| { + published.version == document.version + && published.generation > diagnostics.generation + && published.scope_digest == root.scope_digest().cloned() + && published.scope_set_digest == document.scope_set_digest + }) + || self + .diagnostics + .document_snapshots + .get(uri) + .is_some_and(|retained| { + matches!(&retained.outcome, DiagnosticSnapshotOutcome::Ready { + diagnostics: previous, .. + } if previous.generation > diagnostics.generation) + }); + if superseded || invalid_generation { + DiagnosticSnapshotOutcome::Failed { + source_generation: Some(diagnostics.generation), + failure_class: "superseded-generation".to_owned(), + } + } else { + // Keep the producer lanes separate: push and pull can have + // different capabilities. Each lane's sorted prefix suffices + // for the bounded merge, including Cursor's hidden lanes. + diagnostics.upstream = + DiagnosticMerge::for_document(uri, diagnostics.upstream, Vec::new()).items; + diagnostics.tracedecay = + DiagnosticMerge::for_document(uri, Vec::new(), diagnostics.tracedecay) + .items; + match diagnostic_snapshot_fits(&diagnostics) { + Ok(()) => DiagnosticSnapshotOutcome::Ready { + diagnostics, + completed_operation_id, + }, + Err(failure_class) => DiagnosticSnapshotOutcome::Failed { + source_generation: Some(diagnostics.generation), + failure_class: failure_class.to_owned(), + }, + } + } + } + DiagnosticSnapshotOutcome::Refreshing(identity) => { + self.diagnostics.document_snapshots.remove(uri); + return DiagnosticSnapshotOutcome::Refreshing(identity); + } + terminal => terminal, + }; + + if !self.reserve_document_diagnostic_snapshot(uri) { + return DiagnosticSnapshotOutcome::Failed { + source_generation: None, + failure_class: "diagnostic-result-capacity".to_owned(), + }; + } + if let DiagnosticSnapshotOutcome::Ready { diagnostics, .. } = &outcome { + // Admission establishes the generation floor even if pushing the + // report is backpressured or only pull delivery is negotiated. + self.diagnostics.published.insert( + uri.to_owned(), + PublishedDiagnostic { + version: document.version, + generation: diagnostics.generation, + scope_digest: root.scope_digest().cloned(), + scope_set_digest: document.scope_set_digest.clone(), + }, + ); + } + self.diagnostics.document_snapshots.insert( + uri.to_owned(), + RetainedDiagnosticSnapshot { + document, + outcome: outcome.clone(), + }, + ); + outcome + } + + pub(super) fn reserve_document_diagnostic_snapshot(&mut self, uri: &str) -> bool { + if self.diagnostics.document_snapshots.contains_key(uri) + || self.diagnostics.document_snapshots.len() < MAX_WORKSPACE_DIAGNOSTIC_RESULTS + { + return true; + } + // Open documents retain their current report. Unopened pull results + // share the existing bounded result policy and can be recomputed after + // eviction; an active completion must keep its publication retry. + let evicted = self + .diagnostics + .document_snapshots + .keys() + .find(|candidate| { + self.lifecycle.overlays.version(candidate).is_none() + && !self.diagnostics.active_refreshes.contains_key(*candidate) + }) + .cloned(); + let Some(evicted) = evicted else { + return false; + }; + self.diagnostics.document_snapshots.remove(&evicted); + self.diagnostics.published.remove(&evicted); + true + } + + pub(super) fn retire_diagnostic_refresh(&mut self, uri: &str) { + if let Some(pending) = self.diagnostics.active_refreshes.remove(uri) { + // Removing result ownership and requesting abort do not assert that + // accepted analyzer work has already stopped. + let _ = self.diagnostics.provider.retire_document_refresh( + &pending.document.root, + uri, + &pending.identity.operation_id, + ); + } + } + + pub(super) fn invalidate_document_diagnostics(&mut self, uri: &str) { + self.retire_diagnostic_refresh(uri); + self.diagnostics.document_snapshots.remove(uri); + if self.lifecycle.overlays.version(uri).is_none() { + self.diagnostics.published.remove(uri); + } + } + + pub(super) fn clear_document_diagnostics(&mut self) { + let documents: Vec<_> = self.diagnostics.active_refreshes.keys().cloned().collect(); + for uri in documents { + self.retire_diagnostic_refresh(&uri); + } + self.diagnostics.document_snapshots.clear(); + } +} + +fn diagnostic_snapshot_fits(snapshot: &GenerationDiagnostics) -> Result<(), &'static str> { + // Count every retained field against the existing diagnostic result byte + // bound, including fields that either presentation may omit. + let mut remaining = MAX_WORKSPACE_DIAGNOSTIC_BYTES; + for diagnostic in snapshot.upstream.iter().chain(&snapshot.tracedecay) { + let value = diagnostic_value( + diagnostic.clone(), + DiagnosticSerializationCapabilities::full_identity(), + ); + let bytes = serde_json::to_vec(&value).map_err(|_| "diagnostic-serialization-failed")?; + remaining = remaining + .checked_sub(bytes.len().saturating_add(diagnostic.uri.len())) + .ok_or("diagnostic-result-byte-capacity")?; + } + Ok(()) +} diff --git a/crates/tracedecay-lsp/src/protocol/diagnostics_controller.rs b/crates/tracedecay-lsp/src/protocol/diagnostics_controller.rs index fc66f5f5b0..d4f2712a9f 100644 --- a/crates/tracedecay-lsp/src/protocol/diagnostics_controller.rs +++ b/crates/tracedecay-lsp/src/protocol/diagnostics_controller.rs @@ -1,4 +1,7 @@ -use super::outbound_controller::PublicationTag; +use super::diagnostic_snapshots::{ + DiagnosticDocumentIdentity, PendingDiagnosticRefresh, RetainedDiagnosticSnapshot, +}; +use super::outbound_controller::{PublicationRejection, PublicationTag}; use super::workspace_diagnostics_controller::WorkspaceDiagnosticResultIdentity; use super::{ BTreeMap, ConnectionLocalRequestSequence, DaemonLspProtocolSession, DebouncedDiagnosticKind, @@ -27,13 +30,6 @@ pub(super) struct PublishedDiagnostic { pub(super) scope_set_digest: Option, } -#[derive(Clone)] -pub(super) struct PendingDiagnosticRefresh { - pub(super) identity: DiagnosticRefreshIdentity, - pub(super) overlay_version: i64, - pub(super) scope_digest: Option, - pub(super) scope_set_digest: Option, -} #[derive(Clone, Eq, PartialEq)] pub(super) struct NativeDiagnosticSnapshot { pub(super) version: i64, @@ -47,6 +43,7 @@ where pub(super) provider: D, pub(super) debounce: OverlayDiagnosticDebouncer, pub(super) published: BTreeMap, + pub(super) document_snapshots: BTreeMap, pub(super) native_upstream: BTreeMap, pub(super) cursor_native_mode: bool, pub(super) next_server_request_id: ConnectionLocalRequestSequence, @@ -67,6 +64,7 @@ where provider, debounce: OverlayDiagnosticDebouncer::default(), published: BTreeMap::new(), + document_snapshots: BTreeMap::new(), native_upstream: BTreeMap::new(), cursor_native_mode: false, next_server_request_id: ConnectionLocalRequestSequence::starting_at(1), @@ -234,6 +232,7 @@ where self.diagnostics .native_upstream .insert(uri.clone(), snapshot); + self.invalidate_document_diagnostics(&uri); if !self .diagnostics .debounce @@ -267,14 +266,24 @@ where self.poll_diagnostic_refresh(uri); let overlay = self.lifecycle.overlays.snapshot(uri); let root = self.document_root(uri)?; - let outcome = self - .diagnostics - .provider - .document_diagnostics(&root, uri, overlay.as_ref()); + let outcome = self.document_diagnostic_snapshot(&root, uri, overlay.as_ref()); let version = overlay.as_ref().map_or(0, |overlay| overlay.version); - let source_generation = diagnostic_source_generation(&outcome); - let _refresh_failure = - self.request_diagnostic_refresh(uri, version, overlay.as_ref(), source_generation); + if !matches!(&outcome, DiagnosticSnapshotOutcome::Ready { .. }) { + let source_generation = diagnostic_source_generation(&outcome); + if let Some(failure_class) = + self.request_diagnostic_refresh(uri, overlay.as_ref(), source_generation) + && (matches!(&outcome, DiagnosticSnapshotOutcome::Partial { coverage, .. } + if coverage == "refresh-required") + || matches!(&outcome, DiagnosticSnapshotOutcome::Refreshing(_))) + { + return Err(refresh_pending_failure( + None, + None, + None, + Some(failure_class), + )); + } + } let diagnostics = match outcome { DiagnosticSnapshotOutcome::Ready { diagnostics, .. } => diagnostics, DiagnosticSnapshotOutcome::Refreshing(refresh) => { @@ -303,31 +312,8 @@ where )); } }; + self.diagnostics.debounce.cancel(uri); let generation = diagnostics.generation; - if self - .diagnostics - .published - .get(uri) - .is_some_and(|published| { - published.version == version - && published.generation > generation - && published.scope_digest == root.scope_digest().cloned() - && published.scope_set_digest - == self - .lifecycle - .gateway - .workspace() - .scope_set_digest() - .cloned() - }) - { - return Err(refresh_pending_failure( - None, - None, - None, - Some("superseded-generation".to_owned()), - )); - } let result_id = diagnostic_result_id( self.lifecycle.gateway.workspace().scope_set_digest(), root.scope_digest(), @@ -358,22 +344,6 @@ where DiagnosticSerializationCapabilities::pull(self.lifecycle.gateway.capabilities()), )); } - if overlay.is_some() { - self.diagnostics.published.insert( - uri.to_owned(), - PublishedDiagnostic { - version, - generation, - scope_digest: root.scope_digest().cloned(), - scope_set_digest: self - .lifecycle - .gateway - .workspace() - .scope_set_digest() - .cloned(), - }, - ); - } Ok(value) } @@ -395,12 +365,15 @@ where .get(&scheduled.uri) .map_or(0, |published| published.generation); self.discard_document_publications(&scheduled.uri); - if self.publish_diagnostics( - &scheduled.uri, - scheduled.version, - generation, - Vec::new(), - ) { + if self + .publish_diagnostics( + &scheduled.uri, + scheduled.version, + generation, + Vec::new(), + ) + .is_ok() + { self.diagnostics.published.remove(&scheduled.uri); self.queue_diagnostic_refresh(); } @@ -410,24 +383,21 @@ where let Ok(root) = self.document_root(&scheduled.uri) else { continue; }; - let outcome = self.diagnostics.provider.document_diagnostics( - &root, - &scheduled.uri, - overlay.as_ref(), - ); - let source_generation = diagnostic_source_generation(&outcome); - let _ = self.request_diagnostic_refresh( - &scheduled.uri, - scheduled.version, - overlay.as_ref(), - source_generation, - ); + let outcome = + self.document_diagnostic_snapshot(&root, &scheduled.uri, overlay.as_ref()); if let DiagnosticSnapshotOutcome::Ready { diagnostics, .. } = outcome { let _ = self.publish_complete_snapshot( &scheduled.uri, scheduled.version, diagnostics, ); + } else { + let source_generation = diagnostic_source_generation(&outcome); + let _ = self.request_diagnostic_refresh( + &scheduled.uri, + overlay.as_ref(), + source_generation, + ); } } } @@ -437,21 +407,29 @@ where pub(super) fn request_diagnostic_refresh( &mut self, uri: &str, - version: i64, overlay: Option<&crate::overlay::OverlaySnapshot>, source_generation: Option, ) -> Option { + let Ok(root) = self.document_root(uri) else { + return Some("document-root-unavailable".to_owned()); + }; + let document = DiagnosticDocumentIdentity::new( + &root, + self.lifecycle.gateway.workspace().scope_set_digest(), + overlay, + ); if self .diagnostics .active_refreshes .get(uri) - .is_some_and(|pending| pending.overlay_version == version) + .is_some_and(|pending| pending.document == document) { return None; } - let Ok(root) = self.document_root(uri) else { - return Some("document-root-unavailable".to_owned()); - }; + self.retire_diagnostic_refresh(uri); + if !self.reserve_document_diagnostic_snapshot(uri) { + return Some("diagnostic-result-capacity".to_owned()); + } match self.diagnostics.provider.request_document_refresh( &root, uri, @@ -461,21 +439,18 @@ where DiagnosticRefreshAdmission::Started(identity) | DiagnosticRefreshAdmission::AlreadyRunning(identity) => { if !valid_refresh_identity(&identity, source_generation) { + let _ = self.diagnostics.provider.retire_document_refresh( + &root, + uri, + &identity.operation_id, + ); return Some("invalid-refresh-identity".to_owned()); } + self.diagnostics.document_snapshots.remove(uri); + self.diagnostics.debounce.cancel(uri); self.diagnostics.active_refreshes.insert( uri.to_owned(), - PendingDiagnosticRefresh { - identity, - overlay_version: version, - scope_digest: root.scope_digest().cloned(), - scope_set_digest: self - .lifecycle - .gateway - .workspace() - .scope_set_digest() - .cloned(), - }, + PendingDiagnosticRefresh { identity, document }, ); None } @@ -498,63 +473,38 @@ where let Some(pending) = self.diagnostics.active_refreshes.get(uri).cloned() else { return; }; - let version = self.lifecycle.overlays.version(uri).unwrap_or_default(); - if version != pending.overlay_version { - self.diagnostics.active_refreshes.remove(uri); - return; - } let overlay = self.lifecycle.overlays.snapshot(uri); + let version = overlay.as_ref().map_or(0, |overlay| overlay.version); let Ok(root) = self.document_root(uri) else { - self.diagnostics.active_refreshes.remove(uri); + self.invalidate_document_diagnostics(uri); return; }; - if pending.scope_digest != root.scope_digest().cloned() - || pending.scope_set_digest - != self - .lifecycle - .gateway - .workspace() - .scope_set_digest() - .cloned() + if pending.document + != DiagnosticDocumentIdentity::new( + &root, + self.lifecycle.gateway.workspace().scope_set_digest(), + overlay.as_ref(), + ) { - self.diagnostics.active_refreshes.remove(uri); + self.invalidate_document_diagnostics(uri); return; } - match self - .diagnostics - .provider - .document_diagnostics(&root, uri, overlay.as_ref()) - { + match self.document_diagnostic_snapshot(&root, uri, overlay.as_ref()) { DiagnosticSnapshotOutcome::Ready { diagnostics, completed_operation_id, } if completed_operation_id.as_deref() == Some(pending.identity.operation_id.as_str()) => { - let generation = diagnostics.generation; - let target_matches = pending - .identity - .target_generation - .is_none_or(|target| target == generation); - let source_not_superseded = pending - .identity - .source_generation - .is_none_or(|source| generation >= source); - let publication_not_superseded = - self.diagnostics.published.get(uri).is_none_or(|published| { - published.version != version || published.generation <= generation - }); - if target_matches && source_not_superseded && publication_not_superseded { - if self.publish_complete_snapshot(uri, version, diagnostics) { - self.diagnostics.active_refreshes.remove(uri); - } - } else { - self.diagnostics.active_refreshes.remove(uri); + if self.publish_complete_snapshot(uri, version, diagnostics) + != Err(PublicationRejection::Backpressured) + { + self.retire_diagnostic_refresh(uri); } } DiagnosticSnapshotOutcome::Partial { .. } | DiagnosticSnapshotOutcome::Failed { .. } => { - self.diagnostics.active_refreshes.remove(uri); + self.retire_diagnostic_refresh(uri); self.queue_diagnostic_refresh(); } DiagnosticSnapshotOutcome::Ready { .. } | DiagnosticSnapshotOutcome::Refreshing(_) => {} @@ -566,7 +516,7 @@ where uri: &str, version: i64, snapshot: crate::provider::GenerationDiagnostics, - ) -> bool { + ) -> Result<(), PublicationRejection> { let generation = snapshot.generation; if self .diagnostics @@ -576,7 +526,7 @@ where published.version == version && published.generation > generation }) { - return false; + return Err(PublicationRejection::Discarded); } let merged = self.merge_document_diagnostics( uri, @@ -587,19 +537,23 @@ where .capabilities() .publish_diagnostics_data, ); - if self + let publication = if self .lifecycle .gateway .capabilities() .supports_publish_diagnostics - && !self.publish_diagnostics( + { + self.publish_diagnostics( uri, version, generation, self.scope_diagnostic_related_information(merged.items), ) - { - return false; + } else { + Ok(()) + }; + if publication == Err(PublicationRejection::Backpressured) { + return publication; } self.diagnostics.published.insert( uri.to_owned(), @@ -619,7 +573,7 @@ where }, ); self.queue_diagnostic_refresh(); - true + publication } #[tracing::instrument(name = "lsp_diagnostics_merge_document", level = "trace", skip_all)] @@ -665,14 +619,14 @@ where version: i64, generation: u64, diagnostics: Vec, - ) -> bool { + ) -> Result<(), PublicationRejection> { if !self .lifecycle .gateway .capabilities() .supports_publish_diagnostics { - return false; + return Err(PublicationRejection::Discarded); } let capabilities = self.lifecycle.gateway.capabilities(); let mut params = json!({ diff --git a/crates/tracedecay-lsp/src/protocol/lifecycle_controller.rs b/crates/tracedecay-lsp/src/protocol/lifecycle_controller.rs index 77d7ca74d2..4b2b8b2a38 100644 --- a/crates/tracedecay-lsp/src/protocol/lifecycle_controller.rs +++ b/crates/tracedecay-lsp/src/protocol/lifecycle_controller.rs @@ -264,6 +264,7 @@ where } } } + self.clear_document_diagnostics(); self.lifecycle.overlays.clear(); self.diagnostics.debounce.clear(); self.outbound.queue.clear(); @@ -274,7 +275,6 @@ where self.diagnostics.cursor_native_mode = false; self.diagnostics.refresh_request = None; self.diagnostics.refresh_needed = false; - self.diagnostics.active_refreshes.clear(); self.diagnostics.workspace_results.clear(); self.diagnostics.workspace_snapshots.clear(); self.diagnostics.workspace_failures.clear(); @@ -292,6 +292,17 @@ where pub(super) fn clear_removed_workspace_root_state(&mut self, removed: &[AdmittedRoot]) { let belongs_to_removed_root = |uri: &str| removed.iter().any(|root| root.contains_document(uri)); + let diagnostic_documents = self + .diagnostics + .active_refreshes + .keys() + .chain(self.diagnostics.document_snapshots.keys()) + .filter(|uri| belongs_to_removed_root(uri)) + .cloned() + .collect::>(); + for uri in diagnostic_documents { + self.invalidate_document_diagnostics(&uri); + } let request_ids = self .semantic .pending @@ -327,9 +338,6 @@ where self.diagnostics .native_upstream .retain(|uri, _| !belongs_to_removed_root(uri)); - self.diagnostics - .active_refreshes - .retain(|uri, _| !belongs_to_removed_root(uri)); self.diagnostics .workspace_results .retain(|uri, _| !belongs_to_removed_root(uri)); @@ -449,6 +457,9 @@ where self.cancel_pending_operations(); match self.lifecycle.control.shutdown() { Ok(()) => { + self.clear_document_diagnostics(); + self.diagnostics.published.clear(); + self.diagnostics.debounce.clear(); let _ = self.enqueue_value(success_response(response_id, Value::Null)); } Err(_) => { diff --git a/crates/tracedecay-lsp/src/protocol/outbound_controller.rs b/crates/tracedecay-lsp/src/protocol/outbound_controller.rs index 21edca37a3..b8666fb0c0 100644 --- a/crates/tracedecay-lsp/src/protocol/outbound_controller.rs +++ b/crates/tracedecay-lsp/src/protocol/outbound_controller.rs @@ -23,6 +23,12 @@ pub(super) struct PublicationTag { pub(super) generation: u64, } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(super) enum PublicationRejection { + Discarded, + Backpressured, +} + #[derive(Clone, Debug, Eq, PartialEq)] pub(super) struct QueuedFrame { pub(super) payload: LspFrame, @@ -181,16 +187,18 @@ where true } - pub(super) fn enqueue_publication(&mut self, value: Value, tag: PublicationTag) -> bool { + pub(super) fn enqueue_publication( + &mut self, + value: Value, + tag: PublicationTag, + ) -> Result<(), PublicationRejection> { let Ok(payload) = serde_json::to_vec(&value) else { - return false; + return Err(PublicationRejection::Discarded); }; if payload.len() > MAX_PUBLICATION_BYTES { - return false; - } - if self.publication_replacement(&payload, &tag).is_err() { - return false; + return Err(PublicationRejection::Discarded); } + self.publication_replacement(&payload, &tag)?; match self.lifecycle.control.admit_payload_publication( tag.uri.clone(), tag.version, @@ -198,12 +206,18 @@ where &payload, ) { PublicationAdmission::Accepted => {} - PublicationAdmission::Duplicate | PublicationAdmission::Stale => return false, - PublicationAdmission::TooLarge { .. } | PublicationAdmission::SessionUnavailable => { - return false; + PublicationAdmission::Duplicate + | PublicationAdmission::Stale + | PublicationAdmission::TooLarge { .. } + | PublicationAdmission::SessionUnavailable => { + return Err(PublicationRejection::Discarded); } } - self.enqueue_frame(payload, Some(tag), None) + if self.enqueue_frame(payload, Some(tag), None) { + Ok(()) + } else { + Err(PublicationRejection::Backpressured) + } } pub(super) fn enqueue_frame( @@ -258,7 +272,7 @@ where &self, payload: &[u8], tag: &PublicationTag, - ) -> Result, ()> { + ) -> Result, PublicationRejection> { let replacement = self .outbound .queue @@ -276,12 +290,15 @@ where .and_then(|index| self.outbound.queue.get(index)) .map_or(0, |frame| frame.payload.len()); if let Some(index) = replacement { - let existing = self.outbound.queue[index].publication.as_ref().ok_or(())?; + let existing = self.outbound.queue[index] + .publication + .as_ref() + .ok_or(PublicationRejection::Discarded)?; if (tag.version, tag.generation) < (existing.version, existing.generation) || ((tag.version, tag.generation) == (existing.version, existing.generation) && self.outbound.queue[index].payload == payload) { - return Err(()); + return Err(PublicationRejection::Discarded); } } let projected_messages = self.outbound.queue.len() + usize::from(replacement.is_none()); @@ -293,14 +310,14 @@ where if projected_messages > MAX_ORDINARY_OUTBOUND_MESSAGES || projected_bytes > MAX_ORDINARY_OUTBOUND_BYTES { - return Err(()); + return Err(PublicationRejection::Backpressured); } Ok(replacement) } pub(super) fn discard_document_publications(&mut self, uri: &str) { self.discard_document_context(uri); - self.diagnostics.active_refreshes.remove(uri); + self.invalidate_document_diagnostics(uri); let mut retained = VecDeque::with_capacity(self.outbound.queue.len()); let mut index = 0_usize; while let Some(frame) = self.outbound.queue.pop_front() { diff --git a/crates/tracedecay-lsp/src/protocol/tests.rs b/crates/tracedecay-lsp/src/protocol/tests.rs index 960e44f6fa..04bb61446e 100644 --- a/crates/tracedecay-lsp/src/protocol/tests.rs +++ b/crates/tracedecay-lsp/src/protocol/tests.rs @@ -18,6 +18,7 @@ use tracedecay_domain::{ContentDigest, ManifestDigest}; use tracedecay_tool_catalog::{BindingSurface, CatalogContributionInputV1, CatalogContributionV1}; mod diagnostic_publication; +mod diagnostic_snapshots; mod semantic_lifecycle; mod workspace_diagnostics; diff --git a/crates/tracedecay-lsp/src/protocol/tests/diagnostic_publication.rs b/crates/tracedecay-lsp/src/protocol/tests/diagnostic_publication.rs index 687290d789..c0c382c11a 100644 --- a/crates/tracedecay-lsp/src/protocol/tests/diagnostic_publication.rs +++ b/crates/tracedecay-lsp/src/protocol/tests/diagnostic_publication.rs @@ -164,7 +164,11 @@ fn assert_diagnostic_delivery( .collect::>(); assert!(messages.windows(2).all(|pair| pair[0] < pair[1])); }; - assert!(session.publish_complete_snapshot("file:///root/a.rs", 1, snapshot.clone())); + assert!( + session + .publish_complete_snapshot("file:///root/a.rs", 1, snapshot.clone()) + .is_ok() + ); let publication: Value = session .drain_outbound() .iter() diff --git a/crates/tracedecay-lsp/src/protocol/tests/diagnostic_snapshots.rs b/crates/tracedecay-lsp/src/protocol/tests/diagnostic_snapshots.rs new file mode 100644 index 0000000000..f98141356e --- /dev/null +++ b/crates/tracedecay-lsp/src/protocol/tests/diagnostic_snapshots.rs @@ -0,0 +1,826 @@ +use std::sync::atomic::{AtomicBool, AtomicU64, AtomicUsize, Ordering}; +use std::sync::{Arc, Mutex}; +use std::task::{Context, Poll, Waker}; + +use serde_json::{Value, json}; +use tracedecay_domain::test_fixtures::digest; + +use super::{Feedback, Semantics}; +use crate::analyzer::broker::DiagnosticSeverity; +use crate::capabilities::{ + ClientCapabilities, GatewayCapabilities, UpstreamCapabilities, negotiate_capabilities, +}; +use crate::diagnostics::{DiagnosticSource, GatewayDiagnostic, LspPosition, LspRange}; +use crate::gateway::{ + AdmittedRoot, LspRuntimeFailure, LspRuntimeFuture, LspRuntimeSpawner, LspRuntimeTask, +}; +use crate::overlay::{ + CanonicalDiagnosticRefreshRequest, CanonicalDiagnosticSnapshotAuthority, + DiagnosticSnapshotAdapter, OverlaySnapshot, +}; +use crate::protocol::{DaemonLspProtocolSession, MAX_QUEUED_OUTBOUND_MESSAGES}; +use crate::provider::{ + DiagnosticRefreshAdmission, DiagnosticSnapshotOutcome, DiagnosticSnapshotPort, + GenerationDiagnostics, +}; +use crate::session::AuthorizedLspWorkspace; +use crate::workspace_diagnostics::{ + MAX_WORKSPACE_DIAGNOSTIC_BYTES, MAX_WORKSPACE_DIAGNOSTIC_RESULTS, +}; + +const DOCUMENT: &str = "file:///root/a.rs"; + +#[derive(Default)] +struct ControlledRuntime(Mutex>>); + +struct ControlledTask(Mutex>>); + +impl LspRuntimeTask for Arc { + // Accepted work can finish after an advisory abort. Keep the future so a + // test can explicitly deliver that late completion to the retired channel. + fn abort(&self) {} +} + +impl LspRuntimeSpawner for ControlledRuntime { + fn spawn(&self, future: LspRuntimeFuture<()>) -> Box { + let task = Arc::new(ControlledTask(Mutex::new(Some(future)))); + self.0.lock().unwrap().push(Arc::clone(&task)); + Box::new(task) + } +} + +impl ControlledRuntime { + fn complete_next(&self) { + let tasks = self.0.lock().unwrap(); + let mut future = tasks + .iter() + .find_map(|task| task.0.lock().unwrap().take()) + .expect("an admitted diagnostic computation must be waiting"); + assert_eq!( + future + .as_mut() + .poll(&mut Context::from_waker(Waker::noop())), + Poll::Ready(()), + ); + } + + fn started(&self) -> usize { + self.0.lock().unwrap().len() + } +} + +#[derive(Default)] +struct DiagnosticAuthority { + generation: AtomicU64, + next_generation: Mutex>, + next_failure: Mutex>, + next_code: Mutex>, +} + +impl CanonicalDiagnosticSnapshotAuthority for DiagnosticAuthority { + fn refresh( + &self, + request: CanonicalDiagnosticRefreshRequest, + ) -> LspRuntimeFuture> { + let next_generation = self.generation.fetch_add(1, Ordering::SeqCst) + 1; + let generation = self + .next_generation + .lock() + .unwrap() + .take() + .unwrap_or(next_generation); + let failure = self.next_failure.lock().unwrap().take(); + let code = self + .next_code + .lock() + .unwrap() + .take() + .unwrap_or_else(|| "retained-snapshot".to_owned()); + let version = request + .overlay + .as_ref() + .map_or(0, |overlay| overlay.version); + Box::pin(async move { + if let Some(failure) = failure { + return Err(LspRuntimeFailure::new(failure)); + } + Ok(GenerationDiagnostics { + generation, + authority_digest: digest('a'), + upstream: vec![GatewayDiagnostic { + uri: request.document_uri, + range: LspRange { + start: LspPosition { + line: 0, + character: 0, + }, + end: LspPosition { + line: 0, + character: 1, + }, + }, + severity: Some(DiagnosticSeverity::Warning), + code: Some(code), + code_description_uri: None, + message: format!("generation {generation}, version {version}"), + source: DiagnosticSource::Upstream, + related_information: Vec::new(), + data: None, + }], + tracedecay: Vec::new(), + }) + }) + } +} + +// This wrapper only selects a scheduler interleaving. All operation admission, +// completion ownership, and retirement still belong to the real adapter. +struct ControlledDiagnosticPort { + adapter: DiagnosticSnapshotAdapter, + runtime: Arc, + complete_after_read: AtomicBool, + reads: AtomicUsize, +} + +impl DiagnosticSnapshotPort for ControlledDiagnosticPort { + fn document_diagnostics( + &self, + root: &AdmittedRoot, + document_uri: &str, + overlay: Option<&OverlaySnapshot>, + ) -> DiagnosticSnapshotOutcome { + self.reads.fetch_add(1, Ordering::SeqCst); + let outcome = self + .adapter + .document_diagnostics(root, document_uri, overlay); + if matches!(outcome, DiagnosticSnapshotOutcome::Refreshing(_)) + && self.complete_after_read.swap(false, Ordering::SeqCst) + { + self.runtime.complete_next(); + } + outcome + } + + fn request_document_refresh( + &self, + root: &AdmittedRoot, + document_uri: &str, + overlay: Option<&OverlaySnapshot>, + source_generation: Option, + ) -> DiagnosticRefreshAdmission { + self.adapter + .request_document_refresh(root, document_uri, overlay, source_generation) + } + + fn retire_document_refresh( + &self, + root: &AdmittedRoot, + document_uri: &str, + operation_id: &str, + ) -> bool { + self.adapter + .retire_document_refresh(root, document_uri, operation_id) + } +} + +struct Harness { + session: DaemonLspProtocolSession, + runtime: Arc, + authority: Arc, + now_ms: u64, + next_request_id: u64, +} + +impl Harness { + fn new(push: bool) -> Self { + let runtime = Arc::new(ControlledRuntime::default()); + let authority = Arc::new(DiagnosticAuthority::default()); + let provider = ControlledDiagnosticPort { + adapter: DiagnosticSnapshotAdapter::new(runtime.clone(), authority.clone()), + runtime: runtime.clone(), + complete_after_read: AtomicBool::new(false), + reads: AtomicUsize::new(0), + }; + let capabilities = GatewayCapabilities { + supports_workspace_folders: true, + ..GatewayCapabilities::default() + }; + let upstream = UpstreamCapabilities { + supports_diagnostics: true, + ..UpstreamCapabilities::default() + }; + let workspace = AuthorizedLspWorkspace::new( + Some(digest('c')), + vec![AdmittedRoot::authorized("file:///root", digest('b'))], + ) + .unwrap(); + let session = DaemonLspProtocolSession::from_workspace_ports( + workspace, + negotiate_capabilities(&ClientCapabilities::default(), &capabilities, &upstream), + capabilities, + upstream, + Feedback::default(), + Semantics, + provider, + ); + let mut harness = Self { + session, + runtime, + authority, + now_ms: 0, + next_request_id: 1, + }; + let mut capabilities = json!({ + "general": { "positionEncodings": ["utf-16"] }, + "textDocument": { "diagnostic": {} }, + "workspace": { "workspaceFolders": true }, + }); + if push { + capabilities["textDocument"]["publishDiagnostics"] = json!({ "versionSupport": true }); + } + let initialized = harness.request( + "initialize", + json!({ "rootUri": "file:///root", "capabilities": capabilities }), + ); + assert!(initialized.get("error").is_none(), "{initialized}"); + harness.notify("initialized", json!({})); + assert_eq!( + harness + .session + .lifecycle + .gateway + .capabilities() + .supports_publish_diagnostics, + push, + ); + harness + } + + fn send(&mut self, value: Value) -> Vec { + self.now_ms += 1; + self.session + .handle_payload(&serde_json::to_vec(&value).unwrap(), self.now_ms); + self.messages() + } + + fn messages(&mut self) -> Vec { + self.session + .drain_outbound() + .iter() + .map(|message| serde_json::from_slice(message).unwrap()) + .collect() + } + + fn request(&mut self, method: &str, params: Value) -> Value { + let id = self.next_request_id; + self.next_request_id += 1; + self.send(json!({ "jsonrpc": "2.0", "id": id, "method": method, "params": params })) + .into_iter() + .find(|message| message["id"] == id) + .expect("the protocol must answer the request") + } + + fn notify(&mut self, method: &str, params: Value) -> Vec { + self.send(json!({ "jsonrpc": "2.0", "method": method, "params": params })) + } + + fn open(&mut self, version: i64) { + self.notify("textDocument/didOpen", json!({ + "textDocument": { "uri": DOCUMENT, "languageId": "rust", "version": version, "text": "fn a() {}" }, + })); + assert_eq!(self.session.overlays().version(DOCUMENT), Some(version)); + } + + fn start_refresh(&mut self) { + self.now_ms += crate::overlay::OVERLAY_DIAGNOSTIC_DEBOUNCE_MS; + self.session.flush_due(self.now_ms); + self.messages(); + } + + fn pull(&mut self, previous_result: Option<&str>) -> Value { + self.pull_document(DOCUMENT, previous_result) + } + + fn pull_document(&mut self, uri: &str, previous_result: Option<&str>) -> Value { + let mut params = json!({ "textDocument": { "uri": uri } }); + if let Some(previous) = previous_result { + params["previousResultId"] = json!(previous); + } + self.request("textDocument/diagnostic", params) + } +} + +fn assert_full(response: &Value, generation: u64, version: i64) -> String { + assert_eq!(response["result"]["kind"], "full", "{response}"); + assert_eq!( + response["result"]["items"], + expected_items(generation, version) + ); + response["result"]["resultId"].as_str().unwrap().to_owned() +} + +fn expected_items(generation: u64, version: i64) -> Value { + json!([{ + "range": { "start": { "line": 0, "character": 0 }, "end": { "line": 0, "character": 1 } }, + "severity": 2, + "code": "retained-snapshot", + "source": "upstream", + "message": format!("generation {generation}, version {version}"), + }]) +} + +#[test] +fn completed_refresh_is_a_repeatable_document_snapshot() { + let mut harness = Harness::new(true); + harness.open(1); + harness.start_refresh(); + assert_eq!(harness.runtime.started(), 1); + harness.runtime.complete_next(); + + let result_id = assert_full(&harness.pull(None), 1, 1); + let repeated = harness.pull(Some(&result_id)); + assert_eq!( + repeated["result"], + json!({ "kind": "unchanged", "resultId": result_id }) + ); + assert_eq!( + harness.runtime.started(), + 1, + "reading a fresh snapshot must not restart computation" + ); +} + +#[test] +fn background_publication_preserves_the_snapshot_for_pull() { + let mut harness = Harness::new(true); + harness.open(1); + harness.start_refresh(); + harness.runtime.complete_next(); + harness.session.flush_due(harness.now_ms); + let messages = harness.messages(); + let publication = messages + .iter() + .find(|message| message["method"] == "textDocument/publishDiagnostics") + .expect("completed refresh must publish its diagnostics"); + assert_eq!(publication["params"]["diagnostics"], expected_items(1, 1)); + + assert_full(&harness.pull(None), 1, 1); + assert_eq!(harness.runtime.started(), 1); +} + +#[test] +fn pull_only_clients_keep_completions_between_protocol_ticks() { + let mut harness = Harness::new(false); + harness.open(1); + harness.start_refresh(); + harness.runtime.complete_next(); + harness.session.flush_due(harness.now_ms); + assert!(harness.messages().is_empty(), "push is not negotiated"); + + let result_id = assert_full(&harness.pull(None), 1, 1); + assert_eq!( + harness.pull(Some(&result_id))["result"]["kind"], + "unchanged" + ); + assert_eq!(harness.runtime.started(), 1); +} + +#[test] +fn a_pending_pull_becomes_full_after_its_admitted_work_completes() { + let mut harness = Harness::new(false); + harness.open(1); + let pending = harness.pull(None); + assert!(pending.get("error").is_some(), "{pending}"); + assert!(pending.get("result").is_none()); + assert_eq!(harness.runtime.started(), 1); + + harness.runtime.complete_next(); + assert_full(&harness.pull(None), 1, 1); + harness.start_refresh(); + assert_eq!(harness.runtime.started(), 1); +} + +#[test] +fn completion_between_reads_is_retained_for_later_pulls() { + let mut harness = Harness::new(false); + harness.open(1); + harness.start_refresh(); + harness + .session + .diagnostics + .provider + .complete_after_read + .store(true, Ordering::SeqCst); + + let first = harness.pull(None); + assert!( + !harness + .session + .diagnostics + .provider + .complete_after_read + .load(Ordering::SeqCst) + ); + let full = if first.get("error").is_some() { + assert_eq!(first["error"]["code"], -32802); + harness.pull(None) + } else { + first + }; + let result_id = assert_full(&full, 1, 1); + assert_eq!( + harness.pull(Some(&result_id))["result"]["kind"], + "unchanged" + ); + assert_eq!(harness.runtime.started(), 1); +} + +#[test] +fn content_changes_invalidate_the_retained_document_snapshot() { + let mut harness = Harness::new(false); + harness.open(1); + harness.start_refresh(); + harness.runtime.complete_next(); + let old_result = assert_full(&harness.pull(None), 1, 1); + + harness.notify( + "textDocument/didChange", + json!({ + "textDocument": { "uri": DOCUMENT, "version": 2 }, + "contentChanges": [{ "text": "fn changed() {}" }], + }), + ); + let pending = harness.pull(Some(&old_result)); + assert_eq!(pending["error"]["code"], -32802, "{pending}"); + assert!(pending.get("result").is_none()); + harness.runtime.complete_next(); + let new_result = assert_full(&harness.pull(Some(&old_result)), 2, 2); + assert_ne!(old_result, new_result); + assert_eq!( + harness.pull(Some(&new_result))["result"]["kind"], + "unchanged" + ); + assert_eq!(harness.runtime.started(), 2); +} + +#[test] +fn save_refreshes_a_snapshot_even_when_the_overlay_version_is_unchanged() { + let mut harness = Harness::new(false); + harness.open(1); + harness.start_refresh(); + harness.runtime.complete_next(); + let old_result = assert_full(&harness.pull(None), 1, 1); + + harness.notify( + "textDocument/didSave", + json!({ "textDocument": { "uri": DOCUMENT } }), + ); + let pending = harness.pull(Some(&old_result)); + assert_eq!(pending["error"]["code"], -32802, "{pending}"); + harness.runtime.complete_next(); + let new_result = assert_full(&harness.pull(Some(&old_result)), 2, 1); + assert_ne!(old_result, new_result); + assert_eq!(harness.runtime.started(), 2); +} + +#[test] +fn late_completion_cannot_answer_a_reopened_document_lifetime() { + let mut harness = Harness::new(false); + harness.open(1); + harness.start_refresh(); + harness.notify( + "textDocument/didClose", + json!({ "textDocument": { "uri": DOCUMENT } }), + ); + harness.open(1); + assert_eq!(harness.pull(None)["error"]["code"], -32802); + assert_eq!(harness.runtime.started(), 2); + + harness.runtime.complete_next(); + let still_pending = harness.pull(None); + assert_eq!(still_pending["error"]["code"], -32802, "{still_pending}"); + assert!(still_pending.get("result").is_none()); + assert_eq!( + harness.runtime.started(), + 2, + "the successor must keep its operation" + ); + harness.runtime.complete_next(); + assert_full(&harness.pull(None), 2, 1); + assert_eq!(harness.runtime.started(), 2); +} + +#[test] +fn workspace_scope_changes_invalidate_retained_document_evidence() { + let mut harness = Harness::new(false); + harness.open(1); + harness.start_refresh(); + harness.runtime.complete_next(); + let old_result = assert_full(&harness.pull(None), 1, 1); + + harness.notify( + "workspace/didChangeWorkspaceFolders", + json!({ + "event": { "added": [{ "uri": "file:///other", "name": "other" }], "removed": [] }, + }), + ); + let mutation = harness.session.pending_workspace_folder_mutation().unwrap(); + let workspace = AuthorizedLspWorkspace::new( + Some(digest('d')), + vec![ + AdmittedRoot::authorized("file:///root", digest('b')), + AdmittedRoot::authorized("file:///other", digest('e')), + ], + ) + .unwrap(); + harness + .session + .apply_workspace_folder_mutation(&mutation, workspace) + .unwrap(); + let pending = harness.pull(Some(&old_result)); + assert_eq!(pending["error"]["code"], -32802, "{pending}"); + harness.runtime.complete_next(); + let new_result = assert_full(&harness.pull(Some(&old_result)), 2, 1); + assert_ne!(old_result, new_result); + assert_eq!(harness.runtime.started(), 2); +} + +#[test] +fn unopened_documents_have_repeatable_completed_reports() { + let mut harness = Harness::new(false); + assert_eq!(harness.pull(None)["error"]["code"], -32802); + harness.runtime.complete_next(); + let result_id = assert_full(&harness.pull(None), 1, 0); + assert_eq!( + harness.pull(Some(&result_id))["result"]["kind"], + "unchanged" + ); + assert_eq!(harness.runtime.started(), 1); +} + +#[test] +fn unopened_document_snapshots_reuse_the_existing_result_bound() { + let mut harness = Harness::new(false); + let mut latest = None; + for index in 0..=MAX_WORKSPACE_DIAGNOSTIC_RESULTS { + let uri = format!("file:///root/closed-{index}.rs"); + assert_eq!(harness.pull_document(&uri, None)["error"]["code"], -32802); + harness.runtime.complete_next(); + let result_id = assert_full(&harness.pull_document(&uri, None), index as u64 + 1, 0); + assert!( + harness.session.diagnostics.document_snapshots.len() + <= MAX_WORKSPACE_DIAGNOSTIC_RESULTS + ); + assert!(harness.session.diagnostics.published.len() <= MAX_WORKSPACE_DIAGNOSTIC_RESULTS); + latest = Some((uri, result_id)); + } + let (uri, result_id) = latest.unwrap(); + assert_eq!( + harness.pull_document(&uri, Some(&result_id))["result"]["kind"], + "unchanged" + ); + assert_eq!( + harness.runtime.started(), + MAX_WORKSPACE_DIAGNOSTIC_RESULTS + 1 + ); +} + +fn assert_incomplete_refresh_replaces_old_snapshot(failure: &'static str, field: &str) { + let mut harness = Harness::new(false); + harness.open(1); + harness.start_refresh(); + harness.runtime.complete_next(); + let old_result = assert_full(&harness.pull(None), 1, 1); + + *harness.authority.next_failure.lock().unwrap() = Some(failure); + harness.notify( + "textDocument/didSave", + json!({ "textDocument": { "uri": DOCUMENT } }), + ); + harness.runtime.complete_next(); + let failed = harness.pull(Some(&old_result)); + assert_eq!(failed["error"]["code"], -32802, "{failed}"); + assert_eq!(failed["error"]["data"][field], failure); + assert!(failed.get("result").is_none()); + let repeated = harness.pull(Some(&old_result)); + assert_eq!(repeated["error"]["code"], -32802, "{repeated}"); + assert!(repeated["error"]["data"]["operationId"].as_str().is_some()); + assert!(repeated.get("result").is_none()); + assert_eq!( + harness.runtime.started(), + 3, + "the observed failure admits one coalesced retry" + ); + harness.runtime.complete_next(); + let result_id = assert_full(&harness.pull(Some(&old_result)), 3, 1); + assert_eq!( + harness.pull(Some(&result_id))["result"]["kind"], + "unchanged" + ); + assert_eq!(harness.runtime.started(), 3); +} + +#[test] +fn partial_refreshes_do_not_restore_the_previous_complete_snapshot() { + assert_incomplete_refresh_replaces_old_snapshot( + "diagnostic-broker-coverage-incomplete", + "coverage", + ); +} + +#[test] +fn failed_refreshes_do_not_restore_the_previous_complete_snapshot() { + assert_incomplete_refresh_replaces_old_snapshot("provider-unavailable", "failureClass"); +} + +#[test] +fn publication_backpressure_does_not_discard_a_completed_snapshot() { + let mut harness = Harness::new(true); + harness.open(1); + harness.start_refresh(); + harness.runtime.complete_next(); + for _ in 0..MAX_QUEUED_OUTBOUND_MESSAGES { + if !harness.session.enqueue_value(json!({ + "jsonrpc": "2.0", "method": "window/logMessage", + "params": { "type": 3, "message": "transport occupied" }, + })) { + break; + } + } + assert!( + !harness + .session + .has_outbound_capacity(crate::MAX_PUBLICATION_BYTES) + ); + harness.session.poll_diagnostic_refresh(DOCUMENT); + harness.messages(); + + assert_full(&harness.pull(None), 1, 1); + assert_eq!(harness.runtime.started(), 1); +} + +#[test] +fn shutdown_releases_ready_snapshots_and_retires_late_completions() { + let mut harness = Harness::new(false); + harness.open(1); + harness.start_refresh(); + harness.runtime.complete_next(); + assert_full(&harness.pull(None), 1, 1); + + let pending_uri = "file:///root/pending.rs"; + let root = harness.session.root().clone(); + assert_eq!( + harness.pull_document(pending_uri, None)["error"]["code"], + -32802 + ); + assert_eq!(harness.session.diagnostics.document_snapshots.len(), 1); + assert_eq!(harness.session.diagnostics.active_refreshes.len(), 1); + let shutdown = harness.request("shutdown", Value::Null); + assert!(shutdown.get("error").is_none(), "{shutdown}"); + assert_eq!(shutdown.get("result"), Some(&Value::Null)); + assert!(harness.session.diagnostics.document_snapshots.is_empty()); + assert!(harness.session.diagnostics.active_refreshes.is_empty()); + assert!(harness.session.diagnostics.published.is_empty()); + + harness.runtime.complete_next(); + harness.session.flush_due(harness.now_ms); + assert!(harness.messages().is_empty()); + assert!(matches!( + harness.session.diagnostics.provider.adapter.document_diagnostics(&root, pending_uri, None), + DiagnosticSnapshotOutcome::Partial { coverage, .. } if coverage == "refresh-required" + )); + assert!(harness.session.diagnostics.document_snapshots.is_empty()); + assert_eq!(harness.runtime.started(), 2); +} + +#[test] +fn oversized_diagnostic_codes_refuse_then_recover_with_fresh_work() { + let mut harness = Harness::new(false); + harness.open(1); + harness.start_refresh(); + *harness.authority.next_code.lock().unwrap() = + Some("x".repeat(MAX_WORKSPACE_DIAGNOSTIC_BYTES + 1)); + harness.runtime.complete_next(); + + let refused = harness.pull(None); + assert_eq!(refused["error"]["code"], -32802, "{refused}"); + assert_eq!( + refused["error"]["data"]["failureClass"], + "diagnostic-result-byte-capacity", + ); + assert!(refused.get("result").is_none()); + let pending = harness.pull(None); + assert_eq!(pending["error"]["code"], -32802, "{pending}"); + assert!(pending["error"]["data"]["operationId"].as_str().is_some()); + assert!(pending.get("result").is_none()); + assert!(harness.session.diagnostics.published.is_empty()); + assert_eq!(harness.runtime.started(), 2); + harness.runtime.complete_next(); + let result_id = assert_full(&harness.pull(None), 2, 1); + assert_eq!( + harness.pull(Some(&result_id))["result"]["kind"], + "unchanged" + ); + assert_eq!(harness.runtime.started(), 2); +} + +fn assert_completed_refresh_is_not_polled_again(harness: &mut Harness) { + assert!(harness.session.diagnostics.active_refreshes.is_empty()); + let reads = harness + .session + .diagnostics + .provider + .reads + .load(Ordering::SeqCst); + harness.session.flush_due(harness.now_ms); + harness.session.flush_due(harness.now_ms + 1); + assert_eq!( + harness + .session + .diagnostics + .provider + .reads + .load(Ordering::SeqCst), + reads + ); + assert!(harness.messages().is_empty()); +} + +#[test] +fn an_identical_same_generation_publication_retires_the_completed_refresh() { + let mut harness = Harness::new(true); + harness.open(1); + harness.start_refresh(); + harness.runtime.complete_next(); + let result_id = assert_full(&harness.pull(None), 1, 1); + + *harness.authority.next_generation.lock().unwrap() = Some(1); + harness.notify( + "textDocument/didSave", + json!({ "textDocument": { "uri": DOCUMENT } }), + ); + harness.runtime.complete_next(); + harness.session.flush_due(harness.now_ms); + assert!( + harness.messages().is_empty(), + "an acknowledged identical report is not republished" + ); + assert_completed_refresh_is_not_polled_again(&mut harness); + assert_eq!( + harness.pull(Some(&result_id))["result"]["kind"], + "unchanged" + ); + assert_eq!(harness.runtime.started(), 2); +} + +#[test] +fn a_report_too_large_for_push_remains_available_to_pull_without_polling_again() { + let mut harness = Harness::new(true); + harness.open(1); + harness.start_refresh(); + let code = "x".repeat(300 * 1024); + *harness.authority.next_code.lock().unwrap() = Some(code.clone()); + harness.runtime.complete_next(); + harness.session.flush_due(harness.now_ms); + assert!( + harness.messages().is_empty(), + "the oversized push must be refused" + ); + assert_completed_refresh_is_not_polled_again(&mut harness); + + let full = harness.pull(None); + assert_eq!(full["result"]["kind"], "full", "{full}"); + let mut expected = expected_items(1, 1); + expected[0]["code"] = json!(code); + assert_eq!(full["result"]["items"], expected); + let result_id = full["result"]["resultId"].as_str().unwrap(); + assert_eq!(harness.pull(Some(result_id))["result"]["kind"], "unchanged"); + assert_eq!(harness.runtime.started(), 1); +} + +#[test] +fn open_document_snapshots_refuse_new_results_when_the_bound_is_owned() { + let mut harness = Harness::new(false); + for index in 0..MAX_WORKSPACE_DIAGNOSTIC_RESULTS { + let uri = format!("file:///root/open-{index}.rs"); + harness.notify("textDocument/didOpen", json!({ + "textDocument": { "uri": uri, "languageId": "rust", "version": 1, "text": "fn a() {}" }, + })); + assert_eq!(harness.pull_document(&uri, None)["error"]["code"], -32802); + harness.runtime.complete_next(); + assert_full(&harness.pull_document(&uri, None), index as u64 + 1, 1); + } + + let refused = harness.pull(None); + assert_eq!(refused["error"]["code"], -32802, "{refused}"); + assert_eq!( + refused["error"]["data"]["failureClass"], + "diagnostic-result-capacity" + ); + assert!(refused.get("result").is_none()); + assert_eq!( + harness.session.diagnostics.document_snapshots.len(), + MAX_WORKSPACE_DIAGNOSTIC_RESULTS + ); + assert_eq!(harness.runtime.started(), MAX_WORKSPACE_DIAGNOSTIC_RESULTS); +} diff --git a/crates/tracedecay-lsp/src/provider.rs b/crates/tracedecay-lsp/src/provider.rs index efadb03008..2792efb3ae 100644 --- a/crates/tracedecay-lsp/src/provider.rs +++ b/crates/tracedecay-lsp/src/provider.rs @@ -402,7 +402,7 @@ pub enum DiagnosticSnapshotOutcome { } /// Port implemented by a daemon/application adapter. It is a read projection; -/// no diagnostic record is owned or persisted by the gateway. Refresh methods +/// the session retains admitted reports only for its ephemeral lifetime. Refresh methods /// are non-blocking: the provider starts canonical upstream/compiler work and /// later reports `Ready` with the matching completed operation identity. /// Unsaved overlays are ephemeral inputs and must never be persisted by the @@ -427,6 +427,17 @@ pub trait DiagnosticSnapshotPort { } } + /// Releases one refresh's result ownership and requests task cancellation. + /// A successful retirement does not prove accepted analyzer work has stopped. + fn retire_document_refresh( + &self, + _root: &AdmittedRoot, + _document_uri: &str, + _operation_id: &str, + ) -> bool { + false + } + fn supports_workspace_diagnostics(&self) -> bool { false } @@ -478,6 +489,15 @@ where (**self).request_document_refresh(root, document_uri, overlay, source_generation) } + fn retire_document_refresh( + &self, + root: &AdmittedRoot, + document_uri: &str, + operation_id: &str, + ) -> bool { + (**self).retire_document_refresh(root, document_uri, operation_id) + } + fn supports_workspace_diagnostics(&self) -> bool { (**self).supports_workspace_diagnostics() } From f0af96c31323f5e4a628d89f8459ec2de454adfe Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 24 Oct 2026 19:42:03 +0000 Subject: [PATCH 02/12] fix(lsp): show empty outbound messages on assertion failure --- .../tracedecay-lsp/src/protocol/tests/diagnostic_snapshots.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/tracedecay-lsp/src/protocol/tests/diagnostic_snapshots.rs b/crates/tracedecay-lsp/src/protocol/tests/diagnostic_snapshots.rs index f98141356e..c494080453 100644 --- a/crates/tracedecay-lsp/src/protocol/tests/diagnostic_snapshots.rs +++ b/crates/tracedecay-lsp/src/protocol/tests/diagnostic_snapshots.rs @@ -684,7 +684,7 @@ fn shutdown_releases_ready_snapshots_and_retires_late_completions() { harness.runtime.complete_next(); harness.session.flush_due(harness.now_ms); - assert!(harness.messages().is_empty()); + assert_eq!(harness.messages(), Vec::::new()); assert!(matches!( harness.session.diagnostics.provider.adapter.document_diagnostics(&root, pending_uri, None), DiagnosticSnapshotOutcome::Partial { coverage, .. } if coverage == "refresh-required" @@ -743,7 +743,7 @@ fn assert_completed_refresh_is_not_polled_again(harness: &mut Harness) { .load(Ordering::SeqCst), reads ); - assert!(harness.messages().is_empty()); + assert_eq!(harness.messages(), Vec::::new()); } #[test] From a833f1ae1e6ac5ecb71c6d6f7a4a98437fd4cba6 Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Sat, 24 Oct 2026 07:03:53 -0700 Subject: [PATCH 03/12] fix(source-edit): insert moved imports at Rust syntax boundaries --- .../tracedecay-source-edit/src/edits/mod.rs | 2 + .../src/edits/move_destination_tests.rs | 268 ++++++++++++++++++ .../src/edits/preview.rs | 11 +- .../src/move_symbol/mod.rs | 3 +- .../src/move_symbol/rendering.rs | 213 +++++++++++--- 5 files changed, 449 insertions(+), 48 deletions(-) create mode 100644 crates/tracedecay-source-edit/src/edits/move_destination_tests.rs diff --git a/crates/tracedecay-source-edit/src/edits/mod.rs b/crates/tracedecay-source-edit/src/edits/mod.rs index 63b885dcb7..aaeea9e24b 100644 --- a/crates/tracedecay-source-edit/src/edits/mod.rs +++ b/crates/tracedecay-source-edit/src/edits/mod.rs @@ -10,6 +10,8 @@ mod symbols; #[cfg(test)] mod execute_tests; #[cfg(test)] +mod move_destination_tests; +#[cfg(test)] mod reconcile_tests; #[cfg(test)] mod test_support; diff --git a/crates/tracedecay-source-edit/src/edits/move_destination_tests.rs b/crates/tracedecay-source-edit/src/edits/move_destination_tests.rs new file mode 100644 index 0000000000..e953cb550e --- /dev/null +++ b/crates/tracedecay-source-edit/src/edits/move_destination_tests.rs @@ -0,0 +1,268 @@ +use std::fs; +use std::path::Path; +use std::process::Command; + +use tempfile::tempdir; +use tracedecay_contracts::{ + EffectTermination, SourceEditKind, SourceEditRequest, source_edit_operation, +}; + +use super::test_support::{ + fixture_authorization, fixture_graph, fixture_request_for_edit, fixture_symbol_code_graph, +}; +use crate::execute::resolve_source_edit_preview; +use crate::{SourceEditOutcome, execute_source_edit, preview_source_edit_expected_state}; + +fn compile_move_destination_fixture(project: &Path) { + let output = Command::new(std::env::var_os("RUSTC").unwrap_or_else(|| "rustc".into())) + .current_dir(project) + .args([ + "--edition=2024", + "--crate-type=lib", + "--emit=metadata", + "--crate-name=move_destination", + "src/lib.rs", + "-o", + "fixture.rmeta", + ]) + .output() + .unwrap(); + assert!( + output.status.success(), + "move destination must compile: {}", + String::from_utf8_lossy(&output.stderr) + ); +} + +async fn move_with_destination_header( + before_import: &str, + after_import: &str, + newline: &str, + destination_exists: bool, +) { + let project = tempdir().unwrap(); + fs::create_dir(project.path().join("src")).unwrap(); + let moved_text = "pub fn moved(value: Widget) -> Widget {\n value\n}"; + let source = format!("use crate::model::Widget;\n\n{moved_text}\n"); + let destination = format!("{before_import}{after_import}"); + fs::write(project.path().join("src/source.rs"), &source).unwrap(); + fs::write(project.path().join("src/model.rs"), "pub struct Widget;\n").unwrap(); + fs::write( + project.path().join("src/lib.rs"), + "#![allow(dead_code, unused_imports)]\nmod model;\nmod source;\nmod relocated;\n", + ) + .unwrap(); + if destination_exists { + fs::write(project.path().join("src/relocated.rs"), &destination).unwrap(); + compile_move_destination_fixture(project.path()); + } + let (graph, _) = fixture_graph(project.path()).await; + let code_graph = fixture_symbol_code_graph( + "src/source.rs", + &source, + moved_text, + "moved", + "source::moved", + ); + let mut request = fixture_request_for_edit( + SourceEditRequest::MoveSymbol { + symbol: "source::moved".to_owned(), + dest_file: "src/relocated.rs".to_owned(), + dry_run: false, + update_references: false, + }, + "source-edit.move-destination", + ); + let preview = resolve_source_edit_preview( + &graph, + &code_graph, + &request.context, + request.observed_at, + tracedecay_graph_query::request_graph_cancellation(&request.context), + request.edit.clone(), + ) + .await + .unwrap(); + let SourceEditOutcome::Move(result) = &preview.outcome else { + panic!("expected a move preview"); + }; + assert!(result.success, "{}", result.message); + assert_eq!(result.applied_imports, ["use crate::model::Widget;"]); + assert_eq!(preview.planned_files.len(), 2); + assert_eq!( + fs::read_to_string(project.path().join("src/source.rs")).unwrap(), + source + ); + if destination_exists { + assert_eq!( + fs::read_to_string(project.path().join("src/relocated.rs")).unwrap(), + destination + ); + } else { + assert!(!project.path().join("src/relocated.rs").exists()); + } + request.expected_state = preview.expected_state.unwrap(); + let authorization = fixture_authorization(&request); + let applied = execute_source_edit( + &graph, + &code_graph, + &source_edit_operation(SourceEditKind::MoveSymbol).unwrap(), + request, + &authorization, + ) + .await + .unwrap(); + assert!(applied.outcome.success(), "{}", applied.outcome.message()); + for planned in preview.planned_files { + assert_eq!( + fs::read_to_string(project.path().join(&planned.relative_path)).unwrap(), + planned.intended, + "apply must publish the exact preview for {}", + planned.relative_path + ); + } + compile_move_destination_fixture(project.path()); + let actual = fs::read_to_string(project.path().join("src/relocated.rs")).unwrap(); + let unchanged_destination = + format!("{before_import}use crate::model::Widget;{newline}{after_import}"); + assert!(actual.starts_with(&unchanged_destination), "{actual:?}"); + assert!(actual.ends_with(&format!("{moved_text}\n"))); + let mut parser = tree_sitter::Parser::new(); + let language = tracedecay_code_extraction::ts_provider::try_language("rust").unwrap(); + parser.set_language(&language).unwrap(); + let tree = parser.parse(&actual, None).unwrap(); + assert!(!tree.root_node().has_error()); + assert!( + tree.root_node() + .named_children(&mut tree.walk()) + .any(|node| { + node.kind() == "use_declaration" + && node.utf8_text(actual.as_bytes()).unwrap() == "use crate::model::Widget;" + }), + "the dependency must be at the destination module root" + ); +} + +#[tokio::test] +async fn move_symbol_imports_preserve_destination_syntax_in_preview_and_apply() { + for (before, after) in [ + ( + "use std::{\n fmt::{self, Debug as DebugTrait},\n collections::HashMap,\n};\n", + "pub fn marker() {}\n", + ), + ( + "//! Destination module.\n#![allow(dead_code)]\n/*! Inner block docs. */\n#![allow(unused_imports)]\nuse std::fmt::Debug;\n", + "pub fn marker() {}\n", + ), + ( + "// Header.\nuse std::fmt::Debug; // Existing import.\n\n", + "/// Marker documentation.\n#[inline]\npub fn marker() {}\n", + ), + ( + "", + "#[cfg(any())]\nuse missing_dependency::{Missing, AlsoMissing};\n/** Marker docs. */\n#[inline]\npub fn marker() {}\n", + ), + ( + "", + "mod nested {\n use std::{fmt::Debug, collections::HashMap};\n}\npub fn marker() {}\n", + ), + ( + "\u{feff}#!/usr/bin/env rust-script\n//! Destination module.\n", + "pub fn marker() {}\n", + ), + ] { + move_with_destination_header(before, after, "\n", true).await; + } +} + +#[tokio::test] +async fn move_symbol_preserves_destination_bytes_when_adding_imports() { + move_with_destination_header( + "//! Module ☃.\r\nuse std::fmt::Debug;\r\n\r\n", + "/// Marker.\r\n#[inline]\r\npub fn marker() {}\r\n \t\r\n\r\n", + "\r\n", + true, + ) + .await; + move_with_destination_header("\r\n \t\r\n", "", "\r\n", true).await; + move_with_destination_header("", "", "\n", true).await; + move_with_destination_header("", "", "\n", false).await; +} + +#[tokio::test] +async fn move_symbol_refuses_unparseable_destination_before_preview_or_apply() { + for (destination, dependency) in [ + ("use std::{\n", true), + ("pub fn marker() {\n", false), + ("/* unclosed destination comment", true), + ("/// Detached documentation.\n#[inline]\n", false), + ] { + let project = tempdir().unwrap(); + fs::create_dir(project.path().join("src")).unwrap(); + let moved_text = if dependency { + "pub fn moved(value: Widget) -> Widget { value }" + } else { + "pub fn moved() {}" + }; + let source = format!("use crate::model::Widget;\n\n{moved_text}\n"); + fs::write(project.path().join("src/source.rs"), &source).unwrap(); + fs::write(project.path().join("src/relocated.rs"), destination).unwrap(); + let (graph, _) = fixture_graph(project.path()).await; + let code_graph = fixture_symbol_code_graph( + "src/source.rs", + &source, + moved_text, + "moved", + "source::moved", + ); + let request = fixture_request_for_edit( + SourceEditRequest::MoveSymbol { + symbol: "source::moved".to_owned(), + dest_file: "src/relocated.rs".to_owned(), + dry_run: false, + update_references: false, + }, + "source-edit.move-invalid-destination", + ); + let preview = preview_source_edit_expected_state( + &graph, + &code_graph, + &request.context, + request.observed_at, + request.edit.clone(), + ) + .await; + assert!( + preview.is_err(), + "malformed destination must not get an accepted preview" + ); + let authorization = fixture_authorization(&request); + let applied = execute_source_edit( + &graph, + &code_graph, + &source_edit_operation(SourceEditKind::MoveSymbol).unwrap(), + request, + &authorization, + ) + .await + .unwrap(); + assert!(!applied.outcome.success()); + assert_eq!( + applied.effect.unwrap().receipt.outcome, + EffectTermination::Failed + ); + assert_eq!( + fs::read_to_string(project.path().join("src/source.rs")).unwrap(), + source + ); + assert_eq!( + fs::read_to_string(project.path().join("src/relocated.rs")).unwrap(), + destination + ); + assert!( + !crate::journal::SourceEditDurability::for_graph(&graph) + .journal_path() + .exists() + ); + } +} diff --git a/crates/tracedecay-source-edit/src/edits/preview.rs b/crates/tracedecay-source-edit/src/edits/preview.rs index ab3c938780..d2227df442 100644 --- a/crates/tracedecay-source-edit/src/edits/preview.rs +++ b/crates/tracedecay-source-edit/src/edits/preview.rs @@ -1,13 +1,12 @@ //! Dry-run preview rendering shared by every edit primitive: a bounded //! single-hunk diff of the changed region, the success-message wrapper that -//! marks dry runs, and the line classifier shared by `move_symbol`'s header -//! scanner and inner-doc skip loop. +//! marks dry runs, and the line classifier used by `move_symbol`'s source +//! inner-doc skip loop. /// What a single source line looks like when scanning a file's leading -/// region. One classifier shared by every place that used to hand-roll its -/// own prefix checks in `move_symbol`'s import-insertion header scanner (which -/// leading lines belong to the header vs. the first item) and its `//!` skip -/// loop (never let a moved span start on an inner module doc). +/// region. MoveSymbol's source-span `//!` skip loop uses this classifier to +/// keep an inner module doc out of the moved item. Destination insertion uses +/// Rust syntax and byte ranges instead. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum LeadingKind { /// Empty (after trimming leading whitespace). diff --git a/crates/tracedecay-source-edit/src/move_symbol/mod.rs b/crates/tracedecay-source-edit/src/move_symbol/mod.rs index f8aea3b615..9d5b50c0f4 100644 --- a/crates/tracedecay-source-edit/src/move_symbol/mod.rs +++ b/crates/tracedecay-source-edit/src/move_symbol/mod.rs @@ -215,7 +215,8 @@ pub(crate) async fn move_symbol( &dest_original, dest_module.as_deref(), ); - let dest_modified = build_dest_content(&dest_original, &applied_imports, &moved_text); + let dest_modified = + build_dest_content(&dest_rel, &dest_original, &applied_imports, &moved_text)?; let mut impact = analysis.hints; impact.extend( diff --git a/crates/tracedecay-source-edit/src/move_symbol/rendering.rs b/crates/tracedecay-source-edit/src/move_symbol/rendering.rs index b0ccc0854c..099b1c66dd 100644 --- a/crates/tracedecay-source-edit/src/move_symbol/rendering.rs +++ b/crates/tracedecay-source-edit/src/move_symbol/rendering.rs @@ -6,10 +6,10 @@ use std::collections::HashSet; -use crate::edits::{ - LeadingKind, MAX_PREVIEW_DIFF_LINES, PREVIEW_DIFF_CONTEXT, bounded_region_diff, - classify_leading_line, splice_lines, -}; +use tracedecay_code_extraction::ts_provider::parse_extractor_source; +use tracedecay_domain::errors::{Result, TraceDecayError}; + +use crate::edits::{MAX_PREVIEW_DIFF_LINES, PREVIEW_DIFF_CONTEXT, bounded_region_diff}; /// Removes `lines[start..=end]` and collapses the blank-line separator the /// removed item left behind so the source stays tidy. @@ -35,51 +35,115 @@ pub(super) fn remove_span_with_cleanup(lines: &[&str], start: usize, end: usize) /// import region or at the top of a fresh file), then the moved block after a /// blank-line separator. pub(super) fn build_dest_content( + dest_file: &str, dest_original: &str, imports: &[String], moved_text: &str, -) -> String { +) -> Result { let moved_block = moved_text.trim_end_matches('\n'); - if dest_original.trim().is_empty() { - let mut parts: Vec = Vec::new(); - if !imports.is_empty() { - parts.push(imports.join("\n")); - } - parts.push(moved_block.to_string()); - let mut content = parts.join("\n\n"); - content.push('\n'); - content + let mut content = if dest_file.ends_with(".rs") { + // Even a dependency-free move must not append inside malformed syntax + // or adopt dangling outer attributes from the destination. + insert_imports(dest_original, imports)? + } else if imports.is_empty() { + dest_original.to_owned() } else { - let with_imports = insert_imports(dest_original, imports); - format!("{}\n\n{}\n", with_imports.trim_end(), moved_block) + return Err(destination_syntax_error( + "automatic dependency imports require a Rust destination", + )); + }; + let newline = line_ending(dest_original); + if !content.is_empty() { + if !content.ends_with('\n') { + content.push_str(newline); + } + if !content.ends_with(&format!("{newline}{newline}")) { + content.push_str(newline); + } } + content.push_str(moved_block); + content.push('\n'); + Ok(content) } -/// Inserts `imports` into an existing Rust file after its leading module-doc / -/// comment / existing-`use` region. -pub(super) fn insert_imports(dest_source: &str, imports: &[String]) -> String { +/// Inserts imports between complete top-level Rust items, preserving all +/// existing bytes and the attachments of outer docs and attributes. +pub(super) fn insert_imports(dest_source: &str, imports: &[String]) -> Result { + let offset = rust_import_offset(dest_source)?; if imports.is_empty() { - return dest_source.to_string(); + return Ok(dest_source.to_string()); + } + let newline = line_ending(dest_source); + let (before, after) = dest_source.split_at(offset); + let mut content = before.to_owned(); + if !before.is_empty() && !before.ends_with('\n') { + content.push_str(newline); + } + content.push_str(&imports.join(newline)); + content.push_str(newline); + content.push_str(after); + Ok(content) +} + +fn line_ending(source: &str) -> &'static str { + if source + .find('\n') + .is_some_and(|offset| offset > 0 && source.as_bytes()[offset - 1] == b'\r') + { + "\r\n" + } else { + "\n" } - let lines: Vec<&str> = dest_source.lines().collect(); - let mut idx = 0; - while idx < lines.len() { - // Stop before an OUTER doc-comment block (`///` or `/**`): it documents - // the first item, and inserting a `use` between the doc and its item - // detaches the doc. Inner docs (`//!`), plain comments (`//`), blank - // lines, and existing imports stay in the header region. - match classify_leading_line(lines[idx]) { - LeadingKind::OuterDoc => break, - LeadingKind::Blank | LeadingKind::InnerDoc | LeadingKind::LineComment => idx += 1, - LeadingKind::UseImport => idx += 1, - LeadingKind::BlockComment | LeadingKind::Attribute | LeadingKind::Code => break, +} + +fn destination_syntax_error(message: impl Into) -> TraceDecayError { + TraceDecayError::project_route("source-edit-destination-syntax-unsupported", false, message) +} + +fn rust_import_offset(source: &str) -> Result { + let tree = parse_extractor_source("rust", "Rust", source).map_err(|error| { + destination_syntax_error(format!("cannot parse Rust destination: {error}")) + })?; + let root = tree.root_node(); + if root.has_error() { + return Err(destination_syntax_error( + "Rust destination contains incomplete or unsupported syntax", + )); + } + let mut offset = None; + let mut seen_item = false; + let mut pending_outer = false; + for node in root.named_children(&mut root.walk()) { + let comment = matches!(node.kind(), "line_comment" | "block_comment"); + if node.kind() == "inner_attribute_item" + || (comment && node.child_by_field_name("inner").is_some()) + { + if seen_item || pending_outer { + return Err(destination_syntax_error( + "Rust destination has an inner attribute or doc after an item or outer attribute", + )); + } + } else if node.kind() == "attribute_item" + || (comment && node.child_by_field_name("outer").is_some()) + { + // Outer attachments belong to the following item, including when + // that item is itself an import. Never split this group. + offset.get_or_insert(node.start_byte()); + pending_outer = true; + } else if !comment && node.kind() != "shebang" { + if !matches!(node.kind(), "use_declaration" | "extern_crate_declaration") { + offset.get_or_insert(node.start_byte()); + } + seen_item = true; + pending_outer = false; } } - let mut rebuilt: Vec<&str> = Vec::with_capacity(lines.len() + imports.len()); - rebuilt.extend_from_slice(&lines[..idx]); - rebuilt.extend(imports.iter().map(String::as_str)); - rebuilt.extend_from_slice(&lines[idx..]); - splice_lines(&rebuilt, dest_source.ends_with('\n')) + if pending_outer { + return Err(destination_syntax_error( + "Rust destination ends with an outer attribute or doc without its item", + )); + } + Ok(offset.unwrap_or(source.len())) } /// Drops imports already present verbatim in the destination and de-duplicates @@ -193,10 +257,12 @@ mod tests { #[test] fn build_dest_content_new_file_prepends_imports() { let content = build_dest_content( + "src/relocated.rs", "", &["use crate::pricing::LineItem;".to_string()], "/// doc\nfn f() {}\n", - ); + ) + .unwrap(); assert_eq!( content, "use crate::pricing::LineItem;\n\n/// doc\nfn f() {}\n" @@ -205,7 +271,13 @@ mod tests { #[test] fn build_dest_content_existing_file_appends_after_blank() { - let content = build_dest_content("//! mod\n\nfn existing() {}\n", &[], "fn f() {}"); + let content = build_dest_content( + "src/relocated.rs", + "//! mod\n\nfn existing() {}\n", + &[], + "fn f() {}", + ) + .unwrap(); assert_eq!(content, "//! mod\n\nfn existing() {}\n\nfn f() {}\n"); } @@ -214,7 +286,8 @@ mod tests { let out = insert_imports( "//! module doc\n\nfn a() {}\n", &["use crate::X;".to_string()], - ); + ) + .unwrap(); assert_eq!(out, "//! module doc\n\nuse crate::X;\nfn a() {}\n"); } @@ -225,7 +298,8 @@ mod tests { let out = insert_imports( "/// Docs for other.\npub fn other() {}\n", &["use crate::X;".to_string()], - ); + ) + .unwrap(); assert_eq!( out, "use crate::X;\n/// Docs for other.\npub fn other() {}\n" @@ -239,6 +313,63 @@ mod tests { assert_eq!(lines[doc + 1].trim(), "pub fn other() {}"); } + #[test] + fn insert_imports_preserves_comments_and_complete_same_line_items() { + for (original, expected) in [ + ( + "/* License. */\nuse a::{B, C}; /* Import note. */\nfn marker() {}", + "/* License. */\nuse a::{B, C}; /* Import note. */\nuse crate::X;\nfn marker() {}", + ), + ( + "use a::B; fn marker() {}", + "use a::B; \nuse crate::X;\nfn marker() {}", + ), + ("// License.", "// License.\nuse crate::X;\n"), + ("/* License. */", "/* License. */\nuse crate::X;\n"), + ("//! Module.", "//! Module.\nuse crate::X;\n"), + ] { + assert_eq!( + insert_imports(original, &["use crate::X;".to_owned()]).unwrap(), + expected + ); + } + } + + #[test] + fn destination_validation_rejects_incomplete_or_misplaced_attachments() { + for original in [ + "use a::{B, C", + "/// Detached.\n", + "#[allow(unused)]\n// Detached.\n", + "use a::B;\n#![allow(unused)]\n", + "fn keep() {}\n//! Misplaced module doc.\n", + ] { + assert!( + build_dest_content("src/relocated.rs", original, &[], "fn moved() {}").is_err(), + "must refuse without needing new imports: {original}" + ); + } + } + + #[test] + fn non_rust_destination_without_imports_keeps_generic_append_support() { + let original = "export const keep = 1;\n \t"; + assert_eq!( + build_dest_content("src/relocated.ts", original, &[], "export const moved = 2;") + .unwrap(), + "export const keep = 1;\n \t\n\nexport const moved = 2;\n" + ); + assert!( + build_dest_content( + "src/relocated.ts", + original, + &["use crate::X;".to_owned()], + "export const moved = 2;" + ) + .is_err() + ); + } + #[test] fn dedup_preserve_skips_existing() { let out = dedup_preserve( From 66d30417f5f66ec53b3d5664e8f77c1c7212cef1 Mon Sep 17 00:00:00 2001 From: Zack Jackson Date: Sat, 24 Oct 2026 19:26:20 -0700 Subject: [PATCH 04/12] fix(lsp): revalidate retained diagnostic authority --- .../src/lsp_runtime/feedback_source.rs | 17 +- .../src/lsp_support/factory.rs | 20 +- .../src/lsp_support/runtime_adapters.rs | 87 +++++- .../tests/code_index_scheduler/reconcile.rs | 226 ++++++++++++++ crates/tracedecay-lsp/src/lib.rs | 4 +- crates/tracedecay-lsp/src/overlay.rs | 106 ++++++- .../src/overlay/diagnostic_authority.rs | 13 + crates/tracedecay-lsp/src/protocol.rs | 18 +- .../src/protocol/diagnostic_snapshots.rs | 87 +++++- .../src/protocol/diagnostics_controller.rs | 55 +++- .../src/protocol/outbound_controller.rs | 15 +- .../protocol/tests/diagnostic_publication.rs | 5 +- .../protocol/tests/diagnostic_snapshots.rs | 279 +++++++++++++++++- crates/tracedecay-lsp/src/provider.rs | 29 ++ crates/tracedecay-lsp/src/rpc.rs | 5 +- 15 files changed, 904 insertions(+), 62 deletions(-) diff --git a/crates/tracedecay-application/src/lsp_runtime/feedback_source.rs b/crates/tracedecay-application/src/lsp_runtime/feedback_source.rs index fd2c02c5c0..cc9d1a751c 100644 --- a/crates/tracedecay-application/src/lsp_runtime/feedback_source.rs +++ b/crates/tracedecay-application/src/lsp_runtime/feedback_source.rs @@ -14,7 +14,7 @@ use tracedecay_domain::feedback::{ FeedbackContentIdentityV1, FeedbackDiagnosticProducerV1, FeedbackFindingLifecycleV1, FeedbackFindingV1, }; -use tracedecay_domain::{ContentDigest, UtcMicros}; +use tracedecay_domain::{ContentDigest, ManifestDigest, UtcMicros}; use tracedecay_lsp::{ AdmittedRoot, CanonicalContextProjectionAuthority, CanonicalDiagnosticRefreshRequest, ContextCoverage, ContextExpansionEnvelope, ContextExpansionOutcome, ContextExpansionRequest, @@ -505,6 +505,21 @@ impl FeedbackCycleRuntimePort for ConcreteFeedbackLspSource { } impl ManagedDiagnosticSnapshotPort for ConcreteFeedbackLspSource { + fn current_authority( + &self, + request: CanonicalDiagnosticRefreshRequest, + ) -> LspRuntimeFuture> { + let source = self.clone(); + Box::pin(async move { + let scope = source + .scope + .resolve(request.root.clone(), Some(request.document_uri.clone())) + .await?; + crate::lsp_support::validate_managed_diagnostic_scope(&request, &scope)?; + crate::lsp_support::managed_diagnostic_authority_digest(&scope) + }) + } + fn snapshot( &self, request: CanonicalDiagnosticRefreshRequest, diff --git a/crates/tracedecay-application/src/lsp_support/factory.rs b/crates/tracedecay-application/src/lsp_support/factory.rs index 2e0193b905..fefed52f49 100644 --- a/crates/tracedecay-application/src/lsp_support/factory.rs +++ b/crates/tracedecay-application/src/lsp_support/factory.rs @@ -13,8 +13,9 @@ use tracedecay_lsp::{ ContextProjectionChange, ContextProjectionOutcome, ContextProjectionPort, ContextProjectionRegistration, ContextProjectionRequest, DaemonLspProviderBundle, DaemonLspRuntimeSession, DiagnosticRefreshAdmission, DiagnosticSnapshotAdapter, - DiagnosticSnapshotOutcome, DiagnosticSnapshotPort, FeedbackCycleAdapter, FeedbackCyclePort, - FeedbackCycleRequest, FeedbackCycleResponse, FeedbackCycleRuntimePort, GatewayCapabilities, + DiagnosticSnapshotOutcome, DiagnosticSnapshotPort, DiagnosticSnapshotValidation, + FeedbackCycleAdapter, FeedbackCyclePort, FeedbackCycleRequest, FeedbackCycleResponse, + FeedbackCycleRuntimePort, GatewayCapabilities, GenerationDiagnostics, LspAnalyzerCancellationAuthority, LspRequestId, LspRuntimeFailure, LspRuntimeFuture, LspSemanticDocumentSnapshot, NativeIntegrationStatusPort, OverlaySnapshot, SemanticProviderOutcome, SemanticProviderPort, SemanticRequest, SemanticResponse, @@ -486,6 +487,21 @@ impl DiagnosticSnapshotPort for FederatedDiagnostics { ) } + fn validate_document_snapshot( + &self, + root: &AdmittedRoot, + document_uri: &str, + overlay: Option<&OverlaySnapshot>, + snapshot: &GenerationDiagnostics, + ) -> DiagnosticSnapshotValidation { + self.roots.get(root.uri()).map_or( + DiagnosticSnapshotValidation::Failed { + failure_class: "root-not-authorized".to_owned(), + }, + |port| port.validate_document_snapshot(root, document_uri, overlay, snapshot), + ) + } + fn retire_document_refresh( &self, root: &AdmittedRoot, diff --git a/crates/tracedecay-application/src/lsp_support/runtime_adapters.rs b/crates/tracedecay-application/src/lsp_support/runtime_adapters.rs index e74b9bad1c..c16d904ba2 100644 --- a/crates/tracedecay-application/src/lsp_support/runtime_adapters.rs +++ b/crates/tracedecay-application/src/lsp_support/runtime_adapters.rs @@ -4,9 +4,10 @@ use std::sync::Arc; use std::time::Duration; use tokio::sync::Mutex as AsyncMutex; +use tracedecay_domain::ManifestDigest; use tracedecay_lsp::analyzer::broker::{ CodeDiagnostic, DiagnosticBroker, DiagnosticSeverity as BrokerDiagnosticSeverity, - RefreshCommitOutcome, + DiagnosticsSnapshot, RefreshCommitOutcome, }; use tracedecay_lsp::analyzer::client::{DiagnosticDocumentCoverage, LspDocument}; use tracedecay_lsp::{ @@ -166,6 +167,29 @@ where }) } + fn current_authority( + &self, + mut request: CanonicalDiagnosticRefreshRequest, + ) -> LspRuntimeFuture> { + let broker = Arc::clone(&self.broker); + let documents = Arc::clone(&self.documents); + let managed = Arc::clone(&self.managed); + Box::pin(async move { + let document = documents.load_document(request.clone()).await?; + request.expected_content_digest = Some(tracedecay_domain::ContentDigest::of_bytes( + document.text.as_bytes(), + )); + let managed = managed.current_authority(request).await?; + let snapshot = broker.lock().await.snapshot(); + let upstream = upstream_diagnostic_authority_digest( + &snapshot, + &document.language, + &document.relative_path, + )?; + merged_diagnostic_authority_digest(&upstream, &managed) + }) + } + fn supports_workspace_diagnostics(&self) -> bool { self.documents.is_mounted() } @@ -328,13 +352,8 @@ where "diagnostic-broker-coverage-incomplete", )); } - let upstream_authority_digest = tracedecay_domain::canonical_sha256(&( - "tracedecay.lsp.upstream-diagnostic-authority.v1", - &snapshot.settings, - &snapshot.settings_unavailable, - document_coverage, - )) - .map_err(|_| LspRuntimeFailure::new("diagnostic-broker-authority-identity-unavailable"))?; + let upstream_authority_digest = + upstream_diagnostic_authority_digest(&snapshot, &language, &relative_path)?; let upstream = snapshot .diagnostics .into_iter() @@ -355,12 +374,8 @@ where "managed-diagnostic-generation-stale", )); } - let authority_digest = tracedecay_domain::canonical_sha256(&( - "tracedecay.lsp.merged-diagnostic-authority.v1", - upstream_authority_digest, - &managed.authority_digest, - )) - .map_err(|_| LspRuntimeFailure::new("diagnostic-authority-identity-unavailable"))?; + let authority_digest = + merged_diagnostic_authority_digest(&upstream_authority_digest, &managed.authority_digest)?; Ok(GenerationDiagnostics { generation: managed.generation, authority_digest, @@ -369,6 +384,36 @@ where }) } +fn upstream_diagnostic_authority_digest( + snapshot: &DiagnosticsSnapshot, + language: &str, + relative_path: &str, +) -> Result { + let coverage = snapshot + .coverage + .get(language) + .and_then(|coverage| coverage.documents.get(relative_path)); + tracedecay_domain::canonical_sha256(&( + "tracedecay.lsp.upstream-diagnostic-authority.v1", + &snapshot.settings, + &snapshot.settings_unavailable, + coverage, + )) + .map_err(|_| LspRuntimeFailure::new("diagnostic-broker-authority-identity-unavailable")) +} + +fn merged_diagnostic_authority_digest( + upstream: &ManifestDigest, + managed: &ManifestDigest, +) -> Result { + tracedecay_domain::canonical_sha256(&( + "tracedecay.lsp.merged-diagnostic-authority.v1", + upstream, + managed, + )) + .map_err(|_| LspRuntimeFailure::new("diagnostic-authority-identity-unavailable")) +} + fn broker_diagnostic(document_uri: &str, diagnostic: CodeDiagnostic) -> GatewayDiagnostic { let start_line = diagnostic.line_start.saturating_sub(1); let end_line = diagnostic @@ -487,6 +532,13 @@ mod tests { struct FixedManagedDiagnostics; impl ManagedDiagnosticSnapshotPort for FixedManagedDiagnostics { + fn current_authority( + &self, + _request: CanonicalDiagnosticRefreshRequest, + ) -> LspRuntimeFuture> { + Box::pin(async { Ok(digest('c')) }) + } + fn snapshot( &self, _request: CanonicalDiagnosticRefreshRequest, @@ -535,6 +587,13 @@ mod tests { struct MismatchedManagedDiagnostics; impl ManagedDiagnosticSnapshotPort for MismatchedManagedDiagnostics { + fn current_authority( + &self, + _request: CanonicalDiagnosticRefreshRequest, + ) -> LspRuntimeFuture> { + Box::pin(async { Ok(digest('c')) }) + } + fn snapshot( &self, _request: CanonicalDiagnosticRefreshRequest, diff --git a/crates/tracedecay-code-index-runtime/tests/code_index_scheduler/reconcile.rs b/crates/tracedecay-code-index-runtime/tests/code_index_scheduler/reconcile.rs index 182c11a2ba..4a11afb541 100644 --- a/crates/tracedecay-code-index-runtime/tests/code_index_scheduler/reconcile.rs +++ b/crates/tracedecay-code-index-runtime/tests/code_index_scheduler/reconcile.rs @@ -13514,3 +13514,229 @@ fn a_moved_commit_refuses_the_sealed_generation_despite_identical_bytes() { "the rebuilt generation must name the commit the checkout is on" ); } + +mod diagnostic_authority { + use std::collections::BTreeSet; + use std::sync::Arc; + use std::time::Duration; + + use tempfile::TempDir; + use tracedecay_application::ProjectSourceAccessSnapshot; + use tracedecay_application::diagnostics_store::DiagnosticsStore; + use tracedecay_application::feedback::concrete::open_feedback_runtime; + use tracedecay_application::lsp_runtime::{ + BrokerDiagnosticSnapshotAuthority, ConcreteFeedbackLspSource, + DiagnosticsStoreLspFeedbackProjection, LspCodeIndexProjectionIdentityPort, + LspTestRunProjectionPort, LspWorkspaceDocumentIndexPort, RegisteredProjectLspAuthority, + }; + use tracedecay_domain::configuration::{ + AuthorityRef, ConfigurationRevisionId, ScopeSourceBinding, SourceBindingId, SourceKindV1, + }; + use tracedecay_domain::test_fixtures::{digest, id}; + use tracedecay_domain::{ContentDigest, UtcMicros}; + use tracedecay_lsp::analyzer::broker::DiagnosticBroker; + use tracedecay_lsp::{ + AdmittedRoot, CanonicalDiagnosticRefreshRequest, CanonicalDiagnosticSnapshotAuthority, + ContextProjectionOutcome, FeedbackCycleRequest, FeedbackCycleRuntimePort, + IndexedWorkspaceDocuments, LspRuntimeFailure, LspRuntimeFuture, + }; + use tracedecay_runtime_core::db::{Database, DatabaseAuthority, TestDatabaseRuntimeMode}; + + use super::{ + CodeIndexSchedulerRegistryV1, GitFixture, test_project_id, wait_for_generation_change, + wait_for_live_complete_generation, + }; + use crate::code_index_scheduler::identity::resolved_scope_for_project; + + // Currentness reads must not execute diagnostics, test runs, or workspace sweeps. + struct UnexpectedDiagnosticWork; + + impl FeedbackCycleRuntimePort for UnexpectedDiagnosticWork { + fn execute( + &self, + _: FeedbackCycleRequest, + ) -> LspRuntimeFuture> { + panic!("authority validation must not execute a feedback cycle") + } + } + + impl LspTestRunProjectionPort for UnexpectedDiagnosticWork { + fn snapshot( + &self, + _: AdmittedRoot, + _: Option, + _: Option, + ) -> LspRuntimeFuture { + panic!("authority validation must not execute tests") + } + } + + impl LspWorkspaceDocumentIndexPort for UnexpectedDiagnosticWork { + fn indexed_documents( + &self, + _: AdmittedRoot, + _: usize, + ) -> LspRuntimeFuture> { + panic!("document authority validation must not enumerate the workspace") + } + } + + #[tokio::test] + async fn diagnostic_authority_changes_when_an_unchanged_documents_dependency_is_published() { + let source = "pub fn alpha() -> u32 { 1 }\n"; + let fixture = GitFixture::new(&[ + ("src/lib.rs", source), + ("src/dependency.rs", "pub fn dependency() -> u32 { 1 }\n"), + ]); + let storage = TempDir::new().unwrap(); + let registry = CodeIndexSchedulerRegistryV1::new(1); + registry + .mount_worktree( + test_project_id(), + fixture.path(), + storage.path().join("index"), + ) + .await + .unwrap(); + let initial = wait_for_live_complete_generation(®istry, fixture.path()).await; + let initial_generation = initial.generation().manifest().generation_id.clone(); + let scope = resolved_scope_for_project(fixture.path(), &test_project_id()).unwrap(); + let database_path = storage.path().join("feedback.db"); + crate::register_test_schema_installer(); + let database_authority = + DatabaseAuthority::acquire_test(&database_path, "diagnostic authority publication") + .unwrap(); + let (database, _guard) = Database::publish_test_runtime( + &database_path, + &database_authority, + TestDatabaseRuntimeMode::Initialize, + ) + .await + .unwrap(); + let access = ProjectSourceAccessSnapshot { + scope: scope.clone(), + requester: id("actor.diagnostic-authority"), + binding: ScopeSourceBinding::new( + SourceBindingId::new("binding.diagnostic-authority").unwrap(), + SourceKindV1::Cursor, + id(digest('a').as_str()), + AuthorityRef::Project(scope.project_id.clone()), + ) + .unwrap(), + configuration_revision: ConfigurationRevisionId::new( + "configuration.diagnostic-authority", + ) + .unwrap(), + configuration_digest: digest('b'), + configuration_provenance_digest: digest('c'), + effective_capabilities: BTreeSet::new(), + grant_expires_at: UtcMicros(i64::MAX), + }; + let runtime = Arc::new( + open_feedback_runtime( + database.clone(), + fixture.path(), + storage.path().join("handles"), + scope.clone(), + access, + ) + .await + .unwrap(), + ); + let project = Arc::new( + RegisteredProjectLspAuthority::new( + runtime.clone(), + Arc::new(registry.clone()), + Arc::new(UnexpectedDiagnosticWork), + ) + .unwrap(), + ); + let projection = Arc::new(DiagnosticsStoreLspFeedbackProjection::new( + Arc::new(DiagnosticsStore::new(database)), + project.clone(), + )); + let managed = Arc::new(ConcreteFeedbackLspSource::new( + runtime, + |_| Arc::new(UnexpectedDiagnosticWork), + project.clone(), + projection, + Arc::new(UnexpectedDiagnosticWork), + )); + let authority = BrokerDiagnosticSnapshotAuthority::new( + Arc::new(tokio::sync::Mutex::new(DiagnosticBroker::new_for_test( + fixture.path(), + Vec::new(), + ))), + project, + managed, + Duration::from_millis(1), + ); + let request = CanonicalDiagnosticRefreshRequest { + root: AdmittedRoot::authorized( + url::Url::from_directory_path(fixture.path()) + .unwrap() + .to_string(), + scope.scope_digest, + ), + document_uri: url::Url::from_file_path(fixture.path().join("src/lib.rs")) + .unwrap() + .to_string(), + overlay: None, + source_generation: None, + expected_content_digest: Some(ContentDigest::of_bytes(source.as_bytes())), + expected_code_generation_id: None, + expected_snapshot_digest: None, + }; + let first = authority.current_authority(request.clone()).await.unwrap(); + assert_eq!( + first, + authority.current_authority(request.clone()).await.unwrap() + ); + fixture.edit("src/dependency.rs", "pub fn dependency() -> u32 { 2 }\n"); + registry + .notify_hook_paths(fixture.path(), &["src/dependency.rs".to_owned()]) + .await; + let successor = + wait_for_generation_change(®istry, fixture.path(), &initial_generation).await; + assert_ne!(successor, initial_generation); + wait_for_live_complete_generation(®istry, fixture.path()).await; + let identity = registry + .current_identity(fixture.path().to_path_buf(), Some("src/lib.rs".to_owned())) + .await + .unwrap(); + assert_eq!( + identity.document_content_digest, + request.expected_content_digest + ); + assert_eq!( + authority + .current_authority(request.clone()) + .await + .unwrap_err() + .class(), + "lsp-code-index-source-revision-unavailable" + ); + fixture.commit_all("publish changed dependency"); + registry + .notify_hook_paths(fixture.path(), &["src/dependency.rs".to_owned()]) + .await; + wait_for_generation_change(®istry, fixture.path(), &successor).await; + wait_for_live_complete_generation(®istry, fixture.path()).await; + let second = authority.current_authority(request.clone()).await.unwrap(); + assert_ne!( + first, second, + "unchanged A must observe the real successor containing changed B" + ); + let mut stale = request; + stale.expected_code_generation_id = Some(initial_generation); + assert_eq!( + authority + .current_authority(stale) + .await + .unwrap_err() + .class(), + "managed-diagnostic-generation-stale" + ); + registry.shutdown().await; + } +} diff --git a/crates/tracedecay-lsp/src/lib.rs b/crates/tracedecay-lsp/src/lib.rs index 1c67a047e0..1828c696c4 100644 --- a/crates/tracedecay-lsp/src/lib.rs +++ b/crates/tracedecay-lsp/src/lib.rs @@ -125,8 +125,8 @@ pub use provider::{ AnalyzerCancellationPort, AnalyzerEvent, AnalyzerSemanticAdapter, AnalyzerState, AnalyzerSupervisor, AnalyzerTransitionError, DiagnosticRefreshAdmission, DiagnosticRefreshIdentity, DiagnosticSnapshotOutcome, DiagnosticSnapshotPort, - GenerationDiagnostics, MAX_ANALYZER_RESTARTS, MAX_DIAGNOSTIC_OPERATION_ID_BYTES, - UnavailableDiagnosticSnapshotProvider, + DiagnosticSnapshotValidation, GenerationDiagnostics, MAX_ANALYZER_RESTARTS, + MAX_DIAGNOSTIC_OPERATION_ID_BYTES, UnavailableDiagnosticSnapshotProvider, }; pub use session::{ AuthorizedLspSession, AuthorizedLspWorkspace, CancellationOutcome, CompletionDisposition, diff --git a/crates/tracedecay-lsp/src/overlay.rs b/crates/tracedecay-lsp/src/overlay.rs index 0a7f0d6f47..66189dfdd7 100644 --- a/crates/tracedecay-lsp/src/overlay.rs +++ b/crates/tracedecay-lsp/src/overlay.rs @@ -8,16 +8,18 @@ use std::collections::BTreeMap; use std::sync::{Arc, Weak}; +use futures_util::FutureExt; + use tracedecay_code_extraction::incremental::ParseDocumentIdentity; use tracedecay_daemon_protocol::ProcessLocalRequestSequence; use tracedecay_domain::{ContentDigest, ManifestDigest, canonical_sha256}; use crate::diagnostics::{LspRange, PositionError}; use crate::gateway::operation_table::{BoundedOperationTable, OperationAdmission, OperationPoll}; -use crate::gateway::{AdmittedRoot, LspRuntimeFuture, LspRuntimeSpawner}; +use crate::gateway::{AdmittedRoot, LspRuntimeFailure, LspRuntimeFuture, LspRuntimeSpawner}; use crate::provider::{ DiagnosticRefreshAdmission, DiagnosticRefreshIdentity, DiagnosticSnapshotOutcome, - DiagnosticSnapshotPort, + DiagnosticSnapshotPort, DiagnosticSnapshotValidation, GenerationDiagnostics, }; use crate::session::AuthorizedLspWorkspace; use crate::workspace_diagnostics::WorkspaceDiagnosticSnapshotOutcome; @@ -401,6 +403,11 @@ struct DiagnosticOperationKey { overlay_digest: Option, } +enum DiagnosticValidationPreparation { + Completed(Result), + IdentityExhausted, +} + pub struct DiagnosticSnapshotAdapter { runtime: Arc, authority: Arc, @@ -501,7 +508,16 @@ impl DiagnosticSnapshotPort for DiagnosticSnapshotAdapter { target_generation: None, }; let operation = Box::pin(async move { - match authority.refresh(request).await { + let refreshed = async { + let diagnostics = authority.refresh(request.clone()).await?; + let current = authority.current_authority(request).await?; + if current != diagnostics.authority_digest { + return Err(LspRuntimeFailure::new("diagnostic-authority-stale")); + } + Ok(diagnostics) + } + .await; + match refreshed { Ok(diagnostics) => DiagnosticSnapshotOutcome::Ready { diagnostics, completed_operation_id: Some(operation_id), @@ -536,6 +552,90 @@ impl DiagnosticSnapshotPort for DiagnosticSnapshotAdapter { } } + fn validate_document_snapshot( + &self, + root: &AdmittedRoot, + document_uri: &str, + overlay: Option<&OverlaySnapshot>, + snapshot: &GenerationDiagnostics, + ) -> DiagnosticSnapshotValidation { + let key = Self::key(root, document_uri, overlay); + let admission = self.operations.admit_with(key, self.runtime.as_ref(), || { + // The table owns capacity before even the first poll. A ready read + // releases that permit without creating a task; a pending read keeps + // the same permit and cancellation ownership as a full refresh. + let mut current = self + .authority + .current_authority(CanonicalDiagnosticRefreshRequest { + root: root.clone(), + document_uri: document_uri.to_owned(), + overlay: overlay.cloned(), + source_generation: Some(snapshot.generation), + expected_content_digest: overlay.map(|overlay| overlay.content_digest.clone()), + expected_code_generation_id: None, + expected_snapshot_digest: None, + }); + if let Some(result) = current.as_mut().now_or_never() { + return Err(DiagnosticValidationPreparation::Completed( + result.map(|digest| digest == snapshot.authority_digest), + )); + } + let operation_id = self + .next_operation + .next_string("lsp-diagnostic-") + .map_err(|_| DiagnosticValidationPreparation::IdentityExhausted)?; + let snapshot = snapshot.clone(); + let identity = DiagnosticRefreshIdentity { + operation_id: operation_id.clone(), + source_generation: Some(snapshot.generation), + target_generation: None, + }; + let operation = Box::pin(async move { + match current.await { + Ok(digest) if digest == snapshot.authority_digest => { + DiagnosticSnapshotOutcome::Ready { + diagnostics: snapshot, + completed_operation_id: Some(operation_id), + } + } + Ok(_) => DiagnosticSnapshotOutcome::Partial { + source_generation: Some(snapshot.generation), + coverage: "refresh-required".to_owned(), + }, + Err(error) => DiagnosticSnapshotOutcome::Failed { + source_generation: Some(snapshot.generation), + failure_class: error.class().to_owned(), + }, + } + }) as LspRuntimeFuture; + Ok((identity, operation)) + }); + match admission { + Err(DiagnosticValidationPreparation::Completed(Ok(true))) => { + DiagnosticSnapshotValidation::Current + } + Err(DiagnosticValidationPreparation::Completed(Ok(false))) => { + DiagnosticSnapshotValidation::Changed + } + Err(DiagnosticValidationPreparation::Completed(Err(error))) => { + DiagnosticSnapshotValidation::Failed { + failure_class: error.class().to_owned(), + } + } + Err(DiagnosticValidationPreparation::IdentityExhausted) => { + DiagnosticSnapshotValidation::Failed { + failure_class: "diagnostic-identity-exhausted".to_owned(), + } + } + Ok(OperationAdmission::Started(identity) | OperationAdmission::Existing(identity)) => { + DiagnosticSnapshotValidation::Pending(identity) + } + Ok(OperationAdmission::Saturated) => DiagnosticSnapshotValidation::Failed { + failure_class: "diagnostic-capacity".to_owned(), + }, + } + } + fn retire_document_refresh( &self, root: &AdmittedRoot, diff --git a/crates/tracedecay-lsp/src/overlay/diagnostic_authority.rs b/crates/tracedecay-lsp/src/overlay/diagnostic_authority.rs index 4eb3f9f4d7..f7319dfd0f 100644 --- a/crates/tracedecay-lsp/src/overlay/diagnostic_authority.rs +++ b/crates/tracedecay-lsp/src/overlay/diagnostic_authority.rs @@ -30,6 +30,12 @@ pub struct ManagedDiagnosticSnapshot { } pub trait ManagedDiagnosticSnapshotPort: Send + Sync { + /// Resolves current canonical scope without executing another diagnostic cycle. + fn current_authority( + &self, + request: CanonicalDiagnosticRefreshRequest, + ) -> LspRuntimeFuture>; + fn snapshot( &self, request: CanonicalDiagnosticRefreshRequest, @@ -42,6 +48,13 @@ pub trait CanonicalDiagnosticSnapshotAuthority: Send + Sync { request: CanonicalDiagnosticRefreshRequest, ) -> LspRuntimeFuture>; + fn current_authority( + &self, + _request: CanonicalDiagnosticRefreshRequest, + ) -> LspRuntimeFuture> { + Box::pin(async { Err(LspRuntimeFailure::new("diagnostic-authority-unavailable")) }) + } + fn supports_workspace_diagnostics(&self) -> bool { false } diff --git a/crates/tracedecay-lsp/src/protocol.rs b/crates/tracedecay-lsp/src/protocol.rs index b76392ba09..96a0b00abb 100644 --- a/crates/tracedecay-lsp/src/protocol.rs +++ b/crates/tracedecay-lsp/src/protocol.rs @@ -366,8 +366,7 @@ where .overlays .open(&root, uri.clone(), language_id, version, text) .map_err(|error| self.close_for_overlay_error(error))?; - self.diagnostics.workspace_snapshots.clear(); - self.diagnostics.workspace_failures.clear(); + self.invalidate_document(&uri, &root); // A close followed by a reopen starts a new document incarnation; LSP // versions need not remain monotone across that boundary. Remove any // queued/acknowledged publication ordering state before publishing the @@ -405,7 +404,7 @@ where self.require_ready()?; let text_document = text_document(params)?; let uri = required_nonempty_string(text_document, "uri")?; - self.require_document_root(&uri)?; + let root = self.document_root(&uri)?; let version = required_i64(text_document, "version")?; let changes = params .get("contentChanges") @@ -425,7 +424,7 @@ where .overlays .change(&uri, version, &changes) .map_err(|error| self.close_for_overlay_error(error))?; - self.invalidate_document(&uri); + self.invalidate_document(&uri, &root); self.diagnostics.native_upstream.remove(&uri); self.lifecycle .control @@ -447,13 +446,13 @@ where ) -> Result<(), RpcFailure> { self.require_ready()?; let uri = required_nonempty_string(text_document(params)?, "uri")?; - self.require_document_root(&uri)?; + let root = self.document_root(&uri)?; let closed = self .lifecycle .overlays .close(&uri) .map_err(overlay_failure)?; - self.invalidate_document(&uri); + self.invalidate_document(&uri, &root); self.diagnostics.native_upstream.remove(&uri); self.lifecycle .control @@ -475,8 +474,8 @@ where ) -> Result<(), RpcFailure> { self.require_ready()?; let uri = required_nonempty_string(text_document(params)?, "uri")?; - self.require_document_root(&uri)?; - self.invalidate_document(&uri); + let root = self.document_root(&uri)?; + self.invalidate_document(&uri, &root); if matches!( self.lifecycle.gateway.document_saved(uri.clone()), FeedbackCycleResponse::Accepted @@ -493,7 +492,8 @@ where Ok(()) } - fn invalidate_document(&mut self, uri: &str) { + fn invalidate_document(&mut self, uri: &str, root: &AdmittedRoot) { + self.invalidate_root_diagnostics(root); self.invalidate_document_diagnostics(uri); self.diagnostics.workspace_snapshots.clear(); self.diagnostics.workspace_failures.clear(); diff --git a/crates/tracedecay-lsp/src/protocol/diagnostic_snapshots.rs b/crates/tracedecay-lsp/src/protocol/diagnostic_snapshots.rs index a7f8751fd1..80d3fb7449 100644 --- a/crates/tracedecay-lsp/src/protocol/diagnostic_snapshots.rs +++ b/crates/tracedecay-lsp/src/protocol/diagnostic_snapshots.rs @@ -1,3 +1,5 @@ +use std::collections::BTreeSet; + use tracedecay_domain::{ContentDigest, ManifestDigest}; use crate::diagnostics::DiagnosticMerge; @@ -5,7 +7,7 @@ use crate::gateway::{AdmittedRoot, FeedbackCyclePort, SemanticProviderPort}; use crate::overlay::{OverlayLifetime, OverlaySnapshot}; use crate::provider::{ DiagnosticRefreshIdentity, DiagnosticSnapshotOutcome, DiagnosticSnapshotPort, - GenerationDiagnostics, + DiagnosticSnapshotValidation, GenerationDiagnostics, }; use crate::rpc::{DiagnosticSerializationCapabilities, diagnostic_value}; use crate::workspace_diagnostics::{ @@ -13,7 +15,7 @@ use crate::workspace_diagnostics::{ }; use super::DaemonLspProtocolSession; -use super::diagnostics_controller::PublishedDiagnostic; +use super::diagnostics_controller::{PublishedDiagnostic, valid_refresh_identity}; #[derive(Clone, Debug, Eq, PartialEq)] pub(super) struct DiagnosticDocumentIdentity { @@ -46,6 +48,7 @@ impl DiagnosticDocumentIdentity { pub(super) struct PendingDiagnosticRefresh { pub(super) identity: DiagnosticRefreshIdentity, pub(super) document: DiagnosticDocumentIdentity, + pub(super) validation_only: bool, } pub(super) struct RetainedDiagnosticSnapshot { @@ -66,6 +69,7 @@ where root: &AdmittedRoot, uri: &str, overlay: Option<&OverlaySnapshot>, + for_pull: bool, ) -> DiagnosticSnapshotOutcome { let document = DiagnosticDocumentIdentity::new( root, @@ -97,11 +101,58 @@ where DiagnosticSnapshotOutcome::Partial { ref coverage, .. } if coverage == "refresh-required" => { - return self - .diagnostics - .document_snapshots - .get(uri) - .map_or(outcome, |retained| retained.outcome.clone()); + let Some(retained) = self.diagnostics.document_snapshots.get(uri) else { + return outcome; + }; + let DiagnosticSnapshotOutcome::Ready { diagnostics, .. } = &retained.outcome else { + return retained.outcome.clone(); + }; + let source_generation = Some(diagnostics.generation); + let validation = self.diagnostics.provider.validate_document_snapshot( + root, + uri, + overlay, + diagnostics, + ); + let outcome = match validation { + DiagnosticSnapshotValidation::Current => return retained.outcome.clone(), + DiagnosticSnapshotValidation::Changed => DiagnosticSnapshotOutcome::Partial { + source_generation, + coverage: "refresh-required".to_owned(), + }, + DiagnosticSnapshotValidation::Pending(identity) => { + if valid_refresh_identity(&identity, source_generation) { + self.diagnostics.active_refreshes.insert( + uri.to_owned(), + PendingDiagnosticRefresh { + identity: identity.clone(), + document, + validation_only: for_pull, + }, + ); + DiagnosticSnapshotOutcome::Refreshing(identity) + } else { + let _ = self.diagnostics.provider.retire_document_refresh( + root, + uri, + &identity.operation_id, + ); + DiagnosticSnapshotOutcome::Failed { + source_generation, + failure_class: "invalid-refresh-identity".to_owned(), + } + } + } + DiagnosticSnapshotValidation::Failed { failure_class } => { + DiagnosticSnapshotOutcome::Failed { + source_generation, + failure_class, + } + } + }; + self.diagnostics.document_snapshots.remove(uri); + self.discard_queued_document_publications(uri); + return outcome; } DiagnosticSnapshotOutcome::Ready { mut diagnostics, @@ -258,6 +309,28 @@ where } } + /// A dependency edit can invalidate any report in its admitted root. Route + /// each document through the workspace authority so nested roots stay isolated. + pub(super) fn invalidate_root_diagnostics(&mut self, root: &AdmittedRoot) { + let documents: BTreeSet<_> = self + .diagnostics + .document_snapshots + .keys() + .chain(self.diagnostics.active_refreshes.keys()) + .chain(self.diagnostics.published.keys()) + .filter(|uri| self.document_root(uri).is_ok_and(|owner| &owner == root)) + .cloned() + .collect(); + let invalidated = !documents.is_empty(); + for uri in documents { + self.invalidate_document_diagnostics(&uri); + self.discard_queued_document_publications(&uri); + } + if invalidated { + self.queue_diagnostic_refresh(); + } + } + pub(super) fn invalidate_document_diagnostics(&mut self, uri: &str) { self.retire_diagnostic_refresh(uri); self.diagnostics.document_snapshots.remove(uri); diff --git a/crates/tracedecay-lsp/src/protocol/diagnostics_controller.rs b/crates/tracedecay-lsp/src/protocol/diagnostics_controller.rs index d4f2712a9f..8633d3bdc9 100644 --- a/crates/tracedecay-lsp/src/protocol/diagnostics_controller.rs +++ b/crates/tracedecay-lsp/src/protocol/diagnostics_controller.rs @@ -263,10 +263,14 @@ where self.lifecycle.gateway.request_document_diagnostics(uri), |()| Value::Null, )?; - self.poll_diagnostic_refresh(uri); + let completed = self.poll_diagnostic_refresh(uri, true); let overlay = self.lifecycle.overlays.snapshot(uri); let root = self.document_root(uri)?; - let outcome = self.document_diagnostic_snapshot(&root, uri, overlay.as_ref()); + let outcome = completed + .filter(|outcome| !matches!(outcome, DiagnosticSnapshotOutcome::Refreshing(_))) + .unwrap_or_else(|| { + self.document_diagnostic_snapshot(&root, uri, overlay.as_ref(), true) + }); let version = overlay.as_ref().map_or(0, |overlay| overlay.version); if !matches!(&outcome, DiagnosticSnapshotOutcome::Ready { .. }) { let source_generation = diagnostic_source_generation(&outcome); @@ -319,6 +323,7 @@ where root.scope_digest(), self.lifecycle.gateway.workspace().root_ordinal(&root), generation, + &diagnostics.authority_digest, version, ); let merged = self.merge_document_diagnostics( @@ -383,8 +388,12 @@ where let Ok(root) = self.document_root(&scheduled.uri) else { continue; }; - let outcome = - self.document_diagnostic_snapshot(&root, &scheduled.uri, overlay.as_ref()); + let outcome = self.document_diagnostic_snapshot( + &root, + &scheduled.uri, + overlay.as_ref(), + false, + ); if let DiagnosticSnapshotOutcome::Ready { diagnostics, .. } = outcome { let _ = self.publish_complete_snapshot( &scheduled.uri, @@ -450,7 +459,11 @@ where self.diagnostics.debounce.cancel(uri); self.diagnostics.active_refreshes.insert( uri.to_owned(), - PendingDiagnosticRefresh { identity, document }, + PendingDiagnosticRefresh { + identity, + document, + validation_only: false, + }, ); None } @@ -459,25 +472,35 @@ where } pub(super) fn poll_diagnostic_refreshes(&mut self) { - let documents: Vec<_> = self.diagnostics.active_refreshes.keys().cloned().collect(); + // Pull validation belongs to the requesting reader. Background polling + // must not consume its one-shot completion before that reader retries. + let documents: Vec<_> = self + .diagnostics + .active_refreshes + .iter() + .filter(|(_, pending)| !pending.validation_only) + .map(|(uri, _)| uri.clone()) + .collect(); for document in documents { if !self.has_outbound_capacity(MAX_PUBLICATION_BYTES) { break; } - self.poll_diagnostic_refresh(&document); + self.poll_diagnostic_refresh(&document, false); } } #[tracing::instrument(name = "lsp.diagnostics.refresh_poll", level = "trace", skip_all)] - pub(super) fn poll_diagnostic_refresh(&mut self, uri: &str) { - let Some(pending) = self.diagnostics.active_refreshes.get(uri).cloned() else { - return; - }; + pub(super) fn poll_diagnostic_refresh( + &mut self, + uri: &str, + for_pull: bool, + ) -> Option { + let pending = self.diagnostics.active_refreshes.get(uri).cloned()?; let overlay = self.lifecycle.overlays.snapshot(uri); let version = overlay.as_ref().map_or(0, |overlay| overlay.version); let Ok(root) = self.document_root(uri) else { self.invalidate_document_diagnostics(uri); - return; + return None; }; if pending.document != DiagnosticDocumentIdentity::new( @@ -487,9 +510,10 @@ where ) { self.invalidate_document_diagnostics(uri); - return; + return None; } - match self.document_diagnostic_snapshot(&root, uri, overlay.as_ref()) { + let outcome = self.document_diagnostic_snapshot(&root, uri, overlay.as_ref(), for_pull); + match outcome.clone() { DiagnosticSnapshotOutcome::Ready { diagnostics, completed_operation_id, @@ -509,6 +533,7 @@ where } DiagnosticSnapshotOutcome::Ready { .. } | DiagnosticSnapshotOutcome::Refreshing(_) => {} } + Some(outcome) } pub(super) fn publish_complete_snapshot( @@ -854,7 +879,7 @@ pub(super) fn workspace_diagnostic_failure( } } -fn valid_refresh_identity( +pub(super) fn valid_refresh_identity( identity: &DiagnosticRefreshIdentity, source_generation: Option, ) -> bool { diff --git a/crates/tracedecay-lsp/src/protocol/outbound_controller.rs b/crates/tracedecay-lsp/src/protocol/outbound_controller.rs index b8666fb0c0..79304a4305 100644 --- a/crates/tracedecay-lsp/src/protocol/outbound_controller.rs +++ b/crates/tracedecay-lsp/src/protocol/outbound_controller.rs @@ -318,6 +318,13 @@ where pub(super) fn discard_document_publications(&mut self, uri: &str) { self.discard_document_context(uri); self.invalidate_document_diagnostics(uri); + self.discard_queued_document_publications(uri); + self.lifecycle.control.remove_publication(uri); + self.diagnostics.published.remove(uri); + } + + pub(super) fn discard_queued_document_publications(&mut self, uri: &str) { + let mut discarded = false; let mut retained = VecDeque::with_capacity(self.outbound.queue.len()); let mut index = 0_usize; while let Some(frame) = self.outbound.queue.pop_front() { @@ -329,6 +336,7 @@ where .as_ref() .is_some_and(|publication| publication.uri == uri) { + discarded = true; self.outbound.queued_bytes = self .outbound .queued_bytes @@ -338,8 +346,11 @@ where } } self.outbound.queue = retained; - self.lifecycle.control.remove_publication(uri); - self.diagnostics.published.remove(uri); + if discarded { + // A dropped queue entry was never acknowledged; a fresh identical + // report still needs delivery. Preserve already acknowledged dedupe. + self.lifecycle.control.remove_publication(uri); + } } pub(super) fn has_outbound_capacity(&self, reserve_bytes: usize) -> bool { diff --git a/crates/tracedecay-lsp/src/protocol/tests/diagnostic_publication.rs b/crates/tracedecay-lsp/src/protocol/tests/diagnostic_publication.rs index c0c382c11a..ded8004001 100644 --- a/crates/tracedecay-lsp/src/protocol/tests/diagnostic_publication.rs +++ b/crates/tracedecay-lsp/src/protocol/tests/diagnostic_publication.rs @@ -670,7 +670,10 @@ fn pull_diagnostics_are_generation_bound_and_return_unchanged_for_same_result_id let first = session.drain_outbound(); let first: Value = serde_json::from_slice(&first[0]).unwrap(); let result_id = first["result"]["resultId"].as_str().unwrap().to_owned(); - assert_eq!(result_id, "generation:9:version:0"); + assert_eq!( + result_id, + format!("generation:9:version:0:authority:sha256:{}", "a".repeat(64)) + ); let request = json!({ "jsonrpc": "2.0", "id": 4, diff --git a/crates/tracedecay-lsp/src/protocol/tests/diagnostic_snapshots.rs b/crates/tracedecay-lsp/src/protocol/tests/diagnostic_snapshots.rs index c494080453..76aba96bff 100644 --- a/crates/tracedecay-lsp/src/protocol/tests/diagnostic_snapshots.rs +++ b/crates/tracedecay-lsp/src/protocol/tests/diagnostic_snapshots.rs @@ -3,6 +3,7 @@ use std::sync::{Arc, Mutex}; use std::task::{Context, Poll, Waker}; use serde_json::{Value, json}; +use tracedecay_domain::ManifestDigest; use tracedecay_domain::test_fixtures::digest; use super::{Feedback, Semantics}; @@ -21,7 +22,7 @@ use crate::overlay::{ use crate::protocol::{DaemonLspProtocolSession, MAX_QUEUED_OUTBOUND_MESSAGES}; use crate::provider::{ DiagnosticRefreshAdmission, DiagnosticSnapshotOutcome, DiagnosticSnapshotPort, - GenerationDiagnostics, + DiagnosticSnapshotValidation, GenerationDiagnostics, }; use crate::session::AuthorizedLspWorkspace; use crate::workspace_diagnostics::{ @@ -75,9 +76,30 @@ struct DiagnosticAuthority { next_generation: Mutex>, next_failure: Mutex>, next_code: Mutex>, + authority_digest: Arc>>, + validation_pending: Arc, } impl CanonicalDiagnosticSnapshotAuthority for DiagnosticAuthority { + fn current_authority( + &self, + _request: CanonicalDiagnosticRefreshRequest, + ) -> LspRuntimeFuture> { + let digest = Arc::clone(&self.authority_digest); + let pending = Arc::clone(&self.validation_pending); + Box::pin(std::future::poll_fn(move |_| { + if pending.load(Ordering::SeqCst) { + Poll::Pending + } else { + Poll::Ready(Ok(digest + .lock() + .unwrap() + .clone() + .unwrap_or_else(|| tracedecay_domain::test_fixtures::digest('a')))) + } + })) + } + fn refresh( &self, request: CanonicalDiagnosticRefreshRequest, @@ -100,13 +122,19 @@ impl CanonicalDiagnosticSnapshotAuthority for DiagnosticAuthority { .overlay .as_ref() .map_or(0, |overlay| overlay.version); + let authority_digest = self + .authority_digest + .lock() + .unwrap() + .clone() + .unwrap_or_else(|| digest('a')); Box::pin(async move { if let Some(failure) = failure { return Err(LspRuntimeFailure::new(failure)); } Ok(GenerationDiagnostics { generation, - authority_digest: digest('a'), + authority_digest, upstream: vec![GatewayDiagnostic { uri: request.document_uri, range: LspRange { @@ -172,6 +200,17 @@ impl DiagnosticSnapshotPort for ControlledDiagnosticPort { .request_document_refresh(root, document_uri, overlay, source_generation) } + fn validate_document_snapshot( + &self, + root: &AdmittedRoot, + document_uri: &str, + overlay: Option<&OverlaySnapshot>, + snapshot: &GenerationDiagnostics, + ) -> DiagnosticSnapshotValidation { + self.adapter + .validate_document_snapshot(root, document_uri, overlay, snapshot) + } + fn retire_document_refresh( &self, root: &AdmittedRoot, @@ -188,6 +227,7 @@ struct Harness { runtime: Arc, authority: Arc, now_ms: u64, + message_duration_ms: u64, next_request_id: u64, } @@ -228,6 +268,7 @@ impl Harness { runtime, authority, now_ms: 0, + message_duration_ms: 1, next_request_id: 1, }; let mut capabilities = json!({ @@ -257,7 +298,7 @@ impl Harness { } fn send(&mut self, value: Value) -> Vec { - self.now_ms += 1; + self.now_ms += self.message_duration_ms; self.session .handle_payload(&serde_json::to_vec(&value).unwrap(), self.now_ms); self.messages() @@ -483,6 +524,229 @@ fn save_refreshes_a_snapshot_even_when_the_overlay_version_is_unchanged() { assert_eq!(harness.runtime.started(), 2); } +#[test] +fn retained_reads_revalidate_authority_before_returning_unchanged() { + let mut harness = Harness::new(false); + harness.open(1); + harness.start_refresh(); + harness.runtime.complete_next(); + let old = assert_full(&harness.pull(None), 1, 1); + *harness.authority.authority_digest.lock().unwrap() = Some(digest('b')); + *harness.authority.next_generation.lock().unwrap() = Some(1); + let stale = harness.pull(Some(&old)); + assert_eq!(stale["error"]["code"], -32802, "{stale}"); + assert!(stale.get("result").is_none()); + harness.runtime.complete_next(); + let current = assert_full(&harness.pull(Some(&old)), 1, 1); + assert_ne!(current, old); + assert_eq!(harness.pull(Some(¤t))["result"]["kind"], "unchanged"); + assert_eq!(harness.runtime.started(), 2); +} + +#[test] +fn changed_authority_discards_queued_stale_publications() { + let mut harness = Harness::new(true); + harness.open(1); + harness.start_refresh(); + harness.runtime.complete_next(); + harness.session.flush_due(harness.now_ms); + *harness.authority.authority_digest.lock().unwrap() = Some(digest('b')); + let messages = harness.send(json!({ + "jsonrpc": "2.0", "id": 999, "method": "textDocument/diagnostic", + "params": { "textDocument": { "uri": DOCUMENT } }, + })); + assert!( + messages + .iter() + .any(|message| { message["id"] == 999 && message["error"]["code"] == -32802 }), + "{messages:?}" + ); + assert!( + !messages.iter().any(|message| { + message["method"] == "textDocument/publishDiagnostics" + && message["params"]["uri"] == DOCUMENT + }), + "{messages:?}" + ); +} + +#[test] +fn pending_authority_validation_is_not_consumed_by_background_publication() { + let mut harness = Harness::new(false); + harness.open(1); + harness.start_refresh(); + harness.runtime.complete_next(); + let result = assert_full(&harness.pull(None), 1, 1); + harness + .authority + .validation_pending + .store(true, Ordering::SeqCst); + let pending = harness.pull(Some(&result)); + assert_eq!(pending["error"]["code"], -32802, "{pending}"); + assert!(pending.get("result").is_none()); + harness.session.flush_due(harness.now_ms); + harness + .authority + .validation_pending + .store(false, Ordering::SeqCst); + harness.runtime.complete_next(); + harness.session.flush_due(harness.now_ms); + assert_eq!(harness.pull(Some(&result))["result"]["kind"], "unchanged"); + assert_eq!(harness.runtime.started(), 2); + assert_eq!( + harness.authority.generation.load(Ordering::SeqCst), + 1, + "validation must not execute a new analyzer refresh" + ); +} + +#[test] +fn dependency_changes_invalidate_retained_document_diagnostics() { + for (method, params) in [ + ( + "textDocument/didSave", + json!({ "textDocument": { "uri": "file:///root/b.rs" } }), + ), + ( + "textDocument/didChange", + json!({ + "textDocument": { "uri": "file:///root/b.rs", "version": 2 }, + "contentChanges": [{ "text": "pub fn changed_dependency() {}" }], + }), + ), + ] { + let mut harness = Harness::new(false); + harness.notify( + "textDocument/didOpen", + json!({ + "textDocument": { + "uri": "file:///root/b.rs", "languageId": "rust", "version": 1, + "text": "pub fn dependency() {}", + }, + }), + ); + harness.open(1); + assert_eq!(harness.pull(None)["error"]["code"], -32802); + harness.runtime.complete_next(); + let old_result = assert_full(&harness.pull(None), 1, 1); + + harness.notify(method, params); + let pending = harness.pull(Some(&old_result)); + assert_eq!(pending["error"]["code"], -32802, "{method}: {pending}"); + assert!(pending.get("result").is_none()); + while harness + .runtime + .0 + .lock() + .unwrap() + .iter() + .any(|task| task.0.lock().unwrap().is_some()) + { + harness.runtime.complete_next(); + } + let generation = harness.authority.generation.load(Ordering::SeqCst); + let new_result = assert_full(&harness.pull(Some(&old_result)), generation, 1); + assert_ne!(new_result, old_result); + let started = harness.runtime.started(); + assert_eq!( + harness.pull(Some(&new_result))["result"]["kind"], + "unchanged" + ); + assert_eq!(harness.runtime.started(), started); + } +} + +#[test] +fn dependency_invalidation_retires_late_work_and_preserves_other_roots() { + let mut harness = Harness::new(false); + harness.notify( + "workspace/didChangeWorkspaceFolders", + json!({ + "event": { + "added": [ + { "uri": "file:///other", "name": "other" }, + { "uri": "file:///root/nested", "name": "nested" }, + ], + "removed": [], + }, + }), + ); + let mutation = harness.session.pending_workspace_folder_mutation().unwrap(); + harness + .session + .apply_workspace_folder_mutation( + &mutation, + AuthorizedLspWorkspace::new( + Some(digest('d')), + vec![ + AdmittedRoot::authorized("file:///root", digest('b')), + AdmittedRoot::authorized("file:///other", digest('e')), + AdmittedRoot::authorized("file:///root/nested", digest('f')), + ], + ) + .unwrap(), + ) + .unwrap(); + let mut unaffected = Vec::new(); + for (index, uri) in ["file:///other/a.rs", "file:///root/nested/a.rs"] + .into_iter() + .enumerate() + { + assert_eq!(harness.pull_document(uri, None)["error"]["code"], -32802); + harness.runtime.complete_next(); + let result = assert_full(&harness.pull_document(uri, None), index as u64 + 1, 0); + unaffected.push((uri, result)); + } + harness.open(1); + assert_eq!(harness.pull(None)["error"]["code"], -32802); + harness.notify( + "textDocument/didSave", + json!({ + "textDocument": { "uri": "file:///root/b.rs" }, + }), + ); + assert_eq!(harness.pull(None)["error"]["code"], -32802); + // The first A operation finishes after B invalidated its ownership. + harness.runtime.complete_next(); + let late = harness.pull(None); + assert_eq!(late["error"]["code"], -32802, "{late}"); + assert!(late.get("result").is_none()); + harness.runtime.complete_next(); + harness.runtime.complete_next(); + assert_full(&harness.pull(None), 5, 1); + for (uri, result) in unaffected { + assert_eq!( + harness.pull_document(uri, Some(&result))["result"]["kind"], + "unchanged" + ); + } + assert_eq!(harness.runtime.started(), 5); +} + +#[test] +fn dependency_invalidation_discards_queued_stale_publications() { + let mut harness = Harness::new(true); + harness.open(1); + harness.start_refresh(); + harness.runtime.complete_next(); + harness.session.flush_due(harness.now_ms); + // Leave A's completed publication queued while the dependency changes. + let messages = harness.notify( + "textDocument/didSave", + json!({ + "textDocument": { "uri": "file:///root/b.rs" }, + }), + ); + assert!( + !messages.iter().any(|message| { + message["method"] == "textDocument/publishDiagnostics" + && message["params"]["uri"] == DOCUMENT + }), + "{messages:?}" + ); + assert_eq!(harness.pull(None)["error"]["code"], -32802); +} + #[test] fn late_completion_cannot_answer_a_reopened_document_lifetime() { let mut harness = Harness::new(false); @@ -652,7 +916,7 @@ fn publication_backpressure_does_not_discard_a_completed_snapshot() { .session .has_outbound_capacity(crate::MAX_PUBLICATION_BYTES) ); - harness.session.poll_diagnostic_refresh(DOCUMENT); + harness.session.poll_diagnostic_refresh(DOCUMENT, false); harness.messages(); assert_full(&harness.pull(None), 1, 1); @@ -801,11 +1065,18 @@ fn a_report_too_large_for_push_remains_available_to_pull_without_polling_again() #[test] fn open_document_snapshots_refuse_new_results_when_the_bound_is_owned() { let mut harness = Harness::new(false); + // Capacity admission is independent of time; all messages share one tick. + harness.message_duration_ms = 0; for index in 0..MAX_WORKSPACE_DIAGNOSTIC_RESULTS { + // Admit all overlays before computing reports: opening another file + // now correctly invalidates dependent reports in the same root. let uri = format!("file:///root/open-{index}.rs"); harness.notify("textDocument/didOpen", json!({ "textDocument": { "uri": uri, "languageId": "rust", "version": 1, "text": "fn a() {}" }, })); + } + for index in 0..MAX_WORKSPACE_DIAGNOSTIC_RESULTS { + let uri = format!("file:///root/open-{index}.rs"); assert_eq!(harness.pull_document(&uri, None)["error"]["code"], -32802); harness.runtime.complete_next(); assert_full(&harness.pull_document(&uri, None), index as u64 + 1, 1); diff --git a/crates/tracedecay-lsp/src/provider.rs b/crates/tracedecay-lsp/src/provider.rs index 2792efb3ae..77357e8c30 100644 --- a/crates/tracedecay-lsp/src/provider.rs +++ b/crates/tracedecay-lsp/src/provider.rs @@ -401,6 +401,15 @@ pub enum DiagnosticSnapshotOutcome { }, } +/// Currentness of a retained report, checked against its canonical authority. +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum DiagnosticSnapshotValidation { + Current, + Changed, + Pending(DiagnosticRefreshIdentity), + Failed { failure_class: String }, +} + /// Port implemented by a daemon/application adapter. It is a read projection; /// the session retains admitted reports only for its ephemeral lifetime. Refresh methods /// are non-blocking: the provider starts canonical upstream/compiler work and @@ -427,6 +436,16 @@ pub trait DiagnosticSnapshotPort { } } + fn validate_document_snapshot( + &self, + _root: &AdmittedRoot, + _document_uri: &str, + _overlay: Option<&OverlaySnapshot>, + _snapshot: &GenerationDiagnostics, + ) -> DiagnosticSnapshotValidation { + DiagnosticSnapshotValidation::Changed + } + /// Releases one refresh's result ownership and requests task cancellation. /// A successful retirement does not prove accepted analyzer work has stopped. fn retire_document_refresh( @@ -489,6 +508,16 @@ where (**self).request_document_refresh(root, document_uri, overlay, source_generation) } + fn validate_document_snapshot( + &self, + root: &AdmittedRoot, + document_uri: &str, + overlay: Option<&OverlaySnapshot>, + snapshot: &GenerationDiagnostics, + ) -> DiagnosticSnapshotValidation { + (**self).validate_document_snapshot(root, document_uri, overlay, snapshot) + } + fn retire_document_refresh( &self, root: &AdmittedRoot, diff --git a/crates/tracedecay-lsp/src/rpc.rs b/crates/tracedecay-lsp/src/rpc.rs index 2fb32ca545..a8cfdeb1e7 100644 --- a/crates/tracedecay-lsp/src/rpc.rs +++ b/crates/tracedecay-lsp/src/rpc.rs @@ -717,15 +717,16 @@ pub(crate) fn diagnostic_result_id( scope_digest: Option<&ManifestDigest>, root_ordinal: Option, generation: u64, + authority: &ManifestDigest, version: i64, ) -> String { match (scope_set_digest, scope_digest, root_ordinal) { (Some(scope_set), Some(scope), Some(root_ordinal)) => format!( - "scope-set={};scope={};root={root_ordinal};generation={generation};version={version}", + "scope-set={};scope={};root={root_ordinal};generation={generation};version={version};authority={authority}", scope_set.as_str(), scope.as_str() ), - _ => format!("generation:{generation}:version:{version}"), + _ => format!("generation:{generation}:version:{version}:authority:{authority}"), } } From 5572f7b183fd50cee19b009bdf2c5835e993a66b Mon Sep 17 00:00:00 2001 From: Zack Jackson Date: Sun, 25 Oct 2026 04:54:26 -0700 Subject: [PATCH 05/12] test(lsp): reproduce retained diagnostic lifecycle failures --- .../src/lsp_support/runtime_adapters.rs | 464 +++++++++++++++++- .../protocol/tests/diagnostic_snapshots.rs | 104 +++- 2 files changed, 546 insertions(+), 22 deletions(-) diff --git a/crates/tracedecay-application/src/lsp_support/runtime_adapters.rs b/crates/tracedecay-application/src/lsp_support/runtime_adapters.rs index c16d904ba2..143b6fbb17 100644 --- a/crates/tracedecay-application/src/lsp_support/runtime_adapters.rs +++ b/crates/tracedecay-application/src/lsp_support/runtime_adapters.rs @@ -457,13 +457,22 @@ fn broker_diagnostic(document_uri: &str, diagnostic: CodeDiagnostic) -> GatewayD #[cfg(test)] mod tests { + use std::collections::{BTreeMap, VecDeque}; + use std::path::Path; + use std::sync::Mutex; use std::sync::atomic::{AtomicUsize, Ordering}; + use serde_json::{Value, json}; use tracedecay_domain::{CodeGenerationId, ContentDigest}; + use tracedecay_lsp::analyzer::adapters::{DiagnosticMode, LspAdapterDefinition}; use tracedecay_lsp::{ - AuthorizedLspWorkspace, CanonicalWorkspaceDiagnosticRefreshRequest, - IndexedWorkspaceDocument, ManagedDiagnosticSnapshot, + AuthorizedLspWorkspace, CanonicalWorkspaceDiagnosticRefreshRequest, ClientCapabilities, + DaemonLspProtocolSession, DiagnosticSnapshotAdapter, FeedbackCyclePort, + FeedbackCycleRequest, FeedbackCycleResponse, GatewayCapabilities, IndexedWorkspaceDocument, + ManagedDiagnosticSnapshot, UnavailableSemanticProvider, UpstreamCapabilities, + negotiate_capabilities, }; + use url::Url; use super::*; @@ -472,6 +481,26 @@ mod tests { struct PublishingWorkspaceIndex { reads: AtomicUsize, drift: WorkspaceIndexDrift, + documents: BTreeMap, + } + + impl PublishingWorkspaceIndex { + fn new(drift: WorkspaceIndexDrift) -> Self { + Self { + reads: AtomicUsize::new(0), + drift, + documents: BTreeMap::from([( + "file:///workspace/src/lib.rs".to_owned(), + LspDocument { + language: "unknown".to_owned(), + language_id: "unknown".to_owned(), + relative_path: "src/lib.rs".to_owned(), + text: "fn main() {}".to_owned(), + snapshot: None, + }, + )]), + } + } } #[derive(Clone, Copy)] @@ -483,16 +512,11 @@ mod tests { impl LspDiagnosticDocumentPort for PublishingWorkspaceIndex { fn load_document( &self, - _request: CanonicalDiagnosticRefreshRequest, + request: CanonicalDiagnosticRefreshRequest, ) -> LspRuntimeFuture> { - Box::pin(async { - Ok(LspDocument { - language: "unknown".to_owned(), - language_id: "unknown".to_owned(), - relative_path: "src/lib.rs".to_owned(), - text: "fn main() {}".to_owned(), - snapshot: None, - }) + let document = self.documents.get(&request.document_uri).cloned(); + Box::pin(async move { + document.ok_or_else(|| LspRuntimeFailure::new("fixture-document-not-found")) }) } } @@ -529,7 +553,10 @@ mod tests { } } - struct FixedManagedDiagnostics; + #[derive(Default)] + struct FixedManagedDiagnostics { + snapshots: AtomicUsize, + } impl ManagedDiagnosticSnapshotPort for FixedManagedDiagnostics { fn current_authority( @@ -543,6 +570,7 @@ mod tests { &self, _request: CanonicalDiagnosticRefreshRequest, ) -> LspRuntimeFuture> { + self.snapshots.fetch_add(1, Ordering::SeqCst); Box::pin(async { Ok(ManagedDiagnosticSnapshot { generation: 1, @@ -555,6 +583,406 @@ mod tests { } } + struct DiagnosticTask(Mutex>>); + + struct DiagnosticTaskHandle(Arc); + + impl LspRuntimeTask for DiagnosticTaskHandle { + fn abort(&self) { + self.0.0.lock().unwrap().take(); + } + } + + #[derive(Default)] + struct DiagnosticRuntime { + tasks: Mutex>>, + started: AtomicUsize, + } + + impl LspRuntimeSpawner for DiagnosticRuntime { + fn spawn(&self, future: LspRuntimeFuture<()>) -> Box { + let task = Arc::new(DiagnosticTask(Mutex::new(Some(future)))); + self.tasks.lock().unwrap().push_back(Arc::clone(&task)); + self.started.fetch_add(1, Ordering::SeqCst); + Box::new(DiagnosticTaskHandle(task)) + } + } + + impl DiagnosticRuntime { + async fn complete_next(&self) { + let task = self.tasks.lock().unwrap().pop_front().unwrap(); + let future = task.0.lock().unwrap().take().unwrap(); + future.await; + } + } + + struct Feedback; + + impl FeedbackCyclePort for Feedback { + fn request_feedback_cycle(&self, _request: FeedbackCycleRequest) -> FeedbackCycleResponse { + FeedbackCycleResponse::Accepted + } + } + + fn file_uri(path: &Path) -> String { + Url::from_file_path(path).unwrap().to_string() + } + + const PUBLISHING_ANALYZER: &str = r#" +import json +import sys +from urllib.parse import urlparse + +def read_message(): + headers = {} + while True: + line = sys.stdin.buffer.readline() + if not line: + return None + if line in (b"\r\n", b"\n"): + break + name, value = line.decode("ascii").split(":", 1) + headers[name.lower()] = value.strip() + return json.loads(sys.stdin.buffer.read(int(headers["content-length"]))) + +def send(payload): + body = json.dumps(payload).encode("utf-8") + sys.stdout.buffer.write(b"Content-Length: " + str(len(body)).encode("ascii") + b"\r\n\r\n" + body) + sys.stdout.buffer.flush() + +while True: + message = read_message() + if message is None: + break + method = message.get("method") + if method == "initialize": + send({"jsonrpc": "2.0", "id": message["id"], "result": {"capabilities": {"textDocumentSync": 1}}}) + elif method in ("textDocument/didOpen", "textDocument/didChange"): + uri = message["params"]["textDocument"]["uri"] + with open(sys.argv[1], encoding="utf-8") as findings: + diagnostics = json.load(findings)[urlparse(uri).path.rsplit("/", 1)[-1]] + send({"jsonrpc": "2.0", "method": "textDocument/publishDiagnostics", "params": { + "uri": uri, "diagnostics": diagnostics, + }}) +"#; + + struct BrokerDiagnosticHarness { + session: DaemonLspProtocolSession< + Feedback, + UnavailableSemanticProvider, + DiagnosticSnapshotAdapter, + >, + runtime: Arc, + broker: Arc>, + documents: Arc, + managed: Arc, + project: tempfile::TempDir, + next_request: u64, + } + + impl BrokerDiagnosticHarness { + fn new(findings: Value) -> Self { + let project = tempfile::tempdir().unwrap(); + let script = project.path().join("analyzer.py"); + let publications = project.path().join("findings.json"); + std::fs::write(&script, PUBLISHING_ANALYZER).unwrap(); + std::fs::write(&publications, serde_json::to_vec(&findings).unwrap()).unwrap(); + let python = if cfg!(windows) { + "python" + } else if std::path::Path::new("/usr/bin/python3").is_file() { + "/usr/bin/python3" + } else { + "python3" + }; + let broker = Arc::new(AsyncMutex::new(DiagnosticBroker::new_for_test( + project.path(), + vec![LspAdapterDefinition { + language: "fake".to_owned(), + language_id: "fake".to_owned(), + command: python.to_owned(), + args: vec![ + script.display().to_string(), + publications.display().to_string(), + ], + extensions: vec!["fake".to_owned()], + root_markers: Vec::new(), + install_options: Vec::new(), + diagnostics: DiagnosticMode::Push, + }], + ))); + let documents = Arc::new(PublishingWorkspaceIndex { + reads: AtomicUsize::new(0), + drift: WorkspaceIndexDrift::None, + documents: ["a.fake", "b.fake"] + .into_iter() + .map(|path| { + let document = LspDocument { + language: "fake".to_owned(), + language_id: "fake".to_owned(), + relative_path: path.to_owned(), + text: "fn main() {}".to_owned(), + snapshot: None, + }; + std::fs::write(project.path().join(path), &document.text).unwrap(); + (file_uri(&project.path().join(path)), document) + }) + .collect(), + }); + let managed = Arc::new(FixedManagedDiagnostics::default()); + let runtime = Arc::new(DiagnosticRuntime::default()); + let authority = Arc::new(BrokerDiagnosticSnapshotAuthority::new( + Arc::clone(&broker), + Arc::clone(&documents), + Arc::clone(&managed), + Duration::from_millis(150), + )); + let root = AdmittedRoot::authorized(file_uri(project.path()), digest('d')); + let workspace = AuthorizedLspWorkspace::new(Some(digest('e')), vec![root]).unwrap(); + let gateway = GatewayCapabilities::default(); + let upstream = UpstreamCapabilities { + supports_diagnostics: true, + ..UpstreamCapabilities::default() + }; + let session = DaemonLspProtocolSession::from_workspace_ports( + workspace, + negotiate_capabilities(&ClientCapabilities::default(), &gateway, &upstream), + gateway, + upstream, + Feedback, + UnavailableSemanticProvider, + DiagnosticSnapshotAdapter::new(runtime.clone(), authority), + ); + let mut harness = Self { + session, + runtime, + broker, + documents, + managed, + project, + next_request: 1, + }; + let initialized = harness.request( + "initialize", + json!({ + "rootUri": file_uri(harness.project.path()), + "capabilities": { + "general": { "positionEncodings": ["utf-16"] }, + "textDocument": { "diagnostic": {} }, + }, + }), + ); + assert!(initialized.get("error").is_none(), "{initialized}"); + harness.session.handle_payload( + &serde_json::to_vec(&json!({ + "jsonrpc": "2.0", "method": "initialized", "params": {}, + })) + .unwrap(), + 1, + ); + assert!(harness.session.drain_outbound().is_empty()); + harness + } + + fn uri(&self, path: &str) -> String { + file_uri(&self.project.path().join(path)) + } + + fn request(&mut self, method: &str, params: Value) -> Value { + let id = self.next_request; + self.next_request += 1; + self.session.handle_payload( + &serde_json::to_vec(&json!({ + "jsonrpc": "2.0", "id": id, "method": method, "params": params, + })) + .unwrap(), + id, + ); + self.session + .drain_outbound() + .iter() + .map(|message| serde_json::from_slice::(message).unwrap()) + .find(|message| message["id"] == id) + .expect("diagnostic protocol response") + } + + fn pull(&mut self, uri: &str, previous: Option<&str>) -> Value { + let mut params = json!({ "textDocument": { "uri": uri } }); + if let Some(previous) = previous { + params["previousResultId"] = json!(previous); + } + self.request("textDocument/diagnostic", params) + } + + async fn warm(&mut self, uri: &str) -> Value { + let pending = self.pull(uri, None); + assert_eq!(pending["error"]["code"], -32802, "{pending}"); + assert_eq!(pending["error"]["data"]["coverage"], "refresh-required"); + self.runtime.complete_next().await; + let full = self.pull(uri, None); + assert_eq!(full["result"]["kind"], "full", "{full}"); + full["result"].clone() + } + + async fn restart_analyzer_and_republish(&self, uri: &str) -> DiagnosticsSnapshot { + let document = self.documents.documents.get(uri).unwrap().clone(); + let mut broker = self.broker.lock().await; + let settings = broker.settings().clone(); + broker.update_settings(settings); + broker + .refresh_documents("fake", vec![document], Duration::from_millis(150)) + .await + .unwrap(); + broker.snapshot() + } + } + + fn analyzer_finding(message: &str, start: u32, end: u32) -> Value { + json!({ + "range": { "start": { "line": 0, "character": start }, "end": { "line": 0, "character": end } }, + "severity": 1, + "source": "fake-ls", + "message": message, + }) + } + + #[tokio::test] + async fn retained_report_rejects_changed_broker_findings_with_identical_coverage() { + let original_finding = analyzer_finding("first finding", 0, 1); + let mut harness = BrokerDiagnosticHarness::new(json!({ + "a.fake": [original_finding], "b.fake": [], + })); + let uri = harness.uri("a.fake"); + let original = harness.warm(&uri).await; + let result_id = original["resultId"].as_str().unwrap(); + assert_eq!(original["items"][0]["message"], "first finding"); + let before = harness.broker.lock().await.snapshot(); + + let identical = harness.restart_analyzer_and_republish(&uri).await; + assert_eq!(identical.coverage, before.coverage); + assert_eq!(identical.settings, before.settings); + assert_eq!(identical.diagnostics.len(), 1); + assert_eq!(identical.diagnostics[0].message, "first finding"); + assert_eq!(identical.diagnostics[0].character_start, Some(0)); + assert_eq!(identical.diagnostics[0].character_end, Some(1)); + assert_eq!( + harness.pull(&uri, Some(result_id))["result"], + json!({ "kind": "unchanged", "resultId": result_id }) + ); + assert_eq!(harness.managed.snapshots.load(Ordering::SeqCst), 1); + assert_eq!(harness.runtime.started.load(Ordering::SeqCst), 1); + + let changed_finding = analyzer_finding("replacement finding", 3, 7); + std::fs::write( + harness.project.path().join("findings.json"), + serde_json::to_vec(&json!({ "a.fake": [changed_finding], "b.fake": [] })).unwrap(), + ) + .unwrap(); + let changed = harness.restart_analyzer_and_republish(&uri).await; + assert_eq!(changed.coverage, before.coverage); + assert_eq!(changed.settings, before.settings); + assert_eq!(changed.settings_unavailable, before.settings_unavailable); + assert_eq!(changed.diagnostics.len(), 1); + assert_eq!(changed.diagnostics[0].message, "replacement finding"); + assert_eq!(changed.diagnostics[0].character_start, Some(3)); + assert_eq!(changed.diagnostics[0].character_end, Some(7)); + + let stale = harness.pull(&uri, Some(result_id)); + assert_eq!(stale["error"]["code"], -32802, "{stale}"); + assert!( + stale.get("result").is_none(), + "changed findings must reject retained reuse" + ); + harness.runtime.complete_next().await; + let replacement = harness.pull(&uri, Some(result_id)); + assert_eq!(replacement["result"]["kind"], "full", "{replacement}"); + assert_ne!(replacement["result"]["resultId"], original["resultId"]); + assert_eq!( + replacement["result"]["items"], + json!([{ + "range": { "start": { "line": 0, "character": 3 }, "end": { "line": 0, "character": 7 } }, + "severity": 1, "source": "upstream", "message": "replacement finding", + }]) + ); + assert_eq!(harness.managed.snapshots.load(Ordering::SeqCst), 2); + assert_eq!(harness.runtime.started.load(Ordering::SeqCst), 2); + } + + #[tokio::test] + async fn retained_reports_survive_other_document_refreshes_in_the_same_language() { + let mut harness = BrokerDiagnosticHarness::new(json!({ + "a.fake": [analyzer_finding("retained finding", 0, 1)], "b.fake": [], + })); + let a = harness.uri("a.fake"); + let b = harness.uri("b.fake"); + let report_a = harness.warm(&a).await; + let report_b = harness.warm(&b).await; + assert_eq!(report_a["items"][0]["message"], "retained finding"); + assert_eq!(report_b["items"], json!([])); + let latest = harness.broker.lock().await.snapshot(); + assert_eq!(latest.diagnostics.len(), 1); + assert_eq!(latest.diagnostics[0].file, "a.fake"); + assert_eq!( + latest.coverage["fake"] + .documents + .keys() + .map(String::as_str) + .collect::>(), + vec!["b.fake"], + "dashboard coverage must still describe only the latest refresh" + ); + assert_eq!(harness.managed.snapshots.load(Ordering::SeqCst), 2); + assert_eq!(harness.runtime.started.load(Ordering::SeqCst), 2); + + for (uri, report) in [ + (&a, &report_a), + (&b, &report_b), + (&a, &report_a), + (&b, &report_b), + ] { + let result_id = report["resultId"].as_str().unwrap(); + let reused = harness.pull(uri, Some(result_id)); + assert_eq!( + reused["result"], + json!({ "kind": "unchanged", "resultId": result_id }), + "unchanged document must survive another document's refresh: {reused}" + ); + assert_eq!(harness.runtime.started.load(Ordering::SeqCst), 2); + assert_eq!(harness.managed.snapshots.load(Ordering::SeqCst), 2); + } + + let document_a = harness.documents.documents[&a].clone(); + harness + .broker + .lock() + .await + .refresh_documents("fake", vec![document_a], Duration::from_millis(150)) + .await + .unwrap(); + assert_eq!( + harness.pull(&b, report_b["resultId"].as_str())["result"], + json!({ "kind": "unchanged", "resultId": report_b["resultId"] }), + "a clean publication must also survive a later refresh of another document" + ); + assert_eq!(harness.managed.snapshots.load(Ordering::SeqCst), 2); + assert_eq!(harness.runtime.started.load(Ordering::SeqCst), 2); + + harness + .broker + .lock() + .await + .set_language_enabled("fake", false); + let stale = harness.pull(&a, report_a["resultId"].as_str()); + assert_eq!(stale["error"]["code"], -32802, "{stale}"); + assert!(stale.get("result").is_none()); + harness.runtime.complete_next().await; + let disabled = harness.pull(&a, report_a["resultId"].as_str()); + assert_eq!(disabled["result"]["kind"], "full", "{disabled}"); + assert_eq!(disabled["result"]["items"], json!([])); + assert_ne!(disabled["result"]["resultId"], report_a["resultId"]); + assert_eq!(harness.managed.snapshots.load(Ordering::SeqCst), 3); + assert_eq!(harness.runtime.started.load(Ordering::SeqCst), 3); + } + #[tokio::test] async fn workspace_sweep_rejects_a_generation_published_before_completion() { let root = AdmittedRoot::authorized("file:///workspace", digest('d')); @@ -564,11 +992,8 @@ mod tests { "/workspace", Vec::new(), ))), - Arc::new(PublishingWorkspaceIndex { - reads: AtomicUsize::new(0), - drift: WorkspaceIndexDrift::Publish, - }), - Arc::new(FixedManagedDiagnostics), + Arc::new(PublishingWorkspaceIndex::new(WorkspaceIndexDrift::Publish)), + Arc::new(FixedManagedDiagnostics::default()), Duration::from_millis(1), ); @@ -619,10 +1044,7 @@ mod tests { "/workspace", Vec::new(), ))), - Arc::new(PublishingWorkspaceIndex { - reads: AtomicUsize::new(0), - drift: WorkspaceIndexDrift::None, - }), + Arc::new(PublishingWorkspaceIndex::new(WorkspaceIndexDrift::None)), Arc::new(MismatchedManagedDiagnostics), Duration::from_millis(1), ); diff --git a/crates/tracedecay-lsp/src/protocol/tests/diagnostic_snapshots.rs b/crates/tracedecay-lsp/src/protocol/tests/diagnostic_snapshots.rs index 76aba96bff..7d4da3964b 100644 --- a/crates/tracedecay-lsp/src/protocol/tests/diagnostic_snapshots.rs +++ b/crates/tracedecay-lsp/src/protocol/tests/diagnostic_snapshots.rs @@ -17,7 +17,7 @@ use crate::gateway::{ }; use crate::overlay::{ CanonicalDiagnosticRefreshRequest, CanonicalDiagnosticSnapshotAuthority, - DiagnosticSnapshotAdapter, OverlaySnapshot, + DiagnosticSnapshotAdapter, MAX_DIAGNOSTIC_OPERATIONS, OverlaySnapshot, }; use crate::protocol::{DaemonLspProtocolSession, MAX_QUEUED_OUTBOUND_MESSAGES}; use crate::provider::{ @@ -233,6 +233,10 @@ struct Harness { impl Harness { fn new(push: bool) -> Self { + Self::with_refresh_support(push, false) + } + + fn with_refresh_support(push: bool, refresh_support: bool) -> Self { let runtime = Arc::new(ControlledRuntime::default()); let authority = Arc::new(DiagnosticAuthority::default()); let provider = ControlledDiagnosticPort { @@ -279,6 +283,9 @@ impl Harness { if push { capabilities["textDocument"]["publishDiagnostics"] = json!({ "versionSupport": true }); } + if refresh_support { + capabilities["workspace"]["diagnostic"] = json!({ "refreshSupport": true }); + } let initialized = harness.request( "initialize", json!({ "rootUri": "file:///root", "capabilities": capabilities }), @@ -294,6 +301,15 @@ impl Harness { .supports_publish_diagnostics, push, ); + assert_eq!( + harness + .session + .lifecycle + .gateway + .capabilities() + .supports_workspace_diagnostic_refresh, + refresh_support, + ); harness } @@ -600,6 +616,92 @@ fn pending_authority_validation_is_not_consumed_by_background_publication() { ); } +#[test] +fn settled_pull_validations_release_capacity_without_client_retries() { + let mut harness = Harness::with_refresh_support(true, true); + let mut reports = Vec::new(); + for index in 0..MAX_DIAGNOSTIC_OPERATIONS { + let uri = format!("file:///root/retained-{index:03}.rs"); + assert_eq!(harness.pull_document(&uri, None)["error"]["code"], -32802); + harness.runtime.complete_next(); + let result_id = assert_full(&harness.pull_document(&uri, None), index as u64 + 1, 0); + reports.push((uri, result_id)); + } + assert_eq!(harness.runtime.started(), reports.len()); + harness + .authority + .validation_pending + .store(true, Ordering::SeqCst); + for (uri, result_id) in &reports { + let pending = harness.pull_document(uri, Some(result_id)); + assert_eq!(pending["error"]["code"], -32802, "{pending}"); + assert!(pending["error"]["data"]["operationId"].as_str().is_some()); + assert!(pending.get("result").is_none()); + } + assert_eq!(harness.runtime.started(), reports.len() * 2); + + let fresh_uri = "file:///root/fresh.rs"; + let saturated = harness.pull_document(fresh_uri, None); + assert_eq!(saturated["error"]["code"], -32802, "{saturated}"); + assert_eq!( + saturated["error"]["data"]["failureClass"], + "diagnostic-capacity" + ); + assert_eq!(harness.runtime.started(), reports.len() * 2); + + harness + .authority + .validation_pending + .store(false, Ordering::SeqCst); + for _ in &reports { + harness.runtime.complete_next(); + } + harness.session.flush_due(harness.now_ms); + assert!( + harness.messages().is_empty(), + "validation settlement must emit neither diagnostics nor a workspace refresh" + ); + + let admitted = harness.pull_document(fresh_uri, None); + assert_eq!(admitted["error"]["code"], -32802, "{admitted}"); + assert_eq!( + admitted["error"]["data"]["coverage"], "refresh-required", + "{admitted}" + ); + let started = harness.runtime.started(); + assert_eq!( + started, + reports.len() * 2 + 1, + "settled validations must release capacity without another pull: {admitted}" + ); + harness + .authority + .validation_pending + .store(true, Ordering::SeqCst); + let (uri, result_id) = reports.last().unwrap(); + assert_eq!( + harness.pull_document(uri, Some(result_id))["result"], + json!({ "kind": "unchanged", "resultId": result_id }), + "a surviving report must consume its settled validation" + ); + assert_eq!(harness.runtime.started(), started); + assert_eq!( + harness.authority.generation.load(Ordering::SeqCst), + reports.len() as u64 + ); + + harness + .authority + .validation_pending + .store(false, Ordering::SeqCst); + harness.runtime.complete_next(); + assert_full( + &harness.pull_document(fresh_uri, None), + reports.len() as u64 + 1, + 0, + ); +} + #[test] fn dependency_changes_invalidate_retained_document_diagnostics() { for (method, params) in [ From 648bf9365d05a91bf56ef0466f76796bac11d895 Mon Sep 17 00:00:00 2001 From: Zack Jackson Date: Sun, 25 Oct 2026 05:39:40 -0700 Subject: [PATCH 06/12] fix(lsp): settle retained diagnostic validations without retries --- .../src/protocol/diagnostic_snapshots.rs | 212 +++++++++----- .../src/protocol/diagnostics_controller.rs | 84 ++++-- .../protocol/tests/diagnostic_snapshots.rs | 269 +++++++++++++++++- 3 files changed, 476 insertions(+), 89 deletions(-) diff --git a/crates/tracedecay-lsp/src/protocol/diagnostic_snapshots.rs b/crates/tracedecay-lsp/src/protocol/diagnostic_snapshots.rs index 80d3fb7449..35876f38dd 100644 --- a/crates/tracedecay-lsp/src/protocol/diagnostic_snapshots.rs +++ b/crates/tracedecay-lsp/src/protocol/diagnostic_snapshots.rs @@ -48,12 +48,26 @@ impl DiagnosticDocumentIdentity { pub(super) struct PendingDiagnosticRefresh { pub(super) identity: DiagnosticRefreshIdentity, pub(super) document: DiagnosticDocumentIdentity, - pub(super) validation_only: bool, + pub(super) state: DiagnosticRefreshState, +} + +#[derive(Clone, Copy, Eq, PartialEq)] +pub(super) enum DiagnosticRefreshState { + Refreshing, + ValidatingPull, + PublicationPending, +} + +#[derive(Clone, Copy, Eq, PartialEq)] +pub(super) enum DiagnosticSnapshotDelivery { + Admitted, + SettledValidation, } pub(super) struct RetainedDiagnosticSnapshot { document: DiagnosticDocumentIdentity, outcome: DiagnosticSnapshotOutcome, + delivery: DiagnosticSnapshotDelivery, } impl DaemonLspProtocolSession @@ -97,83 +111,99 @@ where .diagnostics .provider .document_diagnostics(root, uri, overlay); - let outcome = match outcome { - DiagnosticSnapshotOutcome::Partial { ref coverage, .. } - if coverage == "refresh-required" => - { - let Some(retained) = self.diagnostics.document_snapshots.get(uri) else { - return outcome; - }; - let DiagnosticSnapshotOutcome::Ready { diagnostics, .. } = &retained.outcome else { - return retained.outcome.clone(); - }; - let source_generation = Some(diagnostics.generation); - let validation = self.diagnostics.provider.validate_document_snapshot( - root, - uri, - overlay, - diagnostics, - ); - let outcome = match validation { - DiagnosticSnapshotValidation::Current => return retained.outcome.clone(), - DiagnosticSnapshotValidation::Changed => DiagnosticSnapshotOutcome::Partial { - source_generation, - coverage: "refresh-required".to_owned(), - }, - DiagnosticSnapshotValidation::Pending(identity) => { - if valid_refresh_identity(&identity, source_generation) { - self.diagnostics.active_refreshes.insert( - uri.to_owned(), - PendingDiagnosticRefresh { - identity: identity.clone(), - document, - validation_only: for_pull, + if matches!(&outcome, DiagnosticSnapshotOutcome::Partial { coverage, .. } + if coverage == "refresh-required") + { + let Some(retained) = self.diagnostics.document_snapshots.get(uri) else { + return outcome; + }; + let DiagnosticSnapshotOutcome::Ready { diagnostics, .. } = &retained.outcome else { + return retained.outcome.clone(); + }; + let source_generation = Some(diagnostics.generation); + let validation = self.diagnostics.provider.validate_document_snapshot( + root, + uri, + overlay, + diagnostics, + ); + let outcome = match validation { + DiagnosticSnapshotValidation::Current => return retained.outcome.clone(), + DiagnosticSnapshotValidation::Changed => DiagnosticSnapshotOutcome::Partial { + source_generation, + coverage: "refresh-required".to_owned(), + }, + DiagnosticSnapshotValidation::Pending(identity) => { + if valid_refresh_identity(&identity, source_generation) { + self.diagnostics.active_refreshes.insert( + uri.to_owned(), + PendingDiagnosticRefresh { + identity: identity.clone(), + document, + state: if for_pull { + DiagnosticRefreshState::ValidatingPull + } else { + DiagnosticRefreshState::Refreshing }, - ); - DiagnosticSnapshotOutcome::Refreshing(identity) - } else { - let _ = self.diagnostics.provider.retire_document_refresh( - root, - uri, - &identity.operation_id, - ); - DiagnosticSnapshotOutcome::Failed { - source_generation, - failure_class: "invalid-refresh-identity".to_owned(), - } - } - } - DiagnosticSnapshotValidation::Failed { failure_class } => { + }, + ); + DiagnosticSnapshotOutcome::Refreshing(identity) + } else { + let _ = self.diagnostics.provider.retire_document_refresh( + root, + uri, + &identity.operation_id, + ); DiagnosticSnapshotOutcome::Failed { source_generation, - failure_class, + failure_class: "invalid-refresh-identity".to_owned(), } } - }; - self.diagnostics.document_snapshots.remove(uri); - self.discard_queued_document_publications(uri); - return outcome; - } + } + DiagnosticSnapshotValidation::Failed { failure_class } => { + DiagnosticSnapshotOutcome::Failed { + source_generation, + failure_class, + } + } + }; + self.diagnostics.document_snapshots.remove(uri); + self.discard_queued_document_publications(uri); + return outcome; + } + self.admit_document_diagnostic_snapshot( + uri, + document, + outcome, + DiagnosticSnapshotDelivery::Admitted, + ) + } + + pub(super) fn admit_document_diagnostic_snapshot( + &mut self, + uri: &str, + document: DiagnosticDocumentIdentity, + outcome: DiagnosticSnapshotOutcome, + delivery: DiagnosticSnapshotDelivery, + ) -> DiagnosticSnapshotOutcome { + let outcome = match outcome { DiagnosticSnapshotOutcome::Ready { mut diagnostics, completed_operation_id, } => { - if let Some(operation_id) = completed_operation_id.as_deref() { - let pending = self.diagnostics.active_refreshes.get(uri); - if !pending.is_some_and(|pending| { - pending.document == document - && pending.identity.operation_id == operation_id - }) { - return pending.map_or_else( - || DiagnosticSnapshotOutcome::Partial { - source_generation: None, - coverage: "refresh-required".to_owned(), - }, - |pending| { - DiagnosticSnapshotOutcome::Refreshing(pending.identity.clone()) - }, - ); + let pending = self.diagnostics.active_refreshes.get(uri); + if let Some(pending) = pending { + if pending.document != document + || completed_operation_id.as_deref() + != Some(pending.identity.operation_id.as_str()) + { + return DiagnosticSnapshotOutcome::Refreshing(pending.identity.clone()); } + } else if completed_operation_id.is_some() { + return DiagnosticSnapshotOutcome::Partial { + source_generation: None, + coverage: "refresh-required".to_owned(), + }; } let invalid_generation = self .diagnostics @@ -197,7 +227,7 @@ where .is_some_and(|published| { published.version == document.version && published.generation > diagnostics.generation - && published.scope_digest == root.scope_digest().cloned() + && published.scope_digest == document.root.scope_digest().cloned() && published.scope_set_digest == document.scope_set_digest }) || self @@ -256,7 +286,7 @@ where PublishedDiagnostic { version: document.version, generation: diagnostics.generation, - scope_digest: root.scope_digest().cloned(), + scope_digest: document.root.scope_digest().cloned(), scope_set_digest: document.scope_set_digest.clone(), }, ); @@ -266,11 +296,53 @@ where RetainedDiagnosticSnapshot { document, outcome: outcome.clone(), + delivery, }, ); outcome } + pub(super) fn take_settled_diagnostic_validation( + &mut self, + uri: &str, + document: &DiagnosticDocumentIdentity, + ) -> Option { + let retained = self.diagnostics.document_snapshots.get_mut(uri)?; + if retained.document != *document { + self.diagnostics.document_snapshots.remove(uri); + return None; + } + if retained.delivery != DiagnosticSnapshotDelivery::SettledValidation { + return None; + } + let outcome = retained.outcome.clone(); + if matches!(&outcome, DiagnosticSnapshotOutcome::Ready { .. }) { + retained.delivery = DiagnosticSnapshotDelivery::Admitted; + } else { + self.diagnostics.document_snapshots.remove(uri); + } + Some(outcome) + } + + pub(super) fn pending_diagnostic_publication( + &self, + uri: &str, + pending: &PendingDiagnosticRefresh, + ) -> DiagnosticSnapshotOutcome { + if let Some(retained) = self.diagnostics.document_snapshots.get(uri) + && retained.document == pending.document + && matches!(&retained.outcome, DiagnosticSnapshotOutcome::Ready { + completed_operation_id: Some(operation_id), .. + } if operation_id == &pending.identity.operation_id) + { + return retained.outcome.clone(); + } + DiagnosticSnapshotOutcome::Failed { + source_generation: pending.identity.source_generation, + failure_class: "diagnostic-publication-unavailable".to_owned(), + } + } + pub(super) fn reserve_document_diagnostic_snapshot(&mut self, uri: &str) -> bool { if self.diagnostics.document_snapshots.contains_key(uri) || self.diagnostics.document_snapshots.len() < MAX_WORKSPACE_DIAGNOSTIC_RESULTS diff --git a/crates/tracedecay-lsp/src/protocol/diagnostics_controller.rs b/crates/tracedecay-lsp/src/protocol/diagnostics_controller.rs index 8633d3bdc9..353b556e45 100644 --- a/crates/tracedecay-lsp/src/protocol/diagnostics_controller.rs +++ b/crates/tracedecay-lsp/src/protocol/diagnostics_controller.rs @@ -1,5 +1,6 @@ use super::diagnostic_snapshots::{ - DiagnosticDocumentIdentity, PendingDiagnosticRefresh, RetainedDiagnosticSnapshot, + DiagnosticDocumentIdentity, DiagnosticRefreshState, DiagnosticSnapshotDelivery, + PendingDiagnosticRefresh, RetainedDiagnosticSnapshot, }; use super::outbound_controller::{PublicationRejection, PublicationTag}; use super::workspace_diagnostics_controller::WorkspaceDiagnosticResultIdentity; @@ -263,16 +264,39 @@ where self.lifecycle.gateway.request_document_diagnostics(uri), |()| Value::Null, )?; - let completed = self.poll_diagnostic_refresh(uri, true); let overlay = self.lifecycle.overlays.snapshot(uri); let root = self.document_root(uri)?; - let outcome = completed - .filter(|outcome| !matches!(outcome, DiagnosticSnapshotOutcome::Refreshing(_))) - .unwrap_or_else(|| { - self.document_diagnostic_snapshot(&root, uri, overlay.as_ref(), true) + let document = DiagnosticDocumentIdentity::new( + &root, + self.lifecycle.gateway.workspace().scope_set_digest(), + overlay.as_ref(), + ); + let mut settled = self.take_settled_diagnostic_validation(uri, &document); + let validating = self + .diagnostics + .active_refreshes + .get(uri) + .is_some_and(|pending| { + pending.document == document + && pending.state == DiagnosticRefreshState::ValidatingPull }); + let completed = if settled.is_none() { + let completed = self.poll_diagnostic_refresh(uri); + settled = self.take_settled_diagnostic_validation(uri, &document); + completed + } else { + None + }; + let consumed_validation = settled.is_some() + || (validating + && completed.as_ref().is_some_and(|outcome| { + !matches!(outcome, DiagnosticSnapshotOutcome::Refreshing(_)) + })); + let outcome = settled.or(completed).unwrap_or_else(|| { + self.document_diagnostic_snapshot(&root, uri, overlay.as_ref(), true) + }); let version = overlay.as_ref().map_or(0, |overlay| overlay.version); - if !matches!(&outcome, DiagnosticSnapshotOutcome::Ready { .. }) { + if !consumed_validation && !matches!(&outcome, DiagnosticSnapshotOutcome::Ready { .. }) { let source_generation = diagnostic_source_generation(&outcome); if let Some(failure_class) = self.request_diagnostic_refresh(uri, overlay.as_ref(), source_generation) @@ -462,7 +486,7 @@ where PendingDiagnosticRefresh { identity, document, - validation_only: false, + state: DiagnosticRefreshState::Refreshing, }, ); None @@ -472,20 +496,20 @@ where } pub(super) fn poll_diagnostic_refreshes(&mut self) { - // Pull validation belongs to the requesting reader. Background polling - // must not consume its one-shot completion before that reader retries. - let documents: Vec<_> = self + let (validations, publications): (Vec<_>, Vec<_>) = self .diagnostics .active_refreshes .iter() - .filter(|(_, pending)| !pending.validation_only) - .map(|(uri, _)| uri.clone()) - .collect(); - for document in documents { + .map(|(uri, pending)| (uri.clone(), pending.state)) + .partition(|(_, state)| *state == DiagnosticRefreshState::ValidatingPull); + for (uri, _) in validations { + self.poll_diagnostic_refresh(&uri); + } + for (uri, _) in publications { if !self.has_outbound_capacity(MAX_PUBLICATION_BYTES) { break; } - self.poll_diagnostic_refresh(&document, false); + self.poll_diagnostic_refresh(&uri); } } @@ -493,7 +517,6 @@ where pub(super) fn poll_diagnostic_refresh( &mut self, uri: &str, - for_pull: bool, ) -> Option { let pending = self.diagnostics.active_refreshes.get(uri).cloned()?; let overlay = self.lifecycle.overlays.snapshot(uri); @@ -512,7 +535,30 @@ where self.invalidate_document_diagnostics(uri); return None; } - let outcome = self.document_diagnostic_snapshot(&root, uri, overlay.as_ref(), for_pull); + let outcome = if pending.state == DiagnosticRefreshState::PublicationPending { + self.pending_diagnostic_publication(uri, &pending) + } else { + let outcome = + self.diagnostics + .provider + .document_diagnostics(&root, uri, overlay.as_ref()); + self.admit_document_diagnostic_snapshot( + uri, + pending.document.clone(), + outcome, + if pending.state == DiagnosticRefreshState::ValidatingPull { + DiagnosticSnapshotDelivery::SettledValidation + } else { + DiagnosticSnapshotDelivery::Admitted + }, + ) + }; + if pending.state == DiagnosticRefreshState::ValidatingPull { + if !matches!(&outcome, DiagnosticSnapshotOutcome::Refreshing(_)) { + self.retire_diagnostic_refresh(uri); + } + return Some(outcome); + } match outcome.clone() { DiagnosticSnapshotOutcome::Ready { diagnostics, @@ -524,6 +570,8 @@ where != Err(PublicationRejection::Backpressured) { self.retire_diagnostic_refresh(uri); + } else if let Some(pending) = self.diagnostics.active_refreshes.get_mut(uri) { + pending.state = DiagnosticRefreshState::PublicationPending; } } DiagnosticSnapshotOutcome::Partial { .. } diff --git a/crates/tracedecay-lsp/src/protocol/tests/diagnostic_snapshots.rs b/crates/tracedecay-lsp/src/protocol/tests/diagnostic_snapshots.rs index 7d4da3964b..1e1c7373c9 100644 --- a/crates/tracedecay-lsp/src/protocol/tests/diagnostic_snapshots.rs +++ b/crates/tracedecay-lsp/src/protocol/tests/diagnostic_snapshots.rs @@ -78,6 +78,7 @@ struct DiagnosticAuthority { next_code: Mutex>, authority_digest: Arc>>, validation_pending: Arc, + validation_failure: Arc>>, } impl CanonicalDiagnosticSnapshotAuthority for DiagnosticAuthority { @@ -87,9 +88,12 @@ impl CanonicalDiagnosticSnapshotAuthority for DiagnosticAuthority { ) -> LspRuntimeFuture> { let digest = Arc::clone(&self.authority_digest); let pending = Arc::clone(&self.validation_pending); + let failure = Arc::clone(&self.validation_failure); Box::pin(std::future::poll_fn(move |_| { if pending.load(Ordering::SeqCst) { Poll::Pending + } else if let Some(failure) = failure.lock().unwrap().take() { + Poll::Ready(Err(LspRuntimeFailure::new(failure))) } else { Poll::Ready(Ok(digest .lock() @@ -1018,7 +1022,7 @@ fn publication_backpressure_does_not_discard_a_completed_snapshot() { .session .has_outbound_capacity(crate::MAX_PUBLICATION_BYTES) ); - harness.session.poll_diagnostic_refresh(DOCUMENT, false); + harness.session.poll_diagnostic_refresh(DOCUMENT); harness.messages(); assert_full(&harness.pull(None), 1, 1); @@ -1197,3 +1201,266 @@ fn open_document_snapshots_refuse_new_results_when_the_bound_is_owned() { ); assert_eq!(harness.runtime.started(), MAX_WORKSPACE_DIAGNOSTIC_RESULTS); } + +fn fill_outbound_queue(harness: &mut Harness) -> usize { + let mut queued = 0; + for _ in 0..MAX_QUEUED_OUTBOUND_MESSAGES { + if !harness.session.enqueue_value(json!({ + "jsonrpc": "2.0", "method": "window/logMessage", + "params": { "type": 3, "message": "transport occupied" }, + })) { + break; + } + queued += 1; + } + assert!( + !harness + .session + .has_outbound_capacity(crate::MAX_PUBLICATION_BYTES) + ); + queued +} + +fn validation_harness() -> (Harness, String) { + let mut harness = Harness::with_refresh_support(true, true); + harness.open(1); + harness.start_refresh(); + harness.runtime.complete_next(); + let messages = harness.send(json!({ + "jsonrpc": "2.0", "id": "initial-report", "method": "textDocument/diagnostic", + "params": { "textDocument": { "uri": DOCUMENT } }, + })); + let report = messages + .iter() + .find(|message| message["id"] == "initial-report") + .unwrap(); + let result_id = assert_full(report, 1, 1); + let refresh = messages + .iter() + .find(|message| message["method"] == "workspace/diagnostic/refresh") + .unwrap(); + assert!( + harness + .send(json!({ + "jsonrpc": "2.0", "id": refresh["id"], "result": null, + })) + .is_empty() + ); + (harness, result_id) +} + +fn assert_settled_validation_failure(failure: Option<&'static str>, background: bool) { + let (mut harness, result_id) = validation_harness(); + harness + .authority + .validation_pending + .store(true, Ordering::SeqCst); + assert_eq!(harness.pull(Some(&result_id))["error"]["code"], -32802); + assert_eq!(harness.runtime.started(), 2); + + *harness.authority.validation_failure.lock().unwrap() = failure; + if failure.is_none() { + *harness.authority.authority_digest.lock().unwrap() = Some(digest('b')); + } + harness + .authority + .validation_pending + .store(false, Ordering::SeqCst); + harness.runtime.complete_next(); + if background { + let queued = fill_outbound_queue(&mut harness); + harness.session.flush_due(harness.now_ms); + let messages = harness.messages(); + assert_eq!(messages.len(), queued); + assert!( + messages + .iter() + .all(|message| message["method"] == "window/logMessage") + ); + assert!(harness.session.diagnostics.active_refreshes.is_empty()); + } + + let messages = harness.send(json!({ + "jsonrpc": "2.0", "id": "validation-result", "method": "textDocument/diagnostic", + "params": { "textDocument": { "uri": DOCUMENT }, "previousResultId": result_id }, + })); + assert_eq!( + messages.len(), + 1, + "validation consumption must emit only its response: {messages:?}" + ); + let failed = &messages[0]; + assert_eq!(failed["id"], "validation-result"); + assert_eq!(failed["error"]["code"], -32802, "{failed}"); + assert!(failed.get("result").is_none()); + match failure { + Some(failure) => assert_eq!(failed["error"]["data"]["failureClass"], failure), + None => assert_eq!(failed["error"]["data"]["coverage"], "refresh-required"), + } + assert_eq!( + harness.runtime.started(), + 2, + "consuming a terminal validation must not admit refresh work" + ); + assert!(harness.session.diagnostics.active_refreshes.is_empty()); + assert!(harness.session.diagnostics.document_snapshots.is_empty()); + assert_eq!(harness.authority.generation.load(Ordering::SeqCst), 1); + + let retry = harness.pull(Some(&result_id)); + assert_eq!( + retry["error"]["data"]["coverage"], "refresh-required", + "{retry}" + ); + assert_eq!(harness.runtime.started(), 3); + harness.runtime.complete_next(); + assert_full(&harness.pull(Some(&result_id)), 2, 1); +} + +#[test] +fn partial_pull_validation_settles_under_backpressure_and_is_consumed_once() { + assert_settled_validation_failure(None, true); +} + +#[test] +fn failed_pull_validation_settles_under_backpressure_and_is_consumed_once() { + assert_settled_validation_failure(Some("provider-unavailable"), true); +} + +#[test] +fn directly_consumed_failed_validation_does_not_admit_a_refresh() { + assert_settled_validation_failure(Some("provider-unavailable"), false); +} + +#[test] +fn settled_ready_validation_is_consumed_once_and_preserves_the_generation_floor() { + let (mut harness, result_id) = validation_harness(); + harness + .authority + .validation_pending + .store(true, Ordering::SeqCst); + assert_eq!(harness.pull(Some(&result_id))["error"]["code"], -32802); + harness + .authority + .validation_pending + .store(false, Ordering::SeqCst); + harness.runtime.complete_next(); + let queued = fill_outbound_queue(&mut harness); + harness.session.flush_due(harness.now_ms); + let messages = harness.messages(); + assert_eq!(messages.len(), queued); + assert!( + messages + .iter() + .all(|message| message["method"] == "window/logMessage") + ); + assert!(harness.session.diagnostics.active_refreshes.is_empty()); + + harness + .authority + .validation_pending + .store(true, Ordering::SeqCst); + assert_eq!( + harness.pull(Some(&result_id))["result"]["kind"], + "unchanged" + ); + assert_eq!(harness.runtime.started(), 2); + let validating = harness.pull(Some(&result_id)); + assert_eq!(validating["error"]["code"], -32802, "{validating}"); + assert!( + validating["error"]["data"]["operationId"] + .as_str() + .is_some() + ); + assert_eq!(harness.runtime.started(), 3); + harness + .authority + .validation_pending + .store(false, Ordering::SeqCst); + harness.runtime.complete_next(); + harness.session.flush_due(harness.now_ms); + assert!(harness.messages().is_empty()); + + *harness.authority.next_generation.lock().unwrap() = Some(0); + harness.notify( + "textDocument/didSave", + json!({ "textDocument": { "uri": DOCUMENT } }), + ); + harness.runtime.complete_next(); + let stale = harness.pull(Some(&result_id)); + assert_eq!( + stale["error"]["data"]["failureClass"], "superseded-generation", + "{stale}" + ); + assert!(stale.get("result").is_none()); +} + +#[test] +fn publication_retry_uses_the_admitted_snapshot_without_polling_or_revalidation() { + let mut harness = Harness::new(true); + harness.open(1); + harness.start_refresh(); + harness.runtime.complete_next(); + fill_outbound_queue(&mut harness); + harness.session.poll_diagnostic_refresh(DOCUMENT); + harness.messages(); + let reads = harness + .session + .diagnostics + .provider + .reads + .load(Ordering::SeqCst); + harness + .authority + .validation_pending + .store(true, Ordering::SeqCst); + harness.session.flush_due(harness.now_ms); + let messages = harness.messages(); + assert_eq!(messages.len(), 1, "{messages:?}"); + assert_eq!(messages[0]["method"], "textDocument/publishDiagnostics"); + assert_eq!(messages[0]["params"]["diagnostics"], expected_items(1, 1)); + assert_eq!( + harness + .session + .diagnostics + .provider + .reads + .load(Ordering::SeqCst), + reads + ); + assert_eq!(harness.runtime.started(), 1); + assert_completed_refresh_is_not_polled_again(&mut harness); +} + +#[test] +fn late_validation_cannot_answer_a_reopened_document_or_retire_its_refresh() { + let mut harness = Harness::new(false); + harness.open(1); + harness.start_refresh(); + harness.runtime.complete_next(); + let result_id = assert_full(&harness.pull(None), 1, 1); + harness + .authority + .validation_pending + .store(true, Ordering::SeqCst); + assert_eq!(harness.pull(Some(&result_id))["error"]["code"], -32802); + harness.notify( + "textDocument/didClose", + json!({ "textDocument": { "uri": DOCUMENT } }), + ); + harness.open(1); + assert_eq!(harness.pull(Some(&result_id))["error"]["code"], -32802); + assert_eq!(harness.runtime.started(), 3); + harness + .authority + .validation_pending + .store(false, Ordering::SeqCst); + harness.runtime.complete_next(); + harness.session.flush_due(harness.now_ms); + assert!(harness.messages().is_empty()); + let pending = harness.pull(Some(&result_id)); + assert_eq!(pending["error"]["code"], -32802, "{pending}"); + assert!(pending["error"]["data"]["operationId"].as_str().is_some()); + assert_eq!(harness.runtime.started(), 3); + harness.runtime.complete_next(); + assert_full(&harness.pull(Some(&result_id)), 2, 1); +} From bd238e1276338f5c80da302663a190bd58f882a9 Mon Sep 17 00:00:00 2001 From: Zack Jackson Date: Sun, 25 Oct 2026 06:16:12 -0700 Subject: [PATCH 07/12] fix(lsp): bind diagnostic evidence to committed publications --- .../src/lsp_support/runtime_adapters.rs | 131 ++++--- crates/tracedecay-lsp/src/analyzer/broker.rs | 244 +++++++++++- .../src/analyzer/broker/refresh.rs | 69 +++- .../src/analyzer/broker/semantic_authority.rs | 4 + .../src/analyzer/broker/shared_client.rs | 6 +- .../src/analyzer/broker/tests.rs | 83 +--- crates/tracedecay-lsp/src/analyzer/client.rs | 326 +++++++++++----- .../src/analyzer/client/documents.rs | 274 +++++++++++++ crates/tracedecay-lsp/src/diagnostics.rs | 61 +++ crates/tracedecay-lsp/src/lib.rs | 6 +- .../analyzer_runtime/broker_refresh.rs | 368 ++++++++++++++++++ 11 files changed, 1304 insertions(+), 268 deletions(-) create mode 100644 crates/tracedecay-lsp/src/analyzer/client/documents.rs diff --git a/crates/tracedecay-application/src/lsp_support/runtime_adapters.rs b/crates/tracedecay-application/src/lsp_support/runtime_adapters.rs index 143b6fbb17..d2da4e6abc 100644 --- a/crates/tracedecay-application/src/lsp_support/runtime_adapters.rs +++ b/crates/tracedecay-application/src/lsp_support/runtime_adapters.rs @@ -5,11 +5,13 @@ use std::time::Duration; use tokio::sync::Mutex as AsyncMutex; use tracedecay_domain::ManifestDigest; +#[cfg(test)] +use tracedecay_lsp::analyzer::broker::DiagnosticsSnapshot; use tracedecay_lsp::analyzer::broker::{ - CodeDiagnostic, DiagnosticBroker, DiagnosticSeverity as BrokerDiagnosticSeverity, - DiagnosticsSnapshot, RefreshCommitOutcome, + BrokerDocumentEvidence, CodeDiagnostic, DiagnosticBroker, + DiagnosticSeverity as BrokerDiagnosticSeverity, RefreshCommitOutcome, }; -use tracedecay_lsp::analyzer::client::{DiagnosticDocumentCoverage, LspDocument}; +use tracedecay_lsp::analyzer::client::LspDocument; use tracedecay_lsp::{ AdmittedRoot, CanonicalDiagnosticRefreshRequest, CanonicalDiagnosticSnapshotAuthority, CanonicalWorkspaceDiagnosticRefreshRequest, DiagnosticSource, GatewayDiagnostic, @@ -179,13 +181,14 @@ where request.expected_content_digest = Some(tracedecay_domain::ContentDigest::of_bytes( document.text.as_bytes(), )); + let document_uri = request.document_uri.clone(); let managed = managed.current_authority(request).await?; - let snapshot = broker.lock().await.snapshot(); - let upstream = upstream_diagnostic_authority_digest( - &snapshot, - &document.language, - &document.relative_path, - )?; + let evidence = broker + .lock() + .await + .document_diagnostic_evidence(&document) + .map_err(|_| LspRuntimeFailure::new("diagnostic-broker-authority-unavailable"))?; + let (upstream, _) = project_upstream_evidence(evidence, document_uri.as_str())?; merged_diagnostic_authority_digest(&upstream, &managed) }) } @@ -307,26 +310,33 @@ where document.text.as_bytes(), )); let language = document.language.clone(); - let relative_path = document.relative_path.clone(); - let (prepared, immediate_snapshot) = { + let (prepared, immediate_evidence) = { let mut broker = broker.lock().await; let prepared = broker - .prepare_refresh(&language, vec![document]) + .prepare_refresh(&language, vec![document.clone()]) .await .map_err(|_| LspRuntimeFailure::new("diagnostic-broker-preparation-failed"))?; - let immediate_snapshot = prepared.is_none().then(|| broker.snapshot()); - (prepared, immediate_snapshot) + let immediate_evidence = if prepared.is_none() { + Some( + broker + .document_diagnostic_evidence(&document) + .map_err(|_| LspRuntimeFailure::new("diagnostic-broker-coverage-incomplete"))?, + ) + } else { + None + }; + (prepared, immediate_evidence) }; - let requires_publication = prepared.is_some(); - let snapshot = if let Some(prepared) = prepared { + let evidence = if let Some(prepared) = prepared { let completed = prepared.collect_diagnostics(diagnostics_quiet_window).await; + let mut broker = broker.lock().await; match broker - .lock() - .await .finish_refresh_snapshot(completed) .map_err(|_| LspRuntimeFailure::new("diagnostic-broker-refresh-failed"))? { - RefreshCommitOutcome::Applied(snapshot) => *snapshot, + RefreshCommitOutcome::Applied(_) => broker + .document_diagnostic_evidence(&document) + .map_err(|_| LspRuntimeFailure::new("diagnostic-broker-coverage-incomplete"))?, RefreshCommitOutcome::Superseded => { return Err(LspRuntimeFailure::new( "diagnostic-broker-refresh-superseded", @@ -334,32 +344,11 @@ where } } } else { - immediate_snapshot + immediate_evidence .ok_or_else(|| LspRuntimeFailure::new("diagnostic-broker-snapshot-unavailable"))? }; - let document_coverage = snapshot - .coverage - .get(&language) - .and_then(|coverage| coverage.documents.get(&relative_path)); - if requires_publication - && !matches!( - document_coverage, - Some(DiagnosticDocumentCoverage::Published { content_digest, .. }) - if Some(content_digest) == request.expected_content_digest.as_ref() - ) - { - return Err(LspRuntimeFailure::new( - "diagnostic-broker-coverage-incomplete", - )); - } - let upstream_authority_digest = - upstream_diagnostic_authority_digest(&snapshot, &language, &relative_path)?; - let upstream = snapshot - .diagnostics - .into_iter() - .filter(|diagnostic| diagnostic.language == language && diagnostic.file == relative_path) - .map(|diagnostic| broker_diagnostic(request.document_uri.as_str(), diagnostic)) - .collect(); + let (upstream_authority_digest, upstream) = + project_upstream_evidence(evidence, request.document_uri.as_str())?; let expected_code_generation_id = request.expected_code_generation_id.clone(); let expected_snapshot_digest = request.expected_snapshot_digest.clone(); let managed = managed.snapshot(request).await?; @@ -384,22 +373,52 @@ where }) } -fn upstream_diagnostic_authority_digest( - snapshot: &DiagnosticsSnapshot, - language: &str, - relative_path: &str, -) -> Result { - let coverage = snapshot - .coverage - .get(language) - .and_then(|coverage| coverage.documents.get(relative_path)); - tracedecay_domain::canonical_sha256(&( +fn project_upstream_evidence( + evidence: BrokerDocumentEvidence, + document_uri: &str, +) -> Result<(ManifestDigest, Vec), LspRuntimeFailure> { + let (settings, settings_unavailable, coverage, nonparticipation, diagnostics) = match evidence { + BrokerDocumentEvidence::Published { + settings, + settings_unavailable, + coverage, + diagnostics, + } => ( + settings, + settings_unavailable, + Some(coverage), + None, + diagnostics, + ), + BrokerDocumentEvidence::NotParticipating { + settings, + settings_unavailable, + reason, + } => ( + settings, + settings_unavailable, + None, + Some(reason), + Vec::new(), + ), + }; + let (upstream, identity) = tracedecay_lsp::project_upstream_diagnostics( + diagnostics + .into_iter() + .map(|diagnostic| broker_diagnostic(document_uri, diagnostic)) + .collect(), + ) + .map_err(|_| LspRuntimeFailure::new("diagnostic-broker-authority-identity-unavailable"))?; + let digest = tracedecay_domain::canonical_sha256(&( "tracedecay.lsp.upstream-diagnostic-authority.v1", - &snapshot.settings, - &snapshot.settings_unavailable, + settings, + settings_unavailable, coverage, + nonparticipation, + identity, )) - .map_err(|_| LspRuntimeFailure::new("diagnostic-broker-authority-identity-unavailable")) + .map_err(|_| LspRuntimeFailure::new("diagnostic-broker-authority-identity-unavailable"))?; + Ok((digest, upstream)) } fn merged_diagnostic_authority_digest( diff --git a/crates/tracedecay-lsp/src/analyzer/broker.rs b/crates/tracedecay-lsp/src/analyzer/broker.rs index f998238077..21b5c94b03 100644 --- a/crates/tracedecay-lsp/src/analyzer/broker.rs +++ b/crates/tracedecay-lsp/src/analyzer/broker.rs @@ -10,7 +10,7 @@ use super::activity::{adapter_workspace_root_from_canonical_root, canonicalize_p use super::adapters::{LspAdapterDefinition, LspInstallOption}; use super::client::{ DiagnosticCoverage, DiagnosticDocumentCoverage, DiagnosticRootCoverage, LspDocument, - LspRefreshTimeouts, file_uri, + LspRefreshTimeouts, document_identity, file_uri, }; use super::error::{AnalyzerResult as Result, AnalyzerRuntimeError as TraceDecayError}; pub use super::launch::command_available; @@ -201,6 +201,44 @@ pub enum RefreshCommitOutcome { Superseded, } +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +pub enum DiagnosticEvidenceUnavailable { + #[error("diagnostic document identity is invalid")] + DocumentIdentityInvalid, + #[error("diagnostic source is unavailable")] + SourceUnavailable, + #[error("diagnostic publication is not committed")] + PublicationUncommitted, + #[error("diagnostic document content changed")] + ContentChanged, + #[error("diagnostic overlay admission is stale")] + OverlayStale, + #[error("diagnostic analyzer incarnation is retired")] + IncarnationRetired, + #[error("diagnostic document metadata is unavailable")] + MetadataUnavailable, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +pub enum DiagnosticNonparticipation { + Disabled, + Inactive, +} + +pub enum BrokerDocumentEvidence { + Published { + settings: CodeDiagnosticsSettings, + settings_unavailable: Option, + coverage: DiagnosticDocumentCoverage, + diagnostics: Vec, + }, + NotParticipating { + settings: CodeDiagnosticsSettings, + settings_unavailable: Option, + reason: DiagnosticNonparticipation, + }, +} + #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] struct LspSessionKey { language: String, @@ -378,7 +416,7 @@ impl DiagnosticBroker { pub fn update_adapters(&mut self, adapters: Vec) { self.adapters = adapters; - self.clients.clear(); + self.remove_all_clients(); self.clear_launches(); } @@ -482,9 +520,43 @@ impl DiagnosticBroker { language: &str, documents: Vec, ) -> Result> { - // A new attempt supersedes the previous scope's publication evidence, - // including when preparation fails before any analyzer can run. + let epoch = self.next_refresh_epoch(language); self.coverage.remove(language); + let paths = documents + .iter() + .map(|document| { + let path = self.project_root.join(&document.relative_path); + let uri = document + .snapshot + .as_ref() + .map_or_else(|| file_uri(&path), |snapshot| snapshot.uri.clone()); + document_identity(&path, &uri) + }) + .collect::>>(); + let mut invalidation_failure = None; + for (key, client) in &self.clients { + if key.language == language + && let Err(error) = client.documents.invalidate(paths.as_deref(), epoch) + { + invalidation_failure = Some(error); + } + } + if let Some(error) = invalidation_failure { + return Err(TraceDecayError::new(error.to_string())); + } + if paths.is_none() { + return Err(TraceDecayError::new( + DiagnosticEvidenceUnavailable::DocumentIdentityInvalid.to_string(), + )); + } + if documents + .iter() + .any(|document| document.language != language) + { + return Err(TraceDecayError::new( + "diagnostic document language does not match the refresh scope", + )); + } if !self.settings.language_enabled(language) { self.engine_overrides .insert(language.to_string(), EngineState::Disabled); @@ -560,7 +632,6 @@ impl DiagnosticBroker { } self.engine_overrides .insert(language.to_string(), EngineState::Refreshing); - let epoch = self.next_refresh_epoch(language); let reservation = self .refresh_capacity .reserve(resolved_roots.len()) @@ -589,14 +660,34 @@ impl DiagnosticBroker { SharedAnalyzerClient::new(AdmittedRoot::new(file_uri(&workspace_root))) }) .clone(); - RefreshBatch { + let claims = documents + .iter() + .map(|document| { + let path = canonical_project_root.join(&document.relative_path); + let uri = document + .snapshot + .as_ref() + .map_or_else(|| file_uri(&path), |snapshot| snapshot.uri.clone()); + let identity = document_identity(&path, &uri).ok_or_else(|| { + TraceDecayError::new( + DiagnosticEvidenceUnavailable::DocumentIdentityInvalid.to_string(), + ) + })?; + client + .documents + .claim(identity, epoch) + .map_err(|error| TraceDecayError::new(error.to_string())) + }) + .collect::>>()?; + Ok(RefreshBatch { workspace_root, launch, documents, client, - } + claims, + }) }) - .collect(); + .collect::>>()?; Ok(Some(PreparedRefresh::new( language.to_string(), canonical_project_root, @@ -717,19 +808,49 @@ impl DiagnosticBroker { self.clear_language(&language); return Ok(RefreshCommitOutcome::Superseded); } - if !self.command_matches_current_settings(&language, &completed.command) { + if !self.project_languages.contains(&language) + || !self.command_matches_current_settings(&language, &completed.command) + || completed.sources.iter().any(|(key, source)| { + !self + .clients + .get(key) + .is_some_and(|current| Arc::ptr_eq(current, source)) + }) + { return Ok(RefreshCommitOutcome::Superseded); } match completed.result { Ok(mut refresh) => { + let mut committed = BTreeSet::new(); + let mut findings = Vec::new(); + for publication in &completed.publications { + let file = publication.relative_path(); + match publication.commit(|accepted| { + findings.extend(accepted.diagnostics.iter().cloned()); + }) { + Ok(()) => { + committed.insert(file.to_owned()); + } + Err(error) => { + tracing::debug!(%error, file, "diagnostic publication lost commit eligibility"); + refresh.coverage.documents.insert( + file.to_owned(), + DiagnosticDocumentCoverage::StalePublication, + ); + } + } + } + for (file, coverage) in &mut refresh.coverage.documents { + if matches!(coverage, DiagnosticDocumentCoverage::Published { .. }) + && !committed.contains(file) + { + *coverage = DiagnosticDocumentCoverage::StalePublication; + } + } self.diagnostics.retain(|diagnostic| { - diagnostic.language != language - || !matches!( - refresh.coverage.documents.get(&diagnostic.file), - Some(DiagnosticDocumentCoverage::Published { .. }) - ) + diagnostic.language != language || !committed.contains(&diagnostic.file) }); - self.diagnostics.append(&mut refresh.diagnostics); + self.diagnostics.append(&mut findings); // Replace the scope instead of accumulating a historical document // map. Unrequested retained findings must not inherit fresh coverage. self.coverage @@ -813,7 +934,7 @@ impl DiagnosticBroker { // These settings were just persisted, so whatever the broker could not // read at mount time is no longer what it is serving. self.settings_unavailable = None; - self.clients.clear(); + self.remove_all_clients(); self.coverage.clear(); self.clear_launches(); self.launch_refusals.clear(); @@ -832,6 +953,77 @@ impl DiagnosticBroker { } } + pub fn document_diagnostic_evidence( + &self, + document: &LspDocument, + ) -> std::result::Result { + let reason = if !self.settings.language_enabled(&document.language) { + Some(DiagnosticNonparticipation::Disabled) + } else if !self.project_languages.contains(&document.language) { + Some(DiagnosticNonparticipation::Inactive) + } else { + None + }; + if let Some(reason) = reason { + return Ok(BrokerDocumentEvidence::NotParticipating { + settings: self.settings.clone(), + settings_unavailable: self.settings_unavailable.clone(), + reason, + }); + } + let adapter = self + .adapters + .iter() + .find(|adapter| adapter.language == document.language) + .ok_or(DiagnosticEvidenceUnavailable::SourceUnavailable)?; + let root = canonicalize_project_root(&self.project_root) + .map_err(|_| DiagnosticEvidenceUnavailable::DocumentIdentityInvalid)?; + let path = root.join(&document.relative_path); + let uri = document + .snapshot + .as_ref() + .map_or_else(|| file_uri(&path), |snapshot| snapshot.uri.clone()); + let identity = document_identity(&path, &uri) + .ok_or(DiagnosticEvidenceUnavailable::DocumentIdentityInvalid)?; + let workspace_root = match adapter_workspace_root_from_canonical_root( + &root, + adapter, + &document.relative_path, + ) { + Some(workspace_root) => workspace_root, + None => root, + }; + let key = LspSessionKey { + language: document.language.clone(), + command: self + .settings + .command_for(&document.language, &adapter.command), + workspace_root, + }; + let client = self + .clients + .get(&key) + .ok_or(DiagnosticEvidenceUnavailable::SourceUnavailable)?; + client + .documents + .read_committed(&identity, &uri, document, |coverage| { + BrokerDocumentEvidence::Published { + settings: self.settings.clone(), + settings_unavailable: self.settings_unavailable.clone(), + coverage: coverage.clone(), + diagnostics: self + .diagnostics + .iter() + .filter(|finding| { + finding.language == document.language + && finding.file == document.relative_path + }) + .cloned() + .collect(), + } + }) + } + pub fn cache_diagnostic(&mut self, diagnostic: CodeDiagnostic) { self.diagnostics.push(diagnostic); } @@ -918,9 +1110,25 @@ impl DiagnosticBroker { self.coverage.remove(language); } + fn remove_all_clients(&mut self) { + for epoch in self.refresh_epochs.values_mut() { + *epoch = epoch.saturating_add(1); + } + for client in self.clients.values() { + client.documents.retire(None); + } + self.clients.clear(); + } + fn remove_language_clients(&mut self, language: &str) { - self.clients - .retain(|key, _| key.language.as_str() != language); + self.clients.retain(|key, client| { + if key.language == language { + client.documents.retire(None); + false + } else { + true + } + }); } fn next_refresh_epoch(&mut self, language: &str) -> u64 { diff --git a/crates/tracedecay-lsp/src/analyzer/broker/refresh.rs b/crates/tracedecay-lsp/src/analyzer/broker/refresh.rs index 0c82e0087a..c8445c1342 100644 --- a/crates/tracedecay-lsp/src/analyzer/broker/refresh.rs +++ b/crates/tracedecay-lsp/src/analyzer/broker/refresh.rs @@ -10,13 +10,14 @@ use tokio::sync::{OwnedSemaphorePermit, Semaphore}; use tracedecay_runtime_core::native_runtime; use super::super::client::{ - DiagnosticDocumentCoverage, DiagnosticRefreshResult, DiagnosticRootCoverage, LspDocument, - LspRefreshError, LspRefreshTimeouts, StdioLspClientStartup, + CollectedDocumentPublication, DiagnosticDocumentCoverage, DiagnosticRefreshResult, + DiagnosticRootCoverage, DocumentRefreshClaim, LspDocument, LspRefreshError, LspRefreshTimeouts, + StdioLspClientStartup, }; use super::super::error::AnalyzerRuntimeError as TraceDecayError; use super::super::launch::AnalyzerLaunch; -use super::EngineState; use super::shared_client::{AnalyzerClientUse, SharedAnalyzerClient}; +use super::{EngineState, LspSessionKey}; use crate::AnalyzerEvent; pub(crate) struct RefreshBatch { @@ -27,6 +28,7 @@ pub(crate) struct RefreshBatch { pub(crate) launch: AnalyzerLaunch, pub(crate) documents: Vec, pub(crate) client: Arc, + pub(crate) claims: Vec, } /// Each analyzer may wait behind at most this many independent workspace-root @@ -112,6 +114,8 @@ pub struct CompletedRefresh { pub(crate) epoch: u64, pub(crate) result: std::result::Result, pub(crate) root_failures: BTreeMap, + pub(super) sources: Vec<(LspSessionKey, Arc)>, + pub(crate) publications: Vec, } impl CompletedRefresh { @@ -171,10 +175,24 @@ impl PreparedRefresh { let language = self.language.clone(); let command = self.spawn.command.clone(); let epoch = self.epoch; + let sources = self + .batches + .iter() + .map(|batch| { + ( + LspSessionKey { + language: language.clone(), + command: command.clone(), + workspace_root: batch.workspace_root.clone(), + }, + Arc::clone(&batch.client), + ) + }) + .collect(); let collected = self.collect(timeouts).await; - let (result, root_failures) = match collected { - Ok((result, failures)) => (Ok(result), failures), - Err(error) => (Err(error), BTreeMap::new()), + let (result, root_failures, publications) = match collected { + Ok((result, failures, publications)) => (Ok(result), failures, publications), + Err(error) => (Err(error), BTreeMap::new(), Vec::new()), }; CompletedRefresh { language, @@ -182,6 +200,8 @@ impl PreparedRefresh { epoch, result, root_failures, + sources, + publications, } } @@ -189,7 +209,11 @@ impl PreparedRefresh { self, timeouts: LspRefreshTimeouts, ) -> std::result::Result< - (DiagnosticRefreshResult, BTreeMap), + ( + DiagnosticRefreshResult, + BTreeMap, + Vec, + ), RefreshFailure, > { // Batches run concurrently inside this future rather than as spawned @@ -237,9 +261,11 @@ impl PreparedRefresh { completed.sort_by_key(|(ordinal, _, _, _)| *ordinal); let mut refresh = DiagnosticRefreshResult::default(); let mut failures = BTreeMap::new(); + let mut publications = Vec::new(); for (_, workspace_root, documents, result) in completed { match result { - Ok(Ok((_, batch))) => { + Ok(Ok((_, batch, accepted))) => { + publications.extend(accepted); refresh.diagnostics.extend(batch.diagnostics); refresh.coverage.documents.extend(batch.coverage.documents); refresh @@ -278,7 +304,7 @@ impl PreparedRefresh { } } } - Ok((refresh, failures)) + Ok((refresh, failures, publications)) } } @@ -290,7 +316,14 @@ async fn collect_refresh_batch( spawn: AnalyzerSpawn, timeouts: LspRefreshTimeouts, _run_permit: OwnedSemaphorePermit, -) -> std::result::Result<(usize, DiagnosticRefreshResult), RefreshFailure> { +) -> std::result::Result< + ( + usize, + DiagnosticRefreshResult, + Vec, + ), + RefreshFailure, +> { let AnalyzerSpawn { command, args } = spawn; let launch = batch.launch; let shared = batch.client; @@ -311,8 +344,14 @@ async fn collect_refresh_batch( ))); }; let mut use_of_analyzer = AnalyzerClientUse::new(&shared, client_slot, attempt, None); - let startup = - StdioLspClientStartup::start(launch, args, batch.workspace_root.clone(), timeouts); + let startup = StdioLspClientStartup::start( + launch, + args, + batch.workspace_root.clone(), + timeouts, + Arc::clone(&shared.documents), + attempt, + ); match startup { Ok(startup) => use_of_analyzer.start(startup), Err(error) => { @@ -359,7 +398,7 @@ async fn collect_refresh_batch( let result = match use_of_analyzer.client.as_mut() { Some(client) => { client - .collect_document_diagnostics(&project_root, batch.documents, timeouts) + .collect_claimed_diagnostics(&project_root, batch.documents, timeouts, batch.claims) .await } None => Err(LspRefreshError::Transport { @@ -367,9 +406,9 @@ async fn collect_refresh_batch( }), }; match result { - Ok(diagnostics) => { + Ok((diagnostics, publications)) => { use_of_analyzer.restore(AnalyzerEvent::Ready); - Ok((ordinal, diagnostics)) + Ok((ordinal, diagnostics, publications)) } Err(error) => { let failure = RefreshFailure::client(&error); diff --git a/crates/tracedecay-lsp/src/analyzer/broker/semantic_authority.rs b/crates/tracedecay-lsp/src/analyzer/broker/semantic_authority.rs index cdfedd5780..91886760fb 100644 --- a/crates/tracedecay-lsp/src/analyzer/broker/semantic_authority.rs +++ b/crates/tracedecay-lsp/src/analyzer/broker/semantic_authority.rs @@ -150,6 +150,8 @@ impl StdioLspSemanticAuthority { self.inner.args.clone(), self.inner.project_root.clone(), self.inner.timeouts, + Arc::clone(&self.inner.shared.documents), + attempt, ); match startup { Ok(startup) => usage.start(startup), @@ -313,6 +315,8 @@ impl LspSemanticRequestAuthority for StdioLspSemanticAuthority { inner.args.clone(), inner.project_root.clone(), timeouts, + Arc::clone(&inner.shared.documents), + attempt, ) { Ok(startup) => startup, Err(error) => { diff --git a/crates/tracedecay-lsp/src/analyzer/broker/shared_client.rs b/crates/tracedecay-lsp/src/analyzer/broker/shared_client.rs index a1f73cbed6..07498a9339 100644 --- a/crates/tracedecay-lsp/src/analyzer/broker/shared_client.rs +++ b/crates/tracedecay-lsp/src/analyzer/broker/shared_client.rs @@ -14,7 +14,7 @@ use std::sync::{Arc, Mutex as SyncMutex}; use tokio::sync::{Mutex, MutexGuard}; use super::super::client::{ - StdioLspClient, StdioLspClientStartup, StdioLspClientStartupSettlement, + AnalyzerDocuments, StdioLspClient, StdioLspClientStartup, StdioLspClientStartupSettlement, }; use super::super::error::{AnalyzerResult, AnalyzerRuntimeError}; use crate::{AdmittedRoot, AnalyzerEvent, AnalyzerState, AnalyzerSupervisor}; @@ -120,6 +120,7 @@ impl SharedAnalyzerClientSlot { pub(crate) struct SharedAnalyzerClient { client: Mutex, supervisor: SyncMutex, + pub(crate) documents: Arc, } impl SharedAnalyzerClient { @@ -127,6 +128,7 @@ impl SharedAnalyzerClient { Arc::new(Self { client: Mutex::new(SharedAnalyzerClientSlot::new()), supervisor: SyncMutex::new(AnalyzerSupervisor::new(root)), + documents: AnalyzerDocuments::new(), }) } @@ -310,6 +312,7 @@ impl<'a> AnalyzerClientUse<'a> { } pub(crate) fn retire(mut self, event: AnalyzerEvent) { + self.shared.documents.retire(Some(self.attempt)); self.concluded = true; self.shared.record(self.attempt, event); } @@ -319,6 +322,7 @@ impl Drop for AnalyzerClientUse<'_> { fn drop(&mut self) { self.client_slot.cancel_startup(); if !self.concluded { + self.shared.documents.retire(Some(self.attempt)); self.shared.record(self.attempt, AnalyzerEvent::Retired); } if let Some(client) = self.client.take() { diff --git a/crates/tracedecay-lsp/src/analyzer/broker/tests.rs b/crates/tracedecay-lsp/src/analyzer/broker/tests.rs index e52443322d..d91debbb36 100644 --- a/crates/tracedecay-lsp/src/analyzer/broker/tests.rs +++ b/crates/tracedecay-lsp/src/analyzer/broker/tests.rs @@ -814,6 +814,8 @@ fn superseded_refresh_cannot_supply_a_snapshot_to_its_caller() { epoch, result: Ok(crate::analyzer::client::DiagnosticRefreshResult::default()), root_failures: BTreeMap::new(), + sources: Vec::new(), + publications: Vec::new(), }; assert!(matches!( @@ -825,84 +827,3 @@ fn superseded_refresh_cannot_supply_a_snapshot_to_its_caller() { Ok(RefreshCommitOutcome::Superseded) )); } - -#[test] -fn partial_root_refresh_replaces_only_published_document_diagnostics() { - let mut broker = DiagnosticBroker::new_for_test( - "/project", - vec![adapter("fake", "fake-ls", "fake", "workspace.marker")], - ); - broker.refresh_epochs.insert("fake".to_owned(), 1); - broker.diagnostics = vec![ - code_diagnostic("src/a.fake", "old a"), - code_diagnostic("member/src/b.fake", "old b"), - ]; - let root = PathBuf::from("/project/member"); - let failure = super::refresh::RefreshFailure::crashed_message("member exited".to_owned()); - let completed = CompletedRefresh { - language: "fake".to_owned(), - command: "fake-ls".to_owned(), - epoch: 1, - result: Ok(crate::analyzer::client::DiagnosticRefreshResult { - diagnostics: vec![code_diagnostic("src/a.fake", "new a")], - coverage: crate::analyzer::client::DiagnosticCoverage { - documents: BTreeMap::from([ - ( - "src/a.fake".to_owned(), - DiagnosticDocumentCoverage::Published { - version_verified: true, - content_digest: tracedecay_domain::ContentDigest::of_bytes(b"new a"), - }, - ), - ( - "member/src/b.fake".to_owned(), - DiagnosticDocumentCoverage::RootFailed, - ), - ]), - roots: BTreeMap::from([ - ( - PathBuf::from("/project"), - crate::analyzer::client::DiagnosticRootCoverage::Completed, - ), - ( - root.clone(), - crate::analyzer::client::DiagnosticRootCoverage::Failed, - ), - ]), - }, - }), - root_failures: BTreeMap::from([(root, failure)]), - }; - - assert!( - broker.finish_refresh_snapshot(completed).is_err(), - "the caller must see that one root was incomplete" - ); - let diagnostics = broker.snapshot().diagnostics; - assert_eq!(diagnostics.len(), 2); - assert!( - diagnostics - .iter() - .any(|diagnostic| diagnostic.file == "src/a.fake" && diagnostic.message == "new a") - ); - assert!(diagnostics.iter().any(|diagnostic| { - diagnostic.file == "member/src/b.fake" && diagnostic.message == "old b" - })); -} - -fn code_diagnostic(file: &str, message: &str) -> CodeDiagnostic { - CodeDiagnostic { - language: "fake".to_owned(), - source: "fake-ls".to_owned(), - file: file.to_owned(), - line_start: 0, - line_end: 0, - character_start: None, - character_end: None, - severity: DiagnosticSeverity::Error, - code: None, - message: message.to_owned(), - enclosing_node: None, - updated_at: 0, - } -} diff --git a/crates/tracedecay-lsp/src/analyzer/client.rs b/crates/tracedecay-lsp/src/analyzer/client.rs index 168a8819fb..7a1e47cdff 100644 --- a/crates/tracedecay-lsp/src/analyzer/client.rs +++ b/crates/tracedecay-lsp/src/analyzer/client.rs @@ -27,9 +27,9 @@ use tracedecay_daemon_protocol::{ConnectionLocalRequestSequence, FramePoll}; use tracedecay_domain::ContentDigest; use tracedecay_runtime_core::native_runtime; use tracedecay_runtime_core::native_runtime::{NativeTask, NativeTaskOutcome}; -use tracedecay_runtime_core::path_safety::{canonicalize_existing_prefix, plain_host_path}; +use tracedecay_runtime_core::path_safety::plain_host_path; -use super::broker::{CodeDiagnostic, DiagnosticSeverity}; +use super::broker::{CodeDiagnostic, DiagnosticEvidenceUnavailable, DiagnosticSeverity}; use super::error::{ AnalyzerCancellation as CancellationToken, AnalyzerResult as Result, AnalyzerRuntimeError as TraceDecayError, @@ -39,6 +39,12 @@ use super::process::AnalyzerProcess; use crate::{AnalyzerEvent, AsyncContentLengthError, UpstreamCapabilities}; +mod documents; +use documents::{AcceptedDiagnosticPublication, PublicationEligibility, SynchronizedDocument}; +pub(crate) use documents::{ + AnalyzerDocuments, CollectedDocumentPublication, DocumentRefreshClaim, document_identity, +}; + const MIN_MESSAGE_IO_TIMEOUT: Duration = Duration::from_secs(2); const MIN_INITIALIZE_RESPONSE_TIMEOUT: Duration = Duration::from_secs(3); @@ -383,19 +389,12 @@ pub async fn collect_document_diagnostics_with_timeouts( .map_err(TraceDecayError::from) } -struct SynchronizedDocument { - uri: String, - wire_version: i32, - text: String, - snapshot: Option, - diagnostics: Option<(Vec, DiagnosticDocumentCoverage)>, -} - pub struct StdioLspClient { command: String, workspace_root: PathBuf, upstream_capabilities: UpstreamCapabilities, - documents: BTreeMap, + documents: Arc, + attempt: u32, next_request_id: ConnectionLocalRequestSequence, /// Killed and reaped when the client drops. process: AnalyzerProcess, @@ -426,7 +425,12 @@ impl StdioLspClientStartup { args: Vec, project_root: PathBuf, timeouts: LspRefreshTimeouts, + documents: Arc, + attempt: u32, ) -> Result { + documents + .activate(attempt) + .map_err(|error| TraceDecayError::new(error.to_string()))?; let cancellation = CancellationToken::new(); let startup_cancellation = cancellation.clone(); let result = Arc::new(Mutex::new(None)); @@ -438,6 +442,8 @@ impl StdioLspClientStartup { &project_root, timeouts, &startup_cancellation, + documents, + attempt, ) .await; { @@ -567,12 +573,18 @@ impl StdioLspClient { timeouts: LspRefreshTimeouts, ) -> Result { let cancellation = CancellationToken::new(); + let documents = AnalyzerDocuments::new(); + documents + .activate(0) + .map_err(|error| TraceDecayError::new(error.to_string()))?; match Self::start_with_launch_until_cancelled( launch, args, project_root, timeouts, &cancellation, + documents, + 0, ) .await { @@ -591,6 +603,8 @@ impl StdioLspClient { project_root: &Path, timeouts: LspRefreshTimeouts, cancellation: &CancellationToken, + documents: Arc, + attempt: u32, ) -> StdioLspClientStartupSettlement { let timeouts = timeouts.with_deadline(timeouts.operation_deadline()); let deadline = timeouts.operation_deadline(); @@ -721,7 +735,8 @@ impl StdioLspClient { upstream_capabilities: UpstreamCapabilities::from_initialize_response( &initialize_response, ), - documents: BTreeMap::new(), + documents, + attempt, next_request_id: ConnectionLocalRequestSequence::starting_at(2), process, }) @@ -795,7 +810,12 @@ impl StdioLspClient { class: "admitted semantic document snapshot is invalid".to_owned(), }); } - if self.snapshot_is_stale(&identity, snapshot) { + if self + .snapshot_is_stale(&identity, snapshot) + .map_err(|error| LspSemanticRequestError::InvalidResponse { + class: error.to_string(), + })? + { return Err(LspSemanticRequestError::InvalidResponse { class: "admitted semantic document snapshot is stale".to_owned(), }); @@ -820,16 +840,17 @@ impl StdioLspClient { &self, identity: &Path, snapshot: &crate::LspSemanticDocumentSnapshot, - ) -> bool { - !snapshot.lifetime.is_open() - || self.documents.get(identity).is_some_and(|current| { + ) -> std::result::Result { + let state = self.documents.lock()?; + Ok(!snapshot.lifetime.is_open() + || state.documents.get(identity).is_some_and(|current| { current.snapshot.as_ref().is_some_and(|admitted| { admitted.lifetime == snapshot.lifetime && (admitted.version > snapshot.version || (admitted.version == snapshot.version && current.text != snapshot.text.as_ref())) }) - }) + })) } async fn synchronize_document( @@ -841,26 +862,37 @@ impl StdioLspClient { snapshot: Option<&crate::LspSemanticDocumentSnapshot>, timeouts: LspRefreshTimeouts, ) -> std::result::Result { - if let Some(current) = self.documents.get_mut(identity) - && current.uri == uri - && current.text == text - { - current.snapshot = snapshot.cloned(); - return Ok(current.wire_version); - } - let current_version = self - .documents - .get(identity) - .map_or(0, |current| current.wire_version); + let (current_version, previous_uri) = { + let mut state = self.documents.lock().map_err(|error| { + LspMessageWriteError::Failed(TraceDecayError::new(error.to_string())) + })?; + if state.attempt != Some(self.attempt) { + return Err(LspMessageWriteError::Failed(TraceDecayError::Unavailable)); + } + if let Some(current) = state.documents.get_mut(identity) { + if current.uri == uri && current.text == text { + if current.snapshot.as_ref() != snapshot + && !matches!(current.eligibility, PublicationEligibility::Refreshing(_)) + { + current.eligibility = PublicationEligibility::Unverified; + } + current.snapshot = snapshot.cloned(); + return Ok(current.wire_version); + } + current.accepted = None; + if !matches!(current.eligibility, PublicationEligibility::Refreshing(_)) { + current.eligibility = PublicationEligibility::Unverified; + } + (current.wire_version, Some(current.uri.clone())) + } else { + (0, None) + } + }; let version = current_version.checked_add(1).ok_or_else(|| { LspMessageWriteError::Failed(TraceDecayError::Config { message: "analyzer document version is exhausted".to_owned(), }) })?; - let previous_uri = self - .documents - .get(identity) - .map(|current| current.uri.clone()); let notification = if previous_uri.as_deref() == Some(uri) { json!({ "jsonrpc": "2.0", @@ -906,16 +938,32 @@ impl StdioLspClient { .request_deadline() .min(Instant::now() + timeouts.refresh); write_message_until(&self.process, notification, deadline, timeouts.message_io).await?; - self.documents.insert( - identity.to_path_buf(), - SynchronizedDocument { - uri: uri.to_owned(), - wire_version: version, - text: text.to_owned(), - snapshot: snapshot.cloned(), - diagnostics: None, - }, - ); + let mut state = self.documents.lock().map_err(|error| { + LspMessageWriteError::Failed(TraceDecayError::new(error.to_string())) + })?; + if state.attempt != Some(self.attempt) { + return Err(LspMessageWriteError::Failed(TraceDecayError::Unavailable)); + } + if let Some(current) = state.documents.get_mut(identity) { + uri.clone_into(&mut current.uri); + current.wire_version = version; + text.clone_into(&mut current.text); + current.snapshot = snapshot.cloned(); + current.accepted = None; + } else { + state.documents.insert( + identity.to_path_buf(), + SynchronizedDocument { + uri: uri.to_owned(), + wire_version: version, + text: text.to_owned(), + snapshot: snapshot.cloned(), + accepted: None, + epoch: 0, + eligibility: PublicationEligibility::Unverified, + }, + ); + } Ok(version) } @@ -1302,6 +1350,25 @@ impl StdioLspClient { documents: Vec, timeouts: LspRefreshTimeouts, ) -> std::result::Result { + self.collect_claimed_diagnostics(project_root, documents, timeouts, Vec::new()) + .await + .map(|(result, _)| result) + } + + pub(crate) async fn collect_claimed_diagnostics( + &mut self, + project_root: &Path, + documents: Vec, + timeouts: LspRefreshTimeouts, + claims: Vec, + ) -> std::result::Result< + (DiagnosticRefreshResult, Vec), + LspRefreshError, + > { + let mut claims = claims + .into_iter() + .map(|claim| (claim.path().to_path_buf(), claim)) + .collect::>(); let deadline = timeouts.request_deadline(); let mut uri_to_document = BTreeMap::new(); let mut expected_versions = BTreeMap::new(); @@ -1316,6 +1383,15 @@ impl StdioLspClient { document_identity(&path, &uri).ok_or_else(|| LspRefreshError::InvalidResponse { message: "admitted diagnostic document URI is invalid".to_owned(), })?; + if let Some(claim) = claims.get(&identity) + && !claim.is_current().map_err(document_refresh_error)? + { + result.coverage.documents.insert( + document.relative_path.clone(), + DiagnosticDocumentCoverage::StalePublication, + ); + continue; + } if let Some(snapshot) = document.snapshot.as_ref() { if snapshot.text.as_ref() != document.text || ContentDigest::of_bytes(document.text.as_bytes()) != snapshot.content_digest @@ -1324,7 +1400,10 @@ impl StdioLspClient { message: "admitted diagnostic document snapshot is invalid".to_owned(), }); } - if self.snapshot_is_stale(&identity, snapshot) { + if self + .snapshot_is_stale(&identity, snapshot) + .map_err(document_refresh_error)? + { // A queued editor admission may expire before this client // is acquired. It is stale caller work, not an analyzer // protocol failure; do not send its text or retire the @@ -1335,17 +1414,18 @@ impl StdioLspClient { ); continue; } - } else if let Some(current) = self.documents.get(&identity) - && current - .snapshot - .as_ref() - .is_some_and(|snapshot| snapshot.lifetime.is_open()) - { - // Only the overlay store can keep this admission alive. Closing - // or dropping its session returns a later sweep to disk text. - document.text.clone_from(¤t.text); - document.snapshot = current.snapshot.clone(); - uri.clone_from(¤t.uri); + } else { + let state = self.documents.lock().map_err(document_refresh_error)?; + if let Some(current) = state.documents.get(&identity) + && current + .snapshot + .as_ref() + .is_some_and(|snapshot| snapshot.lifetime.is_open()) + { + document.text.clone_from(¤t.text); + document.snapshot = current.snapshot.clone(); + uri.clone_from(¤t.uri); + } } let version = self .synchronize_document( @@ -1358,32 +1438,45 @@ impl StdioLspClient { ) .await .map_err(refresh_write_error)?; + if let Some(claim) = claims.get(&identity) { + match claim.bind() { + Ok(()) => {} + Err(DiagnosticEvidenceUnavailable::PublicationUncommitted) => { + result.coverage.documents.insert( + document.relative_path.clone(), + DiagnosticDocumentCoverage::StalePublication, + ); + continue; + } + Err(error) => return Err(document_refresh_error(error)), + } + } expected_versions.insert(uri.clone(), version); uri_to_document.insert(uri, (identity, document)); } if uri_to_document.is_empty() { - return Ok(result); + return Ok((result, Vec::new())); } - let mut diagnostics_by_uri: BTreeMap> = BTreeMap::new(); + let mut publications_by_uri: BTreeMap> = + BTreeMap::new(); let mut coverage_by_uri = BTreeMap::new(); let mut current_publications = BTreeSet::new(); - // An unchanged document sends no notification, so the server need not - // publish again. Its last accepted publication remains authoritative - // for this wire version. Synchronizing changed text replaces the - // document record and discards that publication. - for (uri, (identity, _)) in &uri_to_document { - if let Some((diagnostics, coverage)) = self - .documents - .get(identity) - .and_then(|document| document.diagnostics.as_ref()) - { - diagnostics_by_uri.insert(uri.clone(), diagnostics.clone()); - coverage_by_uri.insert(uri.clone(), coverage.clone()); + { + let state = self.documents.lock().map_err(document_refresh_error)?; + for (uri, (identity, _)) in &uri_to_document { + if let Some(accepted) = state + .documents + .get(identity) + .and_then(|document| document.accepted.as_ref()) + { + publications_by_uri.insert(uri.clone(), Arc::clone(accepted)); + coverage_by_uri.insert(uri.clone(), accepted.coverage.clone()); + } } } let refresh_deadline = deadline.min(Instant::now() + timeouts.refresh); - let mut quiet_deadline = (diagnostics_by_uri.len() == uri_to_document.len()) + let mut quiet_deadline = (publications_by_uri.len() == uri_to_document.len()) .then(|| refresh_deadline.min(Instant::now() + timeouts.diagnostics_quiet)); loop { let now = Instant::now(); @@ -1441,8 +1534,12 @@ impl StdioLspClient { published.uri.as_str().to_owned(), DiagnosticDocumentCoverage::StalePublication, ); - if let Some(current) = self.documents.get_mut(identity) { - current.diagnostics = None; + let mut state = self.documents.lock().map_err(document_refresh_error)?; + if let Some(current) = state.documents.get_mut(identity) { + current.accepted = None; + if !matches!(current.eligibility, PublicationEligibility::Refreshing(_)) { + current.eligibility = PublicationEligibility::Unverified; + } } } continue; @@ -1456,12 +1553,27 @@ impl StdioLspClient { .into_iter() .map(|diagnostic| code_diagnostic(diagnostic, document, &self.command)) .collect::>(); - if let Some(current) = self.documents.get_mut(identity) { - current.diagnostics = Some((diagnostics.clone(), coverage.clone())); + let accepted = Arc::new(AcceptedDiagnosticPublication { + diagnostics, + coverage: coverage.clone(), + }); + { + let mut state = self.documents.lock().map_err(document_refresh_error)?; + if state.attempt != Some(self.attempt) { + return Err(document_refresh_error( + DiagnosticEvidenceUnavailable::IncarnationRetired, + )); + } + if let Some(current) = state.documents.get_mut(identity) { + current.accepted = Some(Arc::clone(&accepted)); + if !matches!(current.eligibility, PublicationEligibility::Refreshing(_)) { + current.eligibility = PublicationEligibility::Unverified; + } + } } current_publications.insert(published.uri.as_str().to_owned()); coverage_by_uri.insert(published.uri.as_str().to_owned(), coverage); - diagnostics_by_uri.insert(published.uri.as_str().to_owned(), diagnostics); + publications_by_uri.insert(published.uri.as_str().to_owned(), accepted); quiet_deadline = Some(refresh_deadline.min(Instant::now() + timeouts.diagnostics_quiet)); } @@ -1470,7 +1582,7 @@ impl StdioLspClient { // looks partial here. Requiring every requested URI throws away the real // diagnostics of the one file that did report. Only treat the batch as a // genuine timeout when nothing arrived at all. - if diagnostics_by_uri.is_empty() && !uri_to_document.is_empty() { + if publications_by_uri.is_empty() && !uri_to_document.is_empty() { return Err(LspRefreshError::NoPublication { message: format!( "LSP diagnostics collection timed out after {} ms without a publication", @@ -1478,6 +1590,7 @@ impl StdioLspClient { ), }); } + let mut collected = Vec::new(); for (uri, (identity, document)) in &uri_to_document { if document .snapshot @@ -1485,8 +1598,12 @@ impl StdioLspClient { .is_some_and(|snapshot| !snapshot.lifetime.is_open()) { coverage_by_uri.insert(uri.clone(), DiagnosticDocumentCoverage::StalePublication); - if let Some(current) = self.documents.get_mut(identity) { - current.diagnostics = None; + let mut state = self.documents.lock().map_err(document_refresh_error)?; + if let Some(current) = state.documents.get_mut(identity) { + current.accepted = None; + if !matches!(current.eligibility, PublicationEligibility::Refreshing(_)) { + current.eligibility = PublicationEligibility::Unverified; + } } } let coverage = coverage_by_uri @@ -1497,16 +1614,29 @@ impl StdioLspClient { // a newly received publication for a closed overlay must likewise // not replace or append to that retained evidence. if matches!(coverage, DiagnosticDocumentCoverage::Published { .. }) - && let Some(diagnostics) = diagnostics_by_uri.remove(uri) + && let Some(accepted) = publications_by_uri.remove(uri) { - result.diagnostics.extend(diagnostics); + result + .diagnostics + .extend(accepted.diagnostics.iter().cloned()); + if let Some(claim) = claims.remove(identity) + && let Some(version) = expected_versions.get(uri) + { + collected.push(claim.collected( + accepted, + document.clone(), + uri.clone(), + *version, + self.attempt, + )); + } } result .coverage .documents .insert(document.relative_path.clone(), coverage); } - Ok(result) + Ok((result, collected)) } async fn closed_refresh_error(&self, deadline: Instant) -> LspRefreshError { @@ -1556,6 +1686,7 @@ impl StdioLspClient { /// stores this future when an in-flight owner is dropped, and no successor /// may acquire that slot until the future completes. pub(crate) fn reap(mut self) -> impl Future> + Send { + self.documents.retire(Some(self.attempt)); // The slot retains this future so no successor can overlap the old // process. Start termination before returning it: cancellation may // have no successor to poll the retained settlement future. @@ -1572,6 +1703,12 @@ impl StdioLspClient { } } +impl Drop for StdioLspClient { + fn drop(&mut self) { + self.documents.retire(Some(self.attempt)); + } +} + fn is_current_diagnostic_publication( published: &PublishDiagnosticsParams, expected_versions: &BTreeMap, @@ -1704,6 +1841,12 @@ fn semantic_write_error(error: LspMessageWriteError) -> LspSemanticRequestError } } +fn document_refresh_error(error: DiagnosticEvidenceUnavailable) -> LspRefreshError { + LspRefreshError::InvalidResponse { + message: error.to_string(), + } +} + fn refresh_transport_error(error: TraceDecayError) -> LspRefreshError { LspRefreshError::Transport { message: error.to_string(), @@ -1965,16 +2108,6 @@ pub(crate) fn file_uri(path: &Path) -> String { file_uri_from_path_text(&absolute.to_string_lossy()) } -/// Key the shared ledger by filesystem identity while keeping the admitted -/// presentation URI on the wire. A disk sweep through the registered root -/// and an editor request through a root alias must reach the same record. -fn document_identity(path: &Path, uri: &str) -> Option { - let (_, uri_path) = crate::gateway::strict_file_uri_path(uri)?; - let path = canonicalize_existing_prefix(path)?; - let uri_path = canonicalize_existing_prefix(&uri_path)?; - (path == uri_path).then_some(path) -} - fn read_semantic_document(path: &Path) -> std::result::Result { let file = std::fs::File::open(path) .map_err(|error| format!("failed to read semantic document: {error}"))?; @@ -2117,10 +2250,11 @@ mod tests { use serde_json::json; use super::{ - AnalyzerLaunch, JsonRpcMessage, LspRefreshTimeouts, LspSemanticRequestError, - NativeTaskOutcome, StdioLspClientStartup, StdioLspClientStartupSettlement, - cancel_request_message, decode_message, decode_semantic_request, encode_semantic_request, - file_uri_from_path_text, is_current_diagnostic_publication, + AnalyzerDocuments, AnalyzerLaunch, JsonRpcMessage, LspRefreshTimeouts, + LspSemanticRequestError, NativeTaskOutcome, StdioLspClientStartup, + StdioLspClientStartupSettlement, cancel_request_message, decode_message, + decode_semantic_request, encode_semantic_request, file_uri_from_path_text, + is_current_diagnostic_publication, }; #[cfg(unix)] @@ -2175,6 +2309,8 @@ while True: Duration::from_secs(10), Duration::from_millis(50), ), + AnalyzerDocuments::new(), + 0, ) .unwrap(); assert!(matches!( @@ -2229,6 +2365,8 @@ while True: Vec::new(), temp.path().to_path_buf(), LspRefreshTimeouts::from_diagnostics_quiet_window(Duration::from_secs(1)), + AnalyzerDocuments::new(), + 0, ) .unwrap(); assert!(matches!( diff --git a/crates/tracedecay-lsp/src/analyzer/client/documents.rs b/crates/tracedecay-lsp/src/analyzer/client/documents.rs new file mode 100644 index 0000000000..a0e842bd43 --- /dev/null +++ b/crates/tracedecay-lsp/src/analyzer/client/documents.rs @@ -0,0 +1,274 @@ +use std::collections::BTreeMap; +use std::path::{Path, PathBuf}; +use std::sync::{Arc, Mutex, MutexGuard}; + +use tracedecay_runtime_core::path_safety::canonicalize_existing_prefix; + +use super::{DiagnosticDocumentCoverage, LspDocument}; +use crate::LspSemanticDocumentSnapshot; +use crate::analyzer::broker::{CodeDiagnostic, DiagnosticEvidenceUnavailable}; + +pub(crate) struct AnalyzerDocuments { + state: Mutex, +} + +pub(super) struct DocumentState { + pub(super) attempt: Option, + pub(super) documents: BTreeMap, +} + +pub(super) struct SynchronizedDocument { + pub(super) uri: String, + pub(super) wire_version: i32, + pub(super) text: String, + pub(super) snapshot: Option, + pub(super) accepted: Option>, + pub(super) epoch: u64, + pub(super) eligibility: PublicationEligibility, +} + +pub(super) enum PublicationEligibility { + Unverified, + Refreshing(Arc), + Committed, +} + +pub(crate) struct AcceptedDiagnosticPublication { + pub(crate) diagnostics: Vec, + pub(crate) coverage: DiagnosticDocumentCoverage, +} + +pub(super) struct RefreshClaimIdentity { + path: PathBuf, + epoch: u64, +} + +pub(crate) struct DocumentRefreshClaim { + owner: Arc, + identity: Arc, +} + +pub(crate) struct CollectedDocumentPublication { + claim: DocumentRefreshClaim, + accepted: Arc, + document: LspDocument, + uri: String, + wire_version: i32, + attempt: u32, +} + +impl AnalyzerDocuments { + pub(crate) fn new() -> Arc { + Arc::new(Self { + state: Mutex::new(DocumentState { + attempt: None, + documents: BTreeMap::new(), + }), + }) + } + + pub(super) fn lock( + &self, + ) -> Result, DiagnosticEvidenceUnavailable> { + self.state + .lock() + .map_err(|_| DiagnosticEvidenceUnavailable::MetadataUnavailable) + } + + pub(crate) fn activate(&self, attempt: u32) -> Result<(), DiagnosticEvidenceUnavailable> { + let mut state = self.lock()?; + state.documents.clear(); + state.attempt = Some(attempt); + Ok(()) + } + + pub(crate) fn retire(&self, attempt: Option) { + let mut state = match self.state.lock() { + Ok(state) => state, + Err(poisoned) => poisoned.into_inner(), + }; + if attempt.is_none() || state.attempt == attempt { + state.attempt = None; + state.documents.clear(); + } + } + + pub(crate) fn invalidate( + &self, + paths: Option<&[PathBuf]>, + epoch: u64, + ) -> Result<(), DiagnosticEvidenceUnavailable> { + let mut state = self.lock()?; + let invalidate = |document: &mut SynchronizedDocument| { + document.epoch = epoch; + document.eligibility = PublicationEligibility::Unverified; + }; + match paths { + Some(paths) => { + for path in paths { + if let Some(document) = state.documents.get_mut(path) { + invalidate(document); + } + } + } + None => state.documents.values_mut().for_each(invalidate), + } + Ok(()) + } + + pub(crate) fn claim( + self: &Arc, + path: PathBuf, + epoch: u64, + ) -> Result { + let claim = DocumentRefreshClaim { + owner: Arc::clone(self), + identity: Arc::new(RefreshClaimIdentity { path, epoch }), + }; + claim.bind()?; + Ok(claim) + } + + pub(crate) fn read_committed( + &self, + path: &Path, + uri: &str, + document: &LspDocument, + read: impl FnOnce(&DiagnosticDocumentCoverage) -> T, + ) -> Result { + let state = self.lock()?; + if state.attempt.is_none() { + return Err(DiagnosticEvidenceUnavailable::IncarnationRetired); + } + let current = state + .documents + .get(path) + .ok_or(DiagnosticEvidenceUnavailable::SourceUnavailable)?; + validate_document(current, uri, document)?; + if !matches!(current.eligibility, PublicationEligibility::Committed) { + return Err(DiagnosticEvidenceUnavailable::PublicationUncommitted); + } + let accepted = current + .accepted + .as_ref() + .ok_or(DiagnosticEvidenceUnavailable::PublicationUncommitted)?; + Ok(read(&accepted.coverage)) + } +} + +impl DocumentRefreshClaim { + pub(crate) fn path(&self) -> &Path { + &self.identity.path + } + + pub(crate) fn is_current(&self) -> Result { + let state = self.owner.lock()?; + Ok(state.documents.get(self.path()).is_none_or(|document| { + document.epoch < self.identity.epoch + || matches!(&document.eligibility, PublicationEligibility::Refreshing(identity) if Arc::ptr_eq(identity, &self.identity)) + })) + } + + pub(crate) fn bind(&self) -> Result<(), DiagnosticEvidenceUnavailable> { + let mut state = self.owner.lock()?; + if let Some(document) = state.documents.get_mut(self.path()) { + if document.epoch > self.identity.epoch { + return Err(DiagnosticEvidenceUnavailable::PublicationUncommitted); + } + document.epoch = self.identity.epoch; + document.eligibility = PublicationEligibility::Refreshing(Arc::clone(&self.identity)); + } + Ok(()) + } + + pub(crate) fn collected( + self, + accepted: Arc, + document: LspDocument, + uri: String, + wire_version: i32, + attempt: u32, + ) -> CollectedDocumentPublication { + CollectedDocumentPublication { + claim: self, + accepted, + document, + uri, + wire_version, + attempt, + } + } +} + +impl Drop for DocumentRefreshClaim { + fn drop(&mut self) { + let mut state = match self.owner.state.lock() { + Ok(state) => state, + Err(poisoned) => poisoned.into_inner(), + }; + if let Some(document) = state.documents.get_mut(self.path()) + && matches!(&document.eligibility, PublicationEligibility::Refreshing(identity) if Arc::ptr_eq(identity, &self.identity)) + { + document.eligibility = PublicationEligibility::Unverified; + } + } +} + +impl CollectedDocumentPublication { + pub(crate) fn relative_path(&self) -> &str { + &self.document.relative_path + } + + pub(crate) fn commit( + &self, + install: impl FnOnce(&AcceptedDiagnosticPublication), + ) -> Result<(), DiagnosticEvidenceUnavailable> { + let mut state = self.claim.owner.lock()?; + if state.attempt != Some(self.attempt) { + return Err(DiagnosticEvidenceUnavailable::IncarnationRetired); + } + let current = state + .documents + .get_mut(self.claim.path()) + .ok_or(DiagnosticEvidenceUnavailable::SourceUnavailable)?; + validate_document(current, &self.uri, &self.document)?; + if current.wire_version != self.wire_version + || !matches!(¤t.eligibility, PublicationEligibility::Refreshing(identity) if Arc::ptr_eq(identity, &self.claim.identity)) + || !current + .accepted + .as_ref() + .is_some_and(|accepted| Arc::ptr_eq(accepted, &self.accepted)) + { + return Err(DiagnosticEvidenceUnavailable::PublicationUncommitted); + } + install(&self.accepted); + current.eligibility = PublicationEligibility::Committed; + Ok(()) + } +} + +fn validate_document( + current: &SynchronizedDocument, + uri: &str, + document: &LspDocument, +) -> Result<(), DiagnosticEvidenceUnavailable> { + if current.uri != uri || current.text != document.text { + return Err(DiagnosticEvidenceUnavailable::ContentChanged); + } + if current.snapshot != document.snapshot + || current + .snapshot + .as_ref() + .is_some_and(|snapshot| !snapshot.lifetime.is_open()) + { + return Err(DiagnosticEvidenceUnavailable::OverlayStale); + } + Ok(()) +} + +pub(crate) fn document_identity(path: &Path, uri: &str) -> Option { + let (_, uri_path) = crate::gateway::strict_file_uri_path(uri)?; + let path = canonicalize_existing_prefix(path)?; + let uri_path = canonicalize_existing_prefix(&uri_path)?; + (path == uri_path).then_some(path) +} diff --git a/crates/tracedecay-lsp/src/diagnostics.rs b/crates/tracedecay-lsp/src/diagnostics.rs index 67152dc624..82c1a899f1 100644 --- a/crates/tracedecay-lsp/src/diagnostics.rs +++ b/crates/tracedecay-lsp/src/diagnostics.rs @@ -7,6 +7,8 @@ use std::collections::BTreeSet; use crate::analyzer::broker::DiagnosticSeverity; +use crate::rpc::{DiagnosticSerializationCapabilities, diagnostic_value}; +use tracedecay_domain::ManifestDigest; pub const MAX_DOCUMENT_DIAGNOSTICS: usize = 200; pub const MAX_DIAGNOSTIC_MESSAGE_BYTES: usize = 512; @@ -321,6 +323,32 @@ pub fn merge_diagnostics( DiagnosticMerge::new(upstream, tracedecay) } +#[derive(Debug, Clone, Copy, PartialEq, Eq, thiserror::Error)] +#[error("diagnostic projection identity is unavailable")] +pub struct DiagnosticProjectionError; + +/// Returns the bounded upstream projection and its full observable identity. +pub fn project_upstream_diagnostics( + diagnostics: Vec, +) -> Result<(Vec, ManifestDigest), DiagnosticProjectionError> { + let items = merge_diagnostics(diagnostics, Vec::new()).into_items(); + let identity = items + .iter() + .map(|item| { + ( + &item.uri, + diagnostic_value( + item.clone(), + DiagnosticSerializationCapabilities::full_identity(), + ), + ) + }) + .collect::>(); + let digest = + tracedecay_domain::canonical_sha256(&identity).map_err(|_| DiagnosticProjectionError)?; + Ok((items, digest)) +} + /// UTF position conversion failures are explicit; positions inside a surrogate /// pair are never rounded to a neighboring byte. #[derive(Clone, Copy, Debug, Eq, PartialEq)] @@ -466,6 +494,39 @@ mod tests { } } + #[test] + fn upstream_projection_identity_uses_normalized_order_and_complete_wire_fields() { + let a = diagnostic(DiagnosticSource::Upstream, "a"); + let b = diagnostic(DiagnosticSource::TraceDecay, "b"); + let (items, digest) = + project_upstream_diagnostics(vec![b.clone(), a.clone(), a.clone()]).unwrap(); + let (reordered, reordered_digest) = + project_upstream_diagnostics(vec![a.clone(), b]).unwrap(); + assert_eq!(items, reordered); + assert_eq!(digest, reordered_digest); + assert!( + items + .iter() + .all(|item| item.source == DiagnosticSource::Upstream) + ); + + let (_, original) = project_upstream_diagnostics(vec![a.clone()]).unwrap(); + let mut related = a.clone(); + related + .related_information + .push(GatewayDiagnosticRelatedInformation { + uri: "file:///root/b.rs".to_owned(), + range: a.range, + message: "related".to_owned(), + }); + let (_, with_related) = project_upstream_diagnostics(vec![related]).unwrap(); + assert_ne!(original, with_related); + let mut described = a; + described.code_description_uri = Some("https://example.test/diagnostic".to_owned()); + let (_, with_description) = project_upstream_diagnostics(vec![described]).unwrap(); + assert_ne!(original, with_description); + } + #[test] fn merge_is_deterministic_deduplicated_and_provenance_preserving() { let upstream = diagnostic(DiagnosticSource::TraceDecay, "same"); diff --git a/crates/tracedecay-lsp/src/lib.rs b/crates/tracedecay-lsp/src/lib.rs index 1828c696c4..902c97a6c0 100644 --- a/crates/tracedecay-lsp/src/lib.rs +++ b/crates/tracedecay-lsp/src/lib.rs @@ -77,14 +77,14 @@ pub use context::{ TRACEDECAY_CONTEXT_METHOD, TRACEDECAY_CONTEXT_REVISION, TRACEDECAY_SUBSCRIBE_METHOD, }; pub use diagnostics::{ - DiagnosticMerge, DiagnosticSource, DocumentDiagnosticReport, GatewayDiagnostic, - GatewayDiagnosticCoverage, GatewayDiagnosticData, GatewayDiagnosticIdentity, + DiagnosticMerge, DiagnosticProjectionError, DiagnosticSource, DocumentDiagnosticReport, + GatewayDiagnostic, GatewayDiagnosticCoverage, GatewayDiagnosticData, GatewayDiagnosticIdentity, GatewayDiagnosticLifecycle, GatewayDiagnosticProviderState, GatewayDiagnosticRelatedInformation, LspPosition, LspRange, MAX_DIAGNOSTIC_MESSAGE_BYTES, MAX_DIAGNOSTIC_RELATED_INFORMATION, MAX_DIAGNOSTIC_RELATED_MESSAGE_BYTES, MAX_DIAGNOSTIC_URI_BYTES, MAX_DOCUMENT_DIAGNOSTICS, PositionError, TRACEDECAY_DIAGNOSTIC_DATA_REVISION, byte_offset_to_utf16_position, merge_diagnostics, - utf16_position_to_byte_offset, + project_upstream_diagnostics, utf16_position_to_byte_offset, }; pub use framing::{ AsyncContentLengthError, ContentLengthCodec, ContentLengthCodecError, MAX_LSP_HEADER_BYTES, diff --git a/crates/tracedecay-lsp/tests/lsp_suite/analyzer_runtime/broker_refresh.rs b/crates/tracedecay-lsp/tests/lsp_suite/analyzer_runtime/broker_refresh.rs index f96336f169..ec59d9ed31 100644 --- a/crates/tracedecay-lsp/tests/lsp_suite/analyzer_runtime/broker_refresh.rs +++ b/crates/tracedecay-lsp/tests/lsp_suite/analyzer_runtime/broker_refresh.rs @@ -3399,3 +3399,371 @@ async fn semantic_cancellation_reaches_and_releases_the_registered_operation() { assert!(!authority.cancel_request(&root, &request_id)); assert!(!authority.cancel_request(&root, &LspRequestId::Number(20))); } + +#[tokio::test] +async fn partial_root_refresh_replaces_only_published_document_diagnostics() { + let temp = tempfile::tempdir().unwrap(); + for root in [temp.path().to_path_buf(), temp.path().join("member")] { + std::fs::create_dir_all(root.join("src")).unwrap(); + std::fs::write(root.join("workspace.marker"), "").unwrap(); + } + let script_path = temp.path().join("partial_root_lsp.py"); + let publish = FAKE_DIAGNOSTIC_PUBLISH.replace( + r#""fake type error""#, + r#""old b" if uri.endswith("b.fake") else ("old a" if text == "old a" else "new a")"#, + ); + let body = format!( + r#" text = message["params"].get("contentChanges", [message["params"]["textDocument"]])[0]["text"] + if text == "fail b": + sys.exit(17) +{publish}"# + ); + std::fs::write(&script_path, fake_lsp_script_with_preamble("", &body)).unwrap(); + let adapter = fake_adapter_with_root_markers( + FAKE_LANGUAGE, + "fake", + python_command(), + vec![script_path.display().to_string()], + vec!["workspace.marker".to_owned()], + ); + let mut broker = lsp::broker::DiagnosticBroker::new_for_test(temp.path(), vec![adapter]); + broker + .refresh_documents_with_timeouts( + FAKE_LANGUAGE, + vec![ + fake_document(FAKE_LANGUAGE, "src/a.fake", "old a"), + fake_document(FAKE_LANGUAGE, "member/src/b.fake", "old b"), + ], + loaded_runner_fake_lsp_timeouts(), + ) + .await + .unwrap(); + let completed = broker + .prepare_refresh( + FAKE_LANGUAGE, + vec![ + fake_document(FAKE_LANGUAGE, "src/a.fake", "new a"), + fake_document(FAKE_LANGUAGE, "member/src/b.fake", "fail b"), + ], + ) + .await + .unwrap() + .unwrap() + .collect_diagnostics_with_timeouts(loaded_runner_fake_lsp_timeouts()) + .await; + assert!( + broker.finish_refresh_snapshot(completed).is_err(), + "the caller must see that one root was incomplete" + ); + let diagnostics = broker.snapshot().diagnostics; + assert_eq!(diagnostics.len(), 2); + assert!( + diagnostics + .iter() + .any(|diagnostic| diagnostic.file == "src/a.fake" && diagnostic.message == "new a") + ); + assert!( + diagnostics + .iter() + .any(|diagnostic| diagnostic.file == "member/src/b.fake" + && diagnostic.message == "old b") + ); + let a = fake_document(FAKE_LANGUAGE, "src/a.fake", "new a"); + assert!(matches!( + broker.document_diagnostic_evidence(&a), + Ok(lsp::broker::BrokerDocumentEvidence::Published { .. }) + )); + let b = fake_document(FAKE_LANGUAGE, "member/src/b.fake", "old b"); + assert!(broker.document_diagnostic_evidence(&b).is_err()); + broker + .refresh_documents_with_timeouts(FAKE_LANGUAGE, vec![a], loaded_runner_fake_lsp_timeouts()) + .await + .unwrap(); + assert!(broker.document_diagnostic_evidence(&b).is_err()); +} + +#[tokio::test] +async fn queue_denial_keeps_requested_evidence_unavailable_after_an_unrelated_refresh() { + let temp = tempfile::tempdir().unwrap(); + let script_path = temp.path().join("evidence_lsp.py"); + std::fs::write(&script_path, fake_lsp_script()).unwrap(); + let mut broker = lsp::broker::DiagnosticBroker::new_for_test( + temp.path(), + vec![fake_python_adapter(FAKE_LANGUAGE, "fake", &script_path)], + ); + let a = fake_document(FAKE_LANGUAGE, "src/a.fake", "a"); + let b = fake_document(FAKE_LANGUAGE, "src/b.fake", "b"); + broker + .refresh_documents_with_timeouts( + FAKE_LANGUAGE, + vec![a.clone(), b.clone()], + loaded_runner_fake_lsp_timeouts(), + ) + .await + .unwrap(); + assert!(broker.document_diagnostic_evidence(&a).is_ok()); + let mut queued = Vec::new(); + for _ in 0..lsp::broker::MAX_ANALYZER_QUEUED_ROOT_BATCHES { + queued.push( + broker + .prepare_refresh(FAKE_LANGUAGE, vec![b.clone()]) + .await + .unwrap() + .unwrap(), + ); + } + let Err(error) = broker.prepare_refresh(FAKE_LANGUAGE, vec![a.clone()]).await else { + panic!("a full queue must reject the requested document before client acquisition"); + }; + assert!(error.to_string().contains("queue saturated")); + drop(queued); + broker + .refresh_documents_with_timeouts( + FAKE_LANGUAGE, + vec![b.clone()], + loaded_runner_fake_lsp_timeouts(), + ) + .await + .unwrap(); + assert!(broker.document_diagnostic_evidence(&b).is_ok()); + assert!(matches!( + broker.document_diagnostic_evidence(&a), + Err(lsp::broker::DiagnosticEvidenceUnavailable::PublicationUncommitted) + )); + let snapshot = broker.snapshot(); + assert_eq!(snapshot.diagnostics.len(), 2); + assert!( + !snapshot.coverage[FAKE_LANGUAGE] + .documents + .contains_key(&a.relative_path) + ); + broker + .refresh_documents_with_timeouts( + FAKE_LANGUAGE, + vec![a.clone()], + loaded_runner_fake_lsp_timeouts(), + ) + .await + .unwrap(); + assert!(broker.document_diagnostic_evidence(&a).is_ok()); +} + +#[tokio::test] +async fn identical_empty_collection_requires_commit_and_stale_cleanup_preserves_its_successor() { + let temp = tempfile::tempdir().unwrap(); + let script_path = temp.path().join("empty_evidence_lsp.py"); + std::fs::write( + &script_path, + fake_lsp_script_with_preamble("", EMPTY_DIAGNOSTIC_PUBLISH), + ) + .unwrap(); + let mut broker = lsp::broker::DiagnosticBroker::new_for_test( + temp.path(), + vec![fake_python_adapter(FAKE_LANGUAGE, "fake", &script_path)], + ); + let a = fake_document(FAKE_LANGUAGE, "src/a.fake", "a"); + broker + .refresh_documents_with_timeouts( + FAKE_LANGUAGE, + vec![a.clone()], + loaded_runner_fake_lsp_timeouts(), + ) + .await + .unwrap(); + let initial = broker.snapshot(); + let stale = broker + .prepare_refresh(FAKE_LANGUAGE, vec![a.clone()]) + .await + .unwrap() + .unwrap() + .collect_diagnostics_with_timeouts(loaded_runner_fake_lsp_timeouts()) + .await; + assert!(stale.is_ok()); + assert!(stale.diagnostic_refresh().unwrap().diagnostics.is_empty()); + assert_eq!( + stale.diagnostic_refresh().unwrap().coverage, + initial.coverage[FAKE_LANGUAGE] + ); + assert!(matches!( + broker.document_diagnostic_evidence(&a), + Err(lsp::broker::DiagnosticEvidenceUnavailable::PublicationUncommitted) + )); + let abandoned = broker + .prepare_refresh(FAKE_LANGUAGE, vec![a.clone()]) + .await + .unwrap() + .unwrap() + .collect_diagnostics_with_timeouts(loaded_runner_fake_lsp_timeouts()) + .await; + broker + .refresh_documents_with_timeouts( + FAKE_LANGUAGE, + vec![a.clone()], + loaded_runner_fake_lsp_timeouts(), + ) + .await + .unwrap(); + assert!(matches!( + broker.finish_refresh_snapshot(stale).unwrap(), + lsp::broker::RefreshCommitOutcome::Superseded + )); + drop(abandoned); + let lsp::broker::BrokerDocumentEvidence::Published { diagnostics, .. } = + broker.document_diagnostic_evidence(&a).unwrap() + else { + panic!("the successor empty publication must remain committed"); + }; + assert!(diagnostics.is_empty()); + let cancelled = broker + .prepare_refresh(FAKE_LANGUAGE, vec![a.clone()]) + .await + .unwrap() + .unwrap(); + drop(cancelled); + assert!(matches!( + broker.document_diagnostic_evidence(&a), + Err(lsp::broker::DiagnosticEvidenceUnavailable::PublicationUncommitted) + )); +} + +#[tokio::test] +async fn identical_settings_replacement_rejects_collected_work_from_the_old_client() { + let temp = tempfile::tempdir().unwrap(); + let script_path = temp.path().join("replaced_evidence_lsp.py"); + std::fs::write(&script_path, fake_lsp_script()).unwrap(); + let adapter = fake_python_adapter(FAKE_LANGUAGE, "fake", &script_path); + let mut broker = + lsp::broker::DiagnosticBroker::new_for_test(temp.path(), vec![adapter.clone()]); + let a = fake_document(FAKE_LANGUAGE, "src/a.fake", "a"); + broker + .refresh_documents_with_timeouts( + FAKE_LANGUAGE, + vec![a.clone()], + loaded_runner_fake_lsp_timeouts(), + ) + .await + .unwrap(); + let stale = broker + .prepare_refresh(FAKE_LANGUAGE, vec![a.clone()]) + .await + .unwrap() + .unwrap() + .collect_diagnostics_with_timeouts(loaded_runner_fake_lsp_timeouts()) + .await; + broker.update_settings(broker.snapshot().settings); + assert!(broker.document_diagnostic_evidence(&a).is_err()); + broker + .refresh_documents_with_timeouts( + FAKE_LANGUAGE, + vec![a.clone()], + loaded_runner_fake_lsp_timeouts(), + ) + .await + .unwrap(); + assert!(matches!( + broker.finish_refresh_snapshot(stale).unwrap(), + lsp::broker::RefreshCommitOutcome::Superseded + )); + assert!(broker.document_diagnostic_evidence(&a).is_ok()); + let stale = broker + .prepare_refresh(FAKE_LANGUAGE, vec![a.clone()]) + .await + .unwrap() + .unwrap(); + broker.update_adapters(vec![adapter]); + let stale = stale + .collect_diagnostics_with_timeouts(loaded_runner_fake_lsp_timeouts()) + .await; + assert!(matches!( + broker.finish_refresh_snapshot(stale).unwrap(), + lsp::broker::RefreshCommitOutcome::Superseded + )); + assert!(broker.document_diagnostic_evidence(&a).is_err()); + broker + .refresh_documents_with_timeouts( + FAKE_LANGUAGE, + vec![a.clone()], + loaded_runner_fake_lsp_timeouts(), + ) + .await + .unwrap(); + assert!(broker.document_diagnostic_evidence(&a).is_ok()); +} + +#[tokio::test] +async fn semantic_retirement_revokes_committed_diagnostic_evidence() { + let temp = tempfile::tempdir().unwrap(); + std::fs::create_dir_all(temp.path().join("src")).unwrap(); + std::fs::write(temp.path().join(FAKE_PATH), "a").unwrap(); + let control = FakeLspPhaseControl::bind().await; + let script_path = temp.path().join("semantic_evidence_lsp.py"); + let mut script = fake_lsp_script_with_preamble( + &control.script_preamble(&["semantic-request"]), + EMPTY_DIAGNOSTIC_PUBLISH, + ); + script.push_str( + r#" elif message.get("method") == "textDocument/documentSymbol": + tracedecay_maybe_phase("semantic-request") +"#, + ); + std::fs::write(&script_path, script).unwrap(); + let mut broker = lsp::broker::DiagnosticBroker::new_for_test( + temp.path(), + vec![fake_python_adapter(FAKE_LANGUAGE, "fake", &script_path)], + ); + let a = fake_document(FAKE_LANGUAGE, FAKE_PATH, "a"); + broker + .refresh_documents_with_timeouts( + FAKE_LANGUAGE, + vec![a.clone()], + loaded_runner_fake_lsp_timeouts(), + ) + .await + .unwrap(); + assert!(broker.document_diagnostic_evidence(&a).is_ok()); + let root_uri = url::Url::from_directory_path(temp.path()) + .unwrap() + .to_string(); + let authority = broker + .semantic_authority_if_available( + FAKE_LANGUAGE, + temp.path().to_path_buf(), + root_uri.clone(), + loaded_runner_fake_lsp_timeouts(), + ) + .await + .unwrap() + .unwrap(); + let root = AdmittedRoot::new(root_uri.clone()); + let id = LspRequestId::Number(31); + let operation = tokio::spawn( + authority.start( + root.clone(), + id.clone(), + tracedecay_lsp::lsp_semantic_request(&SemanticRequest::DocumentSymbols { + document_uri: format!("{root_uri}{FAKE_PATH}"), + }) + .unwrap(), + ), + ); + let reached = control.wait_for("semantic-request").await; + assert!(authority.cancel_request(&root, &id)); + assert!(matches!( + operation.await.unwrap(), + tracedecay_lsp::LspSemanticOperationOutcome::Partial { .. } + )); + drop(reached); + assert!(matches!( + broker.document_diagnostic_evidence(&a), + Err(lsp::broker::DiagnosticEvidenceUnavailable::IncarnationRetired) + )); + broker + .refresh_documents_with_timeouts( + FAKE_LANGUAGE, + vec![a.clone()], + loaded_runner_fake_lsp_timeouts(), + ) + .await + .unwrap(); + assert!(broker.document_diagnostic_evidence(&a).is_ok()); +} From 07c0cd68613188eb57df22c20cba4151e6ace4ac Mon Sep 17 00:00:00 2001 From: Zack Jackson Date: Sun, 25 Oct 2026 06:36:22 -0700 Subject: [PATCH 08/12] test(lsp): align diagnostic expectations with wire output --- .../src/lsp_support/runtime_adapters.rs | 2 +- .../src/protocol/tests/diagnostic_snapshots.rs | 15 +++++++-------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/crates/tracedecay-application/src/lsp_support/runtime_adapters.rs b/crates/tracedecay-application/src/lsp_support/runtime_adapters.rs index d2da4e6abc..812b216dc9 100644 --- a/crates/tracedecay-application/src/lsp_support/runtime_adapters.rs +++ b/crates/tracedecay-application/src/lsp_support/runtime_adapters.rs @@ -919,7 +919,7 @@ while True: replacement["result"]["items"], json!([{ "range": { "start": { "line": 0, "character": 3 }, "end": { "line": 0, "character": 7 } }, - "severity": 1, "source": "upstream", "message": "replacement finding", + "severity": 1, "code": null, "source": "upstream", "message": "replacement finding", }]) ); assert_eq!(harness.managed.snapshots.load(Ordering::SeqCst), 2); diff --git a/crates/tracedecay-lsp/src/protocol/tests/diagnostic_snapshots.rs b/crates/tracedecay-lsp/src/protocol/tests/diagnostic_snapshots.rs index 1e1c7373c9..0841a12b5c 100644 --- a/crates/tracedecay-lsp/src/protocol/tests/diagnostic_snapshots.rs +++ b/crates/tracedecay-lsp/src/protocol/tests/diagnostic_snapshots.rs @@ -1239,12 +1239,11 @@ fn validation_harness() -> (Harness, String) { .iter() .find(|message| message["method"] == "workspace/diagnostic/refresh") .unwrap(); - assert!( - harness - .send(json!({ - "jsonrpc": "2.0", "id": refresh["id"], "result": null, - })) - .is_empty() + assert_eq!( + harness.send(json!({ + "jsonrpc": "2.0", "id": refresh["id"], "result": null, + })), + Vec::::new() ); (harness, result_id) } @@ -1378,7 +1377,7 @@ fn settled_ready_validation_is_consumed_once_and_preserves_the_generation_floor( .store(false, Ordering::SeqCst); harness.runtime.complete_next(); harness.session.flush_due(harness.now_ms); - assert!(harness.messages().is_empty()); + assert_eq!(harness.messages(), Vec::::new()); *harness.authority.next_generation.lock().unwrap() = Some(0); harness.notify( @@ -1456,7 +1455,7 @@ fn late_validation_cannot_answer_a_reopened_document_or_retire_its_refresh() { .store(false, Ordering::SeqCst); harness.runtime.complete_next(); harness.session.flush_due(harness.now_ms); - assert!(harness.messages().is_empty()); + assert_eq!(harness.messages(), Vec::::new()); let pending = harness.pull(Some(&result_id)); assert_eq!(pending["error"]["code"], -32802, "{pending}"); assert!(pending["error"]["data"]["operationId"].as_str().is_some()); From b01d71212ee1ab59b0a34a7b5f0479497a4bfb1c Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sun, 25 Oct 2026 14:24:33 -0700 Subject: [PATCH 09/12] fix(ci): bound Windows compiler concurrency --- .github/workflows/ci.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3e82ac6c34..dc96d97cb4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -596,7 +596,9 @@ jobs: shell: bash # Compile and link every target, but defer unused test runfiles trees. # Bazel still materializes runfiles required by build-time tools. - run: bazel build --config=ci --nobuild_runfile_links //... + # Four concurrent Rust actions exhausted the hosted runner's commit + # limit while mapping asupersync metadata (Windows error 1455). + run: bazel build --config=ci --jobs=2 --nobuild_runfile_links //... - name: Build and test native targets if: ${{ github.event_name == 'workflow_dispatch' && inputs.run_os }} @@ -619,7 +621,7 @@ jobs: *) echo "Expected a Bazel target pattern: $target" >&2; exit 1 ;; esac done - bazel test --config=ci --test_output=all "${targets[@]}" + bazel test --config=ci --jobs=2 --test_output=all "${targets[@]}" - name: Upload failed test diagnostics if: failure() From 77b5705e40b2ea64cccf26bb61cf6226fba69eaf Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Sun, 25 Oct 2026 22:35:31 -0700 Subject: [PATCH 10/12] test(ci): synchronize seal crashes and retained stop retries --- .../sealed_generation_crash_test.rs | 72 ++++++++++++++++--- .../intent_pass_journey_test.rs | 17 ++++- 2 files changed, 76 insertions(+), 13 deletions(-) diff --git a/crates/tracedecay/tests/daemon_suite/sealed_generation_crash_test.rs b/crates/tracedecay/tests/daemon_suite/sealed_generation_crash_test.rs index 60b6302b7b..4574a60063 100644 --- a/crates/tracedecay/tests/daemon_suite/sealed_generation_crash_test.rs +++ b/crates/tracedecay/tests/daemon_suite/sealed_generation_crash_test.rs @@ -16,7 +16,9 @@ use std::time::Instant; use serde_json::json; use sha2::{Digest, Sha256}; use tracedecay_code_index_retention::code_index_generations::{ + CODE_INDEX_PREPARATION_DIRECTORY_PREFIX_V1, CodeGenerationStoreLockV1, code_generation_segments_root, code_text_artifacts_root, scoped_code_index_store_root, + try_acquire_code_generation_store_read_lock, }; use crate::code_index_journey::{ @@ -26,8 +28,11 @@ use crate::code_index_journey::{ use crate::common::{IsolatedHome, daemon_socket_path, spawn_tracedecay_daemon_with}; use tracedecay_runtime_core::path_safety::canonical_existing_identity; -const PUBLICATION_TEMPORARY_PREFIXES: [&str; 2] = - [".segment-publication.", ".evidence-pack-publication."]; +const PUBLICATION_TEMPORARY_PREFIXES: [&str; 3] = [ + ".segment-publication.", + ".evidence-pack-publication.", + CODE_INDEX_PREPARATION_DIRECTORY_PREFIX_V1, +]; fn scope_root(home: &Path, project: &Path) -> PathBuf { let layout = @@ -89,10 +94,47 @@ fn publication_temporaries(scope: &Path) -> Vec { .collect() } -fn segments_entry_count(scope: &Path) -> usize { - fs::read_dir(code_generation_segments_root(scope)) - .expect("segments directory") - .count() +fn hold_publication_commit(scope: &Path) -> CodeGenerationStoreLockV1 { + let deadline = Instant::now() + RECEIPT_TIMEOUT; + loop { + if let Some(lease) = try_acquire_code_generation_store_read_lock(scope) + .expect("read lease on the published generation") + { + return lease; + } + assert!( + Instant::now() < deadline, + "the published generation stayed busy" + ); + std::thread::sleep(std::time::Duration::from_millis(1)); + } +} + +fn has_staged_file_segment(scope: &Path) -> bool { + for entry in fs::read_dir(code_generation_segments_root(scope)).expect("segments directory") { + let entry = entry.expect("segments entry"); + if !entry + .file_name() + .to_string_lossy() + .starts_with(CODE_INDEX_PREPARATION_DIRECTORY_PREFIX_V1) + { + continue; + } + let staged = match fs::read_dir(entry.path()) { + Ok(staged) => staged, + // A source update can retire an earlier preparation before its + // successor starts. Observe the successor on the next probe. + Err(error) if error.kind() == std::io::ErrorKind::NotFound => continue, + Err(error) => panic!("read generation preparation: {error}"), + }; + if staged + .map(|entry| entry.expect("staged segment entry")) + .any(|entry| entry.file_name().to_string_lossy().starts_with("segment-")) + { + return true; + } + } + false } fn write_batch(project: &Path, files: u32, symbols: u32) { @@ -229,9 +271,12 @@ async fn sigkill_during_the_seal_keeps_the_prior_generation_published() { ) .await; let scope = scope_root(environment.home(), &project); + // The real reader lease permits segment preparation but excludes the + // final publication transaction. Keep it through SIGKILL so even a fast + // writer cannot advance the pointer between observation and the kill. + let publication_hold = hold_publication_commit(&scope); let prior_pointer = active_pointer(&scope); let prior_family = sealed_family(&scope); - let prior_entries = segments_entry_count(&scope); write_batch(&project, 128, 16); let batch_revision = commit_all(&project, "install the batch the killed seal was writing"); @@ -243,10 +288,10 @@ async fn sigkill_during_the_seal_keeps_the_prior_generation_published() { ) .await; assert_eq!(receipt["status"], "queued", "refresh receipt: {receipt}"); - // The seal's first segment write is the first new entry beside the prior - // generation's segments; kill the daemon while that seal is in flight. + // Segments now accumulate inside one preparation directory. Top-level + // entry counts only advance when commit names the complete batch. let deadline = Instant::now() + RECEIPT_TIMEOUT; - while segments_entry_count(&scope) < prior_entries + 2 { + while !has_staged_file_segment(&scope) { assert!( Instant::now() < deadline, "the refresh never started sealing its segments" @@ -254,6 +299,7 @@ async fn sigkill_during_the_seal_keeps_the_prior_generation_published() { std::thread::sleep(std::time::Duration::from_millis(1)); } let killed = daemon.kill_and_wait().expect("SIGKILL the sealing daemon"); + drop(publication_hold); assert!( !killed.success(), "SIGKILL must not look like a graceful exit" @@ -286,6 +332,12 @@ async fn sigkill_during_the_seal_keeps_the_prior_generation_published() { ) .await; assert_ne!(rebuilt.generation_id, prior.generation_id); + // The exclusive segment sweep owns abandoned preparation directories; + // it runs independently of the restarted daemon's generation rebuild. + let deadline = Instant::now() + RECEIPT_TIMEOUT; + while !publication_temporaries(&scope).is_empty() && Instant::now() < deadline { + std::thread::sleep(std::time::Duration::from_millis(10)); + } assert_eq!( publication_temporaries(&scope), Vec::::new(), diff --git a/crates/tracedecay/tests/hooks_lsp_suite/intent_pass_journey_test.rs b/crates/tracedecay/tests/hooks_lsp_suite/intent_pass_journey_test.rs index 6fc34da9e4..d1d0588f23 100644 --- a/crates/tracedecay/tests/hooks_lsp_suite/intent_pass_journey_test.rs +++ b/crates/tracedecay/tests/hooks_lsp_suite/intent_pass_journey_test.rs @@ -643,10 +643,21 @@ fn claude_stop_delivers_the_intent_suggestion_the_daemon_judged_at_the_tool_boun |(emitted, suppressed)| emitted == 1 && suppressed >= 1, ); let after_mount = intent_outcomes(&log, ENABLED_SESSION)[before + 1..].to_vec(); + // Retained stop attempts may truthfully finish unavailable before their + // replay succeeds. The retry loop above already requires an explicit + // retryable receipt for every repeated stop; the settled pass must still + // suppress the delivered suggestion. + assert_eq!( + after_mount.last().map(String::as_str), + Some("suppressed"), + "the settled stop intent pass must suppress its delivered suggestion: {after_mount:?}" + ); assert!( - !after_mount.is_empty() && after_mount.iter().all(|outcome| outcome == "suppressed"), - "the stop boundary's own intent pass withholds the suggestion it just delivered: \ - {after_mount:?}" + after_mount.iter().all(|outcome| { + outcome == "suppressed" || (outcome == "unavailable" && stop_outputs.len() > 1) + }), + "only a stop with verified retryable admission may recover from unavailable before \ + suppressing the delivered suggestion: {after_mount:?}" ); assert_eq!( hint_counts(home, &project, YES_INTENT).0, From 8d98ae1e161274f8c8d16008fe8d098c5a970e4a Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Sun, 25 Oct 2026 22:35:32 -0700 Subject: [PATCH 11/12] ci(cli): classify the full refresh memory workload as large MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fleet logs show five complete 48,000-function generations take 53–65 seconds each. The default medium deadline kills the final refresh near 300 seconds despite steady successful publication. Declare the measured aggregate workload through the existing Cargo-to-Bazel size metadata, preserving every refresh, memory bound, and assertion. --- crates/tracedecay-cli/BUILD.bazel | 1 + crates/tracedecay-cli/Cargo.toml | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/crates/tracedecay-cli/BUILD.bazel b/crates/tracedecay-cli/BUILD.bazel index 63c85c0adc..54557ecf2a 100644 --- a/crates/tracedecay-cli/BUILD.bazel +++ b/crates/tracedecay-cli/BUILD.bazel @@ -152,6 +152,7 @@ rust_test( rust_test( name = "refresh_memory_journey", + size = "large", srcs = glob(["tests/**/*.rs"], exclude = ["tests/fixtures/**"]) + glob(["build-support/**/*.rs", "src/**/*.rs"]) + ["//crates/tracedecay:tests_common_sources", "//tests:support_sources"], crate_root = "tests/refresh_memory_journey.rs", edition = crate_edition(), diff --git a/crates/tracedecay-cli/Cargo.toml b/crates/tracedecay-cli/Cargo.toml index 7f885fff17..a8175f2c44 100644 --- a/crates/tracedecay-cli/Cargo.toml +++ b/crates/tracedecay-cli/Cargo.toml @@ -7,6 +7,12 @@ description = "TraceDecay command-line interface and daemon binary" license = "MIT" repository = "https://github.com/ScriptedAlchemy/tracedecay" +# Five complete 48,000-function generations take 53–65 seconds each on the +# four-vCPU fleet. Keep the memory workload intact instead of letting the +# default five-minute medium deadline interrupt its final verification. +[package.metadata.bazel.test_sizes] +refresh_memory_journey = "large" + [[bin]] name = "tracedecay" path = "src/main.rs" From 13b3cd0044e5e25338a3e571b9c16045da9d474e Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Mon, 26 Oct 2026 16:31:06 -0700 Subject: [PATCH 12/12] test(daemon): observe crash cleanup on its maintenance cadence Use the reader lease to fix the interrupted publication point, then track only its abandoned temporary names through the scheduled cleanup sweep. Keep the multi-file fixture within one bounded retention pass and derive the observation window from the actual maintenance retry cadence. --- .../sealed_generation_crash_test.rs | 43 ++++++++++++++----- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/crates/tracedecay/tests/daemon_suite/sealed_generation_crash_test.rs b/crates/tracedecay/tests/daemon_suite/sealed_generation_crash_test.rs index 4574a60063..be15308ded 100644 --- a/crates/tracedecay/tests/daemon_suite/sealed_generation_crash_test.rs +++ b/crates/tracedecay/tests/daemon_suite/sealed_generation_crash_test.rs @@ -4,14 +4,14 @@ //! temporary name, flushed once, and only then named; the manifest and the //! active pointer follow. A kill after the pointer is durable must restart on //! the exact sealed bytes. A kill before it must leave the prior generation -//! published and no partial generation or temporary behind once the restarted -//! daemon rebuilds. +//! published, then rebuild and collect the abandoned publication through the +//! restarted daemon's scheduled maintenance. use std::collections::BTreeMap; use std::fmt::Write as _; use std::fs; use std::path::{Path, PathBuf}; -use std::time::Instant; +use std::time::{Duration, Instant}; use serde_json::json; use sha2::{Digest, Sha256}; @@ -20,6 +20,8 @@ use tracedecay_code_index_retention::code_index_generations::{ code_generation_segments_root, code_text_artifacts_root, scoped_code_index_store_root, try_acquire_code_generation_store_read_lock, }; +use tracedecay_configuration::RetentionConfig; +use tracedecay_maintenance::tick::MaintenanceCadence; use crate::code_index_journey::{ RECEIPT_TIMEOUT, commit_all, exact_identity, git, initialize_tracedecay, @@ -278,7 +280,10 @@ async fn sigkill_during_the_seal_keeps_the_prior_generation_published() { let prior_pointer = active_pointer(&scope); let prior_family = sealed_family(&scope); - write_batch(&project, 128, 16); + // The reader lease fixes the kill point; a large batch is unnecessary + // and can spread abandoned-file collection across several maintenance + // ticks. Keep this multi-file publication within one bounded sweep. + write_batch(&project, 16, 16); let batch_revision = commit_all(&project, "install the batch the killed seal was writing"); let receipt = tool( &socket, @@ -317,6 +322,11 @@ async fn sigkill_during_the_seal_keeps_the_prior_generation_published() { "the kill damaged the prior generation's durable file {path}" ); } + let abandoned_temporaries = publication_temporaries(&scope); + assert!( + !abandoned_temporaries.is_empty(), + "the interrupted seal must leave publication work for recovery" + ); daemon = spawn_tracedecay_daemon_with(environment.home(), |_| {}); let rebuilt = wait_for_terminal_generation( @@ -327,19 +337,30 @@ async fn sigkill_during_the_seal_keeps_the_prior_generation_published() { "refs/heads/main", Some(&batch_revision), Some(&prior.generation_id), - "crash_probe_0127_015", - Some("src/batch/file_0127.rs"), + "crash_probe_0015_015", + Some("src/batch/file_0015.rs"), ) .await; assert_ne!(rebuilt.generation_id, prior.generation_id); - // The exclusive segment sweep owns abandoned preparation directories; - // it runs independently of the restarted daemon's generation rebuild. - let deadline = Instant::now() + RECEIPT_TIMEOUT; - while !publication_temporaries(&scope).is_empty() && Instant::now() < deadline { + // Rebuilding does not run retention: its next admitted tick is one retry + // delay away (a minute under the default policy). Start the unchanged + // completion budget after that scheduled delay. Observe only the killed + // seal's names, since a live successor may legitimately prepare work. + let cadence = MaintenanceCadence::new(Duration::from_hours( + RetentionConfig::default().interval_hours.max(1), + )); + let deadline = Instant::now() + cadence.retry_delay() + RECEIPT_TIMEOUT; + let pending_temporaries = || { + publication_temporaries(&scope) + .into_iter() + .filter(|name| abandoned_temporaries.contains(name)) + .collect::>() + }; + while !pending_temporaries().is_empty() && Instant::now() < deadline { std::thread::sleep(std::time::Duration::from_millis(10)); } assert_eq!( - publication_temporaries(&scope), + pending_temporaries(), Vec::::new(), "the restarted daemon must collect the killed seal's temporaries" );