feat(lsp): track document versions and include in publish_diagnostics (#544)

* feat(lsp): track document versions and include in publish_diagnostics (#478)

Add a document_versions map to the LSP Backend that tracks the version
number reported by the client on did_open and did_change events. The
tracked version is passed to publish_diagnostics instead of None,
allowing editors to correctly associate diagnostics with the document
revision they were computed against.

- Add document_versions field to Backend struct
- Store version on did_open and did_change, remove on did_close
- Add get_document_version helper method
- Replace None with tracked version in validate_from_content_and_publish
- Keep None for did_close (clearing diagnostics for closed documents)
- Add 5 unit tests and 1 integration test for version lifecycle

* fix(lsp): address review findings for document version tracking (#478)

* fix(lsp): atomize version+content updates, fix version tracking on empty changes (#478)

- Hold both documents and document_versions write locks simultaneously
  in handle_did_open and handle_did_change so readers never see a state
  where content is updated but version is not (or vice versa).
- Always update document version on did_change regardless of whether
  content_changes is empty, aligning with LSP spec where
  VersionedTextDocumentIdentifier.version is authoritative.
- Restore get_document_version to pub(crate) visibility and remove
  duplicate integration test (unit tests in backend/tests.rs cover
  direct version access).
- Fix stale comment in integration test to accurately describe the
  test's approach.

* fix(lsp): correct misleading comment about lock drop order

* docs: add changelog entry for LSP document version tracking (#478)

* fix(lsp): apply rustfmt formatting to LSP backend code

Fixes formatting issues detected in CI Format Check.

* fix(lsp): address race condition and misleading comments in version tracking

Addresses three Copilot review comments:

1. Fixed TOCTOU race in validate_from_content_and_publish: capture version
   at the same time as content snapshot to ensure diagnostics are published
   with the version matching the content they were validated against, not a
   newer version from concurrent did_change events.

2. Corrected misleading comments in handle_did_open/did_change about lock
   guarantees. The comments suggested locks prevent readers from seeing
   inconsistency, but that's only true if readers atomically capture both
   values together (which they now do).

3. Clarified integration test comment to explain why it doesn't assert
   version field in publish_diagnostics (requires consuming socket messages).
   Referenced unit tests in backend/tests.rs that verify version lifecycle.

Author: avifenesh

CHANGELOG.md

@@ -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.

crates/agnix-lsp/src/backend.rs

@@ -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>,
@@ -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()),
@@ -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
@@ -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;
                 }
@@ -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;
     }
 }

crates/agnix-lsp/src/backend/events.rs

@@ -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.
@@ -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
@@ -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);
         }
     }
 
@@ -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;
     }
 }

crates/agnix-lsp/src/backend/helpers.rs

@@ -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()
+    }
 }

crates/agnix-lsp/src/backend/revalidation.rs

@@ -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;
         }