fix(ci): add #[ignore] to ra_e2e_suite and install rust-analyzer in e2e job (#126)

* fix(ci): add #[ignore] to ra_e2e_suite and install rust-analyzer in e2e CI job

Without #[ignore], ra_e2e_suite was not picked up by the CI e2e filter
(--run-ignored ignored-only). Also adds rustup component add rust-analyzer
step on Linux/macOS; Windows skips the ra_e2e suite via MCPLS_SKIP_RA=1.

* fix(ci): run rust-analyzer e2e tests on Windows too

* fix(e2e): resolve rust-analyzer via rustup fallback on Windows, extend code_action timeout

- ra_probe: fall back to `rustup which rust-analyzer` when the binary is not
  in PATH — fixes CI on Windows where rustup toolchain dirs are excluded from PATH
- sc_get_code_actions: increase retry loop from 15s to 30s and add 5s initial
  warm-up sleep; type-checking must complete before code action fixes appear
- compute ca_end_col dynamically from the actual line length instead of a
  hardcoded column

* fix(e2e): fix Windows file URI generation and code_action range

- path_to_uri: use Url::from_file_path instead of manual string formatting;
  on Windows, canonicalize() returns \\?\ prefix which manual replace produced
  invalid file:////?\... URIs — causing all LSP requests to fail with -32603
- sc_get_code_actions: extend range to ca_line+1 because rust-analyzer reports
  the missing-semicolon diagnostic at the closing brace (one line below), so the
  range must overlap that position to trigger the quickfix
- wait_until_ready: default timeout 120s on Windows (vs 60s on Linux/macOS)
- ci: bump test-e2e timeout-minutes from 15 to 20

* fix(bridge): strip \\?\ prefix in path_to_uri on Windows without breaking tests

Reverts to manual URI construction but strips the extended-path prefix
(\\?\) that canonicalize() adds on Windows.  Using Url::from_file_path
panicked on unit tests that pass Unix-style paths (/test/file.rs) which
are invalid Windows absolute paths.

* test(e2e): add diagnostic logging to wait_until_ready for Windows debugging

* fix(e2e): resolve get_code_actions returning empty actions

Add codeActionLiteralSupport to LSP client capabilities so rust-analyzer
returns CodeAction objects instead of falling back to legacy Command-only
format.  Without this capability declaration RA yields an empty response
for structural refactoring assists such as "Implement missing members".

Update sc_get_code_actions to use a point-cursor range on the impl line
and target the empty `impl Greet for CodeActionTarget` block.

Fix sc_get_cached_diagnostics to poll up to 15 s for the push-based
publishDiagnostics cache, which may arrive asynchronously after pull-based
diagnostics have already been satisfied by sc_get_diagnostics.

* refactor(lsp): use CodeActionKind constants for value_set instead of raw strings

* test(e2e): require 3 consecutive ready signals and extend Windows cached-diag timeout

Readiness gate now requires 3 consecutive hover successes to avoid false-positive
from transient RA analysis during workspace indexing (observed on Windows CI).

sc_get_cached_diagnostics timeout raised to 60 s on Windows where publishDiagnostics
notifications arrive later due to slower CI runners.

* fix(bridge): use path_to_uri for cached diagnostics lookup on Windows

Url::from_file_path on a canonicalized \?\-prefixed Windows path
produces a URI that does not match what rust-analyzer stores in
publishDiagnostics notifications (which omit the \?\ prefix).

Replace the Url::from_file_path call in handle_cached_diagnostics with
path_to_uri, which strips \?\ before building the URI, keeping the
cache lookup key consistent with the stored key.

* test(e2e): reduce cached-diagnostics timeout to 20 s on all platforms

The URI mismatch fix (path_to_uri instead of Url::from_file_path) means
the push cache lookup now works on Windows.  The previous 60 s Windows
timeout combined with the ~32 s readiness gate was hitting the 120 s
nextest terminate-after limit.

20 s is sufficient: if the cache is populated (URI fix works) the data
is found on the first poll; if not, we fail clearly within budget.

* test(e2e): use lib.rs for cached-diagnostics push verification

Newer rust-analyzer versions skip publishDiagnostics push for files that
were already served via the pull-based textDocument/diagnostic API.
sc_get_diagnostics calls pull on broken.rs, so RA no longer pushes for it.

lib.rs is opened only via hover (wait_until_ready), never via pull
diagnostics, so RA unconditionally pushes publishDiagnostics for it after
initial workspace indexing.  lib.rs also contains a guaranteed E0425 error
(undefined_variable in has_error) that RA always reports.

* fix(bridge): lowercase Windows drive letter in path_to_uri to match RA's URI normalization

rust-analyzer normalizes Windows drive letters to lowercase in file URIs
(e.g. C: → c:). path_to_uri was preserving the uppercase letter from
canonicalize(), causing push-diagnostic cache lookups to miss every time
on Windows CI — the stored key (RA's URI) and the lookup key (our URI)
differed only in drive-letter case.

* fix(bridge): normalize URI cache keys to lowercase on Windows

Different tools (rust-analyzer, std::fs::canonicalize) produce Windows
file URIs with inconsistent drive-letter casing (C: vs c:). Instead of
assuming one canonical form, normalize both sides to lowercase at the
cache boundary via uri_cache_key(). Applies to store, get, and clear
operations in NotificationCache.

Author: bug-ops

.github/workflows/ci.yml

@@ -203,7 +203,7 @@ jobs:
     needs: [detect-changes, fmt, clippy, build-tests, build]
     if: needs.detect-changes.outputs.run-full-ci == 'true'
     runs-on: ${{ matrix.os }}
-    timeout-minutes: 15
+    timeout-minutes: 20
     strategy:
       fail-fast: false
       matrix:
@@ -225,6 +225,8 @@ jobs:
       - name: Make binary executable
         if: matrix.os != 'windows-latest'
         run: chmod +x target/debug/mcpls
+      - name: Install rust-analyzer
+        run: rustup component add rust-analyzer
       - name: Run e2e tests
         run: cargo nextest run --archive-file nextest-archive.tar.zst --workspace-remap . --run-ignored ignored-only -E 'kind(test) and test(e2e)'
 

crates/mcpls-core/src/bridge/notifications.rs

@@ -11,6 +11,21 @@ use serde::{Deserialize, Serialize};
 /// Maximum number of log entries to store.
 const MAX_LOG_ENTRIES: usize = 100;
 
+/// Normalize a URI string to a stable cache key.
+///
+/// On Windows, URI comparisons must be case-insensitive: the filesystem is
+/// case-insensitive and different tools (e.g. rust-analyzer vs std) may
+/// produce drive letters in different cases (`C:` vs `c:`).
+/// Lowercasing the entire URI is safe for `file://` URIs because they have
+/// no case-sensitive query or fragment components.
+fn uri_cache_key(uri: &str) -> std::borrow::Cow<'_, str> {
+    if cfg!(windows) {
+        std::borrow::Cow::Owned(uri.to_ascii_lowercase())
+    } else {
+        std::borrow::Cow::Borrowed(uri)
+    }
+}
+
 /// Maximum number of server messages to store.
 const MAX_SERVER_MESSAGES: usize = 50;
 
@@ -141,7 +156,8 @@ impl NotificationCache {
             version,
             diagnostics,
         };
-        self.diagnostics.insert(uri.to_string(), info);
+        self.diagnostics
+            .insert(uri_cache_key(uri.as_str()).into_owned(), info);
     }
 
     /// Store a log entry.
@@ -180,7 +196,7 @@ impl NotificationCache {
     #[inline]
     #[must_use]
     pub fn get_diagnostics(&self, uri: &str) -> Option<&DiagnosticInfo> {
-        self.diagnostics.get(uri)
+        self.diagnostics.get(uri_cache_key(uri).as_ref())
     }
 
     /// Get all stored log entries.
@@ -201,7 +217,7 @@ impl NotificationCache {
     ///
     /// Returns the cleared diagnostics if they existed.
     pub fn clear_diagnostics(&mut self, uri: &str) -> Option<DiagnosticInfo> {
-        self.diagnostics.remove(uri)
+        self.diagnostics.remove(uri_cache_key(uri).as_ref())
     }
 
     /// Clear all diagnostics.

crates/mcpls-core/src/bridge/state.rs

@@ -212,13 +212,15 @@ impl DocumentTracker {
 /// not occur for valid absolute paths.
 #[must_use]
 pub fn path_to_uri(path: &Path) -> Uri {
-    // Convert path to file:// URI string and parse
     let uri_string = if cfg!(windows) {
-        format!("file:///{}", path.display().to_string().replace('\\', "/"))
+        let path_str = path.to_string_lossy();
+        // canonicalize() on Windows adds a \\?\ extended-path prefix.
+        // Strip it before building the URI — file:////?\C:/ is not valid.
+        let stripped = path_str.strip_prefix(r"\\?\").unwrap_or(&path_str);
+        format!("file:///{}", stripped.replace('\\', "/"))
     } else {
         format!("file://{}", path.display())
     };
-    // Path-to-URI conversion should always succeed for valid paths
     #[allow(clippy::expect_used)]
     uri_string.parse().expect("failed to create URI from path")
 }

crates/mcpls-core/src/bridge/translator.rs

@@ -17,9 +17,8 @@ use lsp_types::{
 };
 use serde::{Deserialize, Serialize};
 use tokio::time::Duration;
-use url::Url;
 
-use super::state::{ResourceLimits, detect_language};
+use super::state::{ResourceLimits, detect_language, path_to_uri};
 use super::{DocumentTracker, NotificationCache};
 use crate::bridge::encoding::mcp_to_lsp_position;
 use crate::error::{Error, Result};
@@ -1212,7 +1211,7 @@ impl Translator {
             context: lsp_types::CodeActionContext {
                 diagnostics: context_diagnostics,
                 only,
-                trigger_kind: None,
+                trigger_kind: Some(lsp_types::CodeActionTriggerKind::INVOKED),
             },
             work_done_progress_params: WorkDoneProgressParams::default(),
             partial_result_params: PartialResultParams::default(),
@@ -1222,7 +1221,6 @@ impl Translator {
         let response: Option<lsp_types::CodeActionResponse> = client
             .request("textDocument/codeAction", params, timeout_duration)
             .await?;
-
         let response_vec = response.unwrap_or_default();
         let mut actions = Vec::with_capacity(response_vec.len());
 
@@ -1418,10 +1416,9 @@ impl Translator {
         let path = PathBuf::from(file_path);
         let validated_path = self.validate_path(&path)?;
 
-        // Convert path to URI format for cache lookup (cross-platform)
-        let uri = Url::from_file_path(&validated_path)
-            .map_err(|()| Error::InvalidUri(validated_path.display().to_string()))?
-            .to_string();
+        // Use path_to_uri (strips \\?\ on Windows) so the key matches what
+        // rust-analyzer stores in publishDiagnostics notifications.
+        let uri = path_to_uri(&validated_path).to_string();
 
         let diagnostics =
             self.notification_cache

crates/mcpls-core/src/lsp/lifecycle.rs

@@ -290,7 +290,9 @@ impl LspServer {
                     Error::InvalidUri(format!("Invalid UTF-8 in path: {root_display}"))
                 })?;
                 let uri_str = if cfg!(windows) {
-                    format!("file:///{}", path_str.replace('\\', "/"))
+                    // Strip \\?\ extended-path prefix that canonicalize() adds on Windows.
+                    let stripped = path_str.strip_prefix(r"\\?\").unwrap_or(path_str);
+                    format!("file:///{}", stripped.replace('\\', "/"))
                 } else {
                     format!("file://{path_str}")
                 };
@@ -343,6 +345,25 @@ impl LspServer {
                         resolve_support: Some(lsp_types::CodeActionCapabilityResolveSupport {
                             properties: vec!["edit".to_string()],
                         }),
+                        // Declare supported action kinds so the server returns
+                        // CodeAction objects (not just legacy Command objects).
+                        code_action_literal_support: Some(lsp_types::CodeActionLiteralSupport {
+                            code_action_kind: lsp_types::CodeActionKindLiteralSupport {
+                                value_set: [
+                                    lsp_types::CodeActionKind::EMPTY,
+                                    lsp_types::CodeActionKind::QUICKFIX,
+                                    lsp_types::CodeActionKind::REFACTOR,
+                                    lsp_types::CodeActionKind::REFACTOR_EXTRACT,
+                                    lsp_types::CodeActionKind::REFACTOR_INLINE,
+                                    lsp_types::CodeActionKind::REFACTOR_REWRITE,
+                                    lsp_types::CodeActionKind::SOURCE,
+                                    lsp_types::CodeActionKind::SOURCE_ORGANIZE_IMPORTS,
+                                ]
+                                .iter()
+                                .map(|k| k.as_str().to_string())
+                                .collect(),
+                            },
+                        }),
                         ..Default::default()
                     }),
                     ..Default::default()

crates/mcpls-core/tests/common/ra_probe.rs

@@ -29,17 +29,30 @@ pub fn resolve_rust_analyzer() -> Resolution {
         return Resolution::Found(PathBuf::from(p));
     }
 
-    // Probe PATH by attempting to run rust-analyzer --version.
-    match std::process::Command::new("rust-analyzer")
+    // Probe PATH: if rust-analyzer --version succeeds, resolve the full path.
+    if std::process::Command::new("rust-analyzer")
         .arg("--version")
         .output()
+        .is_ok_and(|o| o.status.success())
+        && let Some(p) = find_in_path("rust-analyzer")
     {
-        Ok(out) if out.status.success() => {
-            // Resolve full path via `which`-equivalent: find the binary in PATH.
-            find_in_path("rust-analyzer").map_or(Resolution::Missing, Resolution::Found)
-        }
-        _ => Resolution::Missing,
+        return Resolution::Found(p);
+    }
+
+    // Fallback: ask rustup where it installed the component.
+    // This covers Windows CI where rustup toolchain bin dirs are not in PATH.
+    if let Some(path) = std::process::Command::new("rustup")
+        .args(["which", "rust-analyzer"])
+        .output()
+        .ok()
+        .filter(|o| o.status.success())
+        .map(|o| String::from_utf8_lossy(&o.stdout).trim().to_string())
+        .filter(|p| !p.is_empty())
+    {
+        return Resolution::Found(PathBuf::from(path));
     }
+
+    Resolution::Missing
 }
 
 /// Find a binary in PATH, returning its absolute path.

crates/mcpls-core/tests/fixtures/rust_workspace/src/lib.rs

@@ -71,11 +71,26 @@ impl fmt::Display for Point {
     }
 }
 
-/// Target function for code-action e2e testing.
-///
-/// The assignment `let ca_var = 1` is missing a semicolon so that
-/// rust-analyzer reliably offers an "add semicolon" quickfix here.
+/// Previously used for code-action quickfix testing (missing semicolon).
+/// The sub-case now uses structural actions instead; semicolon fixed.
 #[allow(dead_code)]
 pub fn code_action_target() {
-    let ca_var = 1
+    let _ca_var = 1;
+}
+
+/// Trait used as a code-action trigger.
+///
+/// `CodeActionTarget` below has an empty `impl Greet` — rust-analyzer
+/// reliably offers "Implement missing members" there, a context-free
+/// structural action that does not depend on diagnostic data.
+pub trait Greet {
+    fn hello(&self) -> String;
+}
+
+/// Struct with an empty trait impl for code-action testing.
+#[allow(dead_code)]
+pub struct CodeActionTarget;
+
+// split to multi-line so RA can offer "implement missing members" inside the block
+impl Greet for CodeActionTarget {
 }