Skip to content
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- **`LintConfig` cheap cloning**: Introduced `Arc<ConfigData>` inner struct to hold all serializable fields. Cloning a `LintConfig` (e.g., in `validate_project` / `validate_project_with_registry` parallel dispatch) now bumps an `Arc` refcount instead of deep-copying `Vec<String>` fields and nested structs. Mutations use `Arc::make_mut` for copy-on-write semantics, so the allocation only occurs when the `Arc` is actually shared (#467)

### Fixed
- **LSP document version tracking**: The LSP backend now tracks document versions reported by the client (`did_open`, `did_change`) and includes them in all `publish_diagnostics` calls. Editors that inspect diagnostic version tags (e.g., for stale-result suppression) now receive accurate version numbers instead of `None`. Version and content updates are atomized under a single lock acquisition so readers never observe a state where content and version are out of sync. Empty `did_change` notifications (no content changes) also correctly advance the tracked version per the LSP spec (#478)
- **Frontmatter leading newline stripped**: `split_frontmatter()` no longer includes the newline that follows the opening `---` delimiter in the extracted frontmatter string. Downstream validators (`AgentValidator`, `AmpValidator`, `KiroSteeringValidator`) have been updated to compute correct 1-based line numbers; diagnostic line numbers for AMP-001, CC-AG-007, and KIRO-001 through KIRO-004 are now accurate (#482)
- **Empty-frontmatter panic guard**: `split_frontmatter()` now uses `str::get()` instead of direct slice indexing when extracting frontmatter content, preventing an index-out-of-bounds panic on files with an opening `---` delimiter but no content (#482)
- **Predictable UUID Generation for Telemetry**: Replaced the custom, insecure random number generator with a cryptographically secure pseudo-random number generator (CSPRNG) using the `uuid` crate. Ensures telemetry installation IDs are unpredictable and unique.
Expand Down
25 changes: 19 additions & 6 deletions crates/agnix-lsp/src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ pub struct Backend {
/// Canonicalized workspace root cached at initialize() to avoid blocking I/O on hot paths.
pub(crate) workspace_root_canonical: Arc<RwLock<Option<PathBuf>>>,
pub(crate) documents: Arc<RwLock<HashMap<Url, Arc<String>>>>,
/// Tracks the latest document version from the client (did_open / did_change).
/// Used to tag published diagnostics with the version they were computed against.
pub(crate) document_versions: Arc<RwLock<HashMap<Url, i32>>>,
/// Monotonic generation incremented on each config change.
/// Used to drop stale diagnostics from older revalidation batches.
pub(crate) config_generation: Arc<AtomicU64>,
Expand All @@ -78,6 +81,7 @@ impl Backend {
workspace_root: Arc::new(RwLock::new(None)),
workspace_root_canonical: Arc::new(RwLock::new(None)),
documents: Arc::new(RwLock::new(HashMap::new())),
document_versions: Arc::new(RwLock::new(HashMap::new())),
config_generation: Arc::new(AtomicU64::new(0)),
project_validation_generation: Arc::new(AtomicU64::new(0)),
registry: Arc::new(agnix_core::ValidatorRegistry::with_defaults()),
Expand Down Expand Up @@ -188,19 +192,23 @@ impl Backend {
let config = self.config.load();
let file_type = agnix_core::resolve_file_type(&file_path, &config);
if file_type.is_generic() {
// Read version just-in-time to minimize TOCTOU window
let version = self.get_document_version(&uri).await;
// Publish empty diagnostics to clear any stale results
self.client.publish_diagnostics(uri, vec![], None).await;
self.client.publish_diagnostics(uri, vec![], version).await;
return;
}
}

// Get content from cache
let (content, expected_content) = {
// Get content from cache and capture version at same time to avoid TOCTOU
// between content validation and version publish
let (content, expected_content, captured_version) = {
let docs = self.documents.read().await;
match docs.get(&uri) {
Some(cached) => {
let snapshot = Arc::clone(cached);
(Arc::clone(&snapshot), Some(snapshot))
let version = self.get_document_version(&uri).await;
(Arc::clone(&snapshot), Some(snapshot), version)
}
None => {
// Fall back to file-based validation
Expand All @@ -212,8 +220,10 @@ impl Backend {
{
return;
}
// Read version just-in-time to minimize TOCTOU window
let version = self.get_document_version(&uri).await;
self.client
.publish_diagnostics(uri, diagnostics, None)
.publish_diagnostics(uri, diagnostics, version)
.await;
return;
}
Expand Down Expand Up @@ -259,8 +269,11 @@ impl Backend {
return;
}

// Use version captured at time of content snapshot to avoid publishing
// newer version with older (already-validated) diagnostics
let version = captured_version;
self.client
.publish_diagnostics(uri, diagnostics, None)
.publish_diagnostics(uri, diagnostics, version)
.await;
}
}
Comment on lines 275 to 279

Copilot AI Feb 20, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validate_from_content_and_publish reads the document version after should_publish_diagnostics returns. A concurrent did_change can happen between those awaits, causing stale diagnostics (computed from old content) to be published with the newer version, which defeats client-side stale-result suppression.

Consider capturing (content_arc, version) together under a single lock acquisition (or a combined struct/lock for content+version) and re-checking that the current content still matches before publishing, then publish using the version associated with that same snapshot.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 6872268. Now capturing version atomically with content snapshot (lines 210-211 in backend.rs). The captured_version is used at publish time, ensuring diagnostics are published with the version matching the content they were validated from, not a newer version from concurrent did_change events.

Expand Down
28 changes: 24 additions & 4 deletions crates/agnix-lsp/src/backend/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use super::*;

impl Backend {
pub(crate) async fn handle_did_open(&self, params: DidOpenTextDocumentParams) {
let version = params.text_document.version;
let uri = params.text_document.uri;
// Normalize CRLF so the cached content matches the LF-relative byte offsets
// produced by validate_content and used by code actions for fix ranges.
Expand All @@ -13,14 +14,21 @@ impl Backend {
std::borrow::Cow::Borrowed(_) => raw,
std::borrow::Cow::Owned(normalized) => normalized,
};
// Acquire both locks atomically to update content and version together.
// Readers that need both values must capture them in a single operation
// (see validate_from_content_and_publish).
{
let mut docs = self.documents.write().await;
let mut versions = self.document_versions.write().await;
docs.insert(uri.clone(), Arc::new(text));
versions.insert(uri.clone(), version);
// Both guards dropped here in reverse acquisition order (versions then docs)
}
self.validate_from_content_and_publish(uri, None).await;
}

pub(crate) async fn handle_did_change(&self, params: DidChangeTextDocumentParams) {
let version = params.text_document.version;
let uri = params.text_document.uri;
if let Some(change) = params.content_changes.into_iter().next() {
// Normalize CRLF so the cached content matches the LF-relative byte offsets
Expand All @@ -31,11 +39,21 @@ impl Backend {
std::borrow::Cow::Borrowed(_) => raw,
std::borrow::Cow::Owned(normalized) => normalized,
};
// Acquire both locks atomically to update content and version together.
// Readers that need both values must capture them in a single operation
// (see validate_from_content_and_publish).
{
let mut docs = self.documents.write().await;
let mut versions = self.document_versions.write().await;
docs.insert(uri.clone(), Arc::new(text));
versions.insert(uri.clone(), version);
// Both guards dropped here in reverse acquisition order (versions then docs)
}
self.validate_from_content_and_publish(uri, None).await;
} else {
// Even when content_changes is empty, the version from
// VersionedTextDocumentIdentifier is authoritative per LSP spec.
self.document_versions.write().await.insert(uri, version);
}
}

Expand All @@ -53,12 +71,14 @@ impl Backend {
}

pub(crate) async fn handle_did_close(&self, params: DidCloseTextDocumentParams) {
let uri = params.text_document.uri;
{
let mut docs = self.documents.write().await;
docs.remove(&params.text_document.uri);
docs.remove(&uri);
}
self.client
.publish_diagnostics(params.text_document.uri, vec![], None)
.await;
self.document_versions.write().await.remove(&uri);
// Clearing diagnostics for a closed document - version is intentionally None
// since the document is no longer tracked.
self.client.publish_diagnostics(uri, vec![], None).await;
}
}
7 changes: 7 additions & 0 deletions crates/agnix-lsp/src/backend/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,11 @@ impl Backend {
pub(crate) async fn get_document_content(&self, uri: &Url) -> Option<Arc<String>> {
self.documents.read().await.get(uri).cloned()
}

/// Get the latest document version reported by the client for a URI.
///
/// Returns `None` if the document has not been opened or has been closed.
pub(crate) async fn get_document_version(&self, uri: &Url) -> Option<i32> {
self.document_versions.read().await.get(uri).copied()
}
}
1 change: 1 addition & 0 deletions crates/agnix-lsp/src/backend/revalidation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ impl Backend {

// Publish diagnostics for files not open in the editor
for (uri, lsp_diags) in non_open_publish {
// None: non-open files have no client-tracked version
self.client.publish_diagnostics(uri, lsp_diags, None).await;
}

Expand Down
Loading