From 05a1032e00c3efd5bcdeb506ee77895b8594808b Mon Sep 17 00:00:00 2001 From: Hunter Bown Date: Tue, 28 Apr 2026 00:29:28 -0500 Subject: [PATCH] feat(lsp): #136 post-edit diagnostics injection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Inject LSP diagnostics as a synthetic user message after every successful file edit (`edit_file`, `apply_patch`, `write_file`) so the agent sees compile breaks before its next reasoning step. Largest agent-quality lever in v0.7.0. Pieces: - `crates/tui/src/lsp/`: thin JSON-RPC stdio client (no `tower-lsp`), per-language registry, diagnostics renderer producing the `` block format. `LspManager` owns lazily spawned per-language transports keyed by `Language`. - `core/engine.rs`: hook on the success branch of the tool-result loop derives the edited file path(s) per tool, queries the LspManager with a 5 s timeout, and collects rendered blocks into `pending_lsp_blocks`. The queue is flushed as a `text` content block on the next request iteration so the model sees the diagnostics before it streams its next turn. - `[lsp]` config schema (`enabled`, `poll_after_edit_ms`, `max_diagnostics_per_file`, `include_warnings`, optional `servers` override) with built-in defaults for rust-analyzer, gopls, pyright, typescript-language-server, and clangd. - Failure modes are non-blocking by design: a missing LSP binary logs a one-time warning and skips the hook; a crashed server or poll timeout simply drops that turn's diagnostics. The agent's work is never blocked. Tests: 24 unit tests cover language detection, registry overrides, filter/sort/truncate behavior, and the rendered block format. Three engine-level tokio tests exercise the full path through a fake transport (no real LSP server is ever spawned in CI). Acceptance criteria (per #136): - Edit introducing a type error -> next request body contains `` block at the right line/col. - `[lsp] enabled = false` -> no diagnostics injected. - Snapshot test exercises full path with mock transport. - LSP binary not on PATH -> one-time warning, agent proceeds. - 5 s timeout, errors-only by default. - Transports spawn lazily on first edit per language. Co-Authored-By: Claude Opus 4.7 (1M context) --- config.example.toml | 29 ++ crates/config/src/lib.rs | 21 ++ crates/tui/src/config.rs | 50 +++ crates/tui/src/core/engine.rs | 145 ++++++++ crates/tui/src/core/engine/tests.rs | 161 +++++++++ crates/tui/src/lsp/client.rs | 472 ++++++++++++++++++++++++ crates/tui/src/lsp/diagnostics.rs | 216 +++++++++++ crates/tui/src/lsp/mod.rs | 535 ++++++++++++++++++++++++++++ crates/tui/src/lsp/registry.rs | 146 ++++++++ crates/tui/src/main.rs | 7 + crates/tui/src/runtime_threads.rs | 6 + crates/tui/src/tui/ui.rs | 4 + 12 files changed, 1792 insertions(+) create mode 100644 crates/tui/src/lsp/client.rs create mode 100644 crates/tui/src/lsp/diagnostics.rs create mode 100644 crates/tui/src/lsp/mod.rs create mode 100644 crates/tui/src/lsp/registry.rs diff --git a/config.example.toml b/config.example.toml index a38f73cf..bfb86e5c 100644 --- a/config.example.toml +++ b/config.example.toml @@ -213,6 +213,35 @@ default_text_model = "deepseek-ai/deepseek-v4-pro" # threshold_secs = 30 # include_summary = false +# ───────────────────────────────────────────────────────────────────────────────── +# LSP Diagnostics (post-edit) (#136) +# ───────────────────────────────────────────────────────────────────────────────── +# After every successful file edit (`edit_file`, `apply_patch`, `write_file`), +# the engine asks an LSP server for diagnostics on the file and injects them +# as a synthetic system message before the next API call. This lets the agent +# see compile breaks immediately without round-tripping through the user. +# +# Enabled by default. Failure modes are non-blocking: a missing LSP binary, +# a crashed server, or a timeout simply skips the post-edit hook for that +# turn — the agent's work is never blocked. +# +# Built-in language → server defaults: +# rust → rust-analyzer +# go → gopls serve +# python → pyright-langserver --stdio +# typescript → typescript-language-server --stdio +# c, cpp → clangd +# +# Override the defaults via the `servers` table below. +[lsp] +# enabled = true +# poll_after_edit_ms = 5000 +# max_diagnostics_per_file = 20 +# include_warnings = false +# [lsp.servers] +# rust = ["rust-analyzer"] +# go = ["gopls", "serve"] + # ───────────────────────────────────────────────────────────────────────────────── # Hooks (optional) # ───────────────────────────────────────────────────────────────────────────────── diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index 9e7b1884..346d4eab 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -128,6 +128,10 @@ pub struct ConfigToml { /// to a permissive default that mirrors pre-v0.7.0 behavior. #[serde(default)] pub network: Option, + /// Post-edit LSP diagnostics injection (#136). When absent, the engine + /// applies the defaults documented in [`LspConfigToml`]. + #[serde(default)] + pub lsp: Option, #[serde(flatten)] pub extras: BTreeMap, } @@ -171,6 +175,23 @@ impl Default for NetworkPolicyToml { } } +/// On-disk schema for the `[lsp]` table (#136). See `config.example.toml` +/// for documentation. All fields are optional so the TUI runtime can fall +/// back to its own defaults when keys are absent. +#[derive(Debug, Clone, Serialize, Deserialize, Default)] +pub struct LspConfigToml { + /// Master switch. + pub enabled: Option, + /// Maximum time to wait for diagnostics after an edit, in milliseconds. + pub poll_after_edit_ms: Option, + /// Cap on diagnostics surfaced per file. + pub max_diagnostics_per_file: Option, + /// When `true`, warnings (severity 2) are surfaced in addition to errors. + pub include_warnings: Option, + /// Optional override for the `language -> [cmd, ...args]` table. + pub servers: Option>>, +} + impl ConfigToml { #[must_use] pub fn get_value(&self, key: &str) -> Option { diff --git a/crates/tui/src/config.rs b/crates/tui/src/config.rs index 7b0a7275..b335daa8 100644 --- a/crates/tui/src/config.rs +++ b/crates/tui/src/config.rs @@ -429,6 +429,11 @@ pub struct Config { /// to a permissive default that mirrors pre-v0.7.0 behavior. #[serde(default)] pub network: Option, + + /// Post-edit LSP diagnostics injection (#136). When absent, the engine + /// applies the defaults documented in [`LspConfigToml`]. + #[serde(default)] + pub lsp: Option, } /// `[network]` table — mirrors `deepseek_config::NetworkPolicyToml` so the live @@ -486,6 +491,50 @@ impl NetworkPolicyToml { } } +/// `[lsp]` table — mirrors [`crate::lsp::LspConfig`]. Documented in +/// `config.example.toml`. When omitted, defaults from `LspConfig::default()` +/// apply (enabled, 5 s poll, 20 diagnostics/file, errors only, no overrides). +#[derive(Debug, Clone, Deserialize, Default)] +pub struct LspConfigToml { + /// Master switch. Defaults to `true`. + #[serde(default)] + pub enabled: Option, + /// How long to wait for the LSP server to publish diagnostics after a + /// `didOpen`/`didChange`. Defaults to 5000 ms. + #[serde(default)] + pub poll_after_edit_ms: Option, + /// Cap on diagnostics surfaced per file. Defaults to 20. + #[serde(default)] + pub max_diagnostics_per_file: Option, + /// Whether to surface warnings in addition to errors. Defaults to `false`. + #[serde(default)] + pub include_warnings: Option, + /// Optional override for the `Language -> [cmd, ...args]` table. Keys + /// are language slugs (`"rust"`, `"go"`, etc.). + #[serde(default)] + pub servers: Option>>, +} + +impl LspConfigToml { + /// Build a runtime [`crate::lsp::LspConfig`] from the on-disk schema, + /// falling back to defaults for any unset fields. + #[must_use] + pub fn into_runtime(self) -> crate::lsp::LspConfig { + let defaults = crate::lsp::LspConfig::default(); + crate::lsp::LspConfig { + enabled: self.enabled.unwrap_or(defaults.enabled), + poll_after_edit_ms: self + .poll_after_edit_ms + .unwrap_or(defaults.poll_after_edit_ms), + max_diagnostics_per_file: self + .max_diagnostics_per_file + .unwrap_or(defaults.max_diagnostics_per_file), + include_warnings: self.include_warnings.unwrap_or(defaults.include_warnings), + servers: self.servers.unwrap_or_default(), + } + } +} + #[derive(Debug, Clone, Default, Deserialize)] pub struct ProviderConfig { pub api_key: Option, @@ -1384,6 +1433,7 @@ fn merge_config(base: Config, override_cfg: Config) -> Config { features: merge_features(base.features, override_cfg.features), notifications: override_cfg.notifications.or(base.notifications), network: override_cfg.network.or(base.network), + lsp: override_cfg.lsp.or(base.lsp), } } diff --git a/crates/tui/src/core/engine.rs b/crates/tui/src/core/engine.rs index 6175a467..6a90a4c7 100644 --- a/crates/tui/src/core/engine.rs +++ b/crates/tui/src/core/engine.rs @@ -110,6 +110,9 @@ pub struct EngineConfig { /// session-scoped approvals (`/network allow `) persist for the /// remainder of the run. pub network_policy: Option, + /// Post-edit LSP diagnostics injection (#136). When `None`, the engine + /// constructs a disabled manager so the field is always present. + pub lsp_config: Option, } impl Default for EngineConfig { @@ -131,6 +134,7 @@ impl Default for EngineConfig { plan_state: new_shared_plan_state(), max_spawn_depth: crate::tools::subagent::DEFAULT_MAX_SPAWN_DEPTH, network_policy: None, + lsp_config: None, } } } @@ -260,6 +264,13 @@ pub struct Engine { capacity_controller: CapacityController, coherence_state: CoherenceState, turn_counter: u64, + /// Post-edit LSP diagnostics injection (#136). Populated unconditionally + /// — when LSP is disabled in config, this is an inert manager that + /// always returns `None` from `diagnostics_for`. + lsp_manager: Arc, + /// Diagnostics collected during the current step's tool calls. Drained + /// and forwarded as a synthetic user message before the next API call. + pending_lsp_blocks: Vec, } // === Internal stream helpers === @@ -835,6 +846,67 @@ fn caller_type_for_tool_use(caller: Option<&ToolCaller>) -> &str { caller.map_or("direct", |c| c.caller_type.as_str()) } +/// #136: derive the file path(s) edited by a tool call. Returns the empty +/// vec for tools that don't modify files. We intentionally only handle the +/// three known edit tools — adding more (e.g. specialized refactor tools) +/// is a one-line change here. +fn edited_paths_for_tool(tool_name: &str, input: &serde_json::Value) -> Vec { + match tool_name { + "edit_file" | "write_file" => { + if let Some(path) = input.get("path").and_then(|v| v.as_str()) { + vec![PathBuf::from(path)] + } else { + Vec::new() + } + } + "apply_patch" => { + // `apply_patch` accepts either a `path` override or a list of + // `files` (each `{path, content}`). We try both shapes. + let mut out = Vec::new(); + if let Some(path) = input.get("path").and_then(|v| v.as_str()) { + out.push(PathBuf::from(path)); + } + if let Some(files) = input.get("files").and_then(|v| v.as_array()) { + for entry in files { + if let Some(path) = entry.get("path").and_then(|v| v.as_str()) { + out.push(PathBuf::from(path)); + } + } + } + // Fallback: parse `---`/`+++` headers from a unified diff payload. + if out.is_empty() + && let Some(patch) = input.get("patch").and_then(|v| v.as_str()) + { + out.extend(parse_patch_paths(patch)); + } + out + } + _ => Vec::new(), + } +} + +/// Lightweight parser for `+++ b/` lines in a unified diff. Used as a +/// fallback when `apply_patch` is invoked with raw `patch` text and no +/// `path`/`files` override. We deliberately keep this dumb — the real +/// `apply_patch` tool already validates the patch shape; we only need a +/// best-effort hint for the LSP hook. +fn parse_patch_paths(patch: &str) -> Vec { + let mut out = Vec::new(); + for line in patch.lines() { + if let Some(rest) = line.strip_prefix("+++ ") { + let trimmed = rest.trim(); + // Strip leading `b/` per git diff conventions. + let path = trimmed.strip_prefix("b/").unwrap_or(trimmed); + // Skip `/dev/null` (deletion). + if path == "/dev/null" { + continue; + } + out.push(PathBuf::from(path)); + } + } + out +} + fn caller_allowed_for_tool(caller: Option<&ToolCaller>, tool_def: Option<&Tool>) -> bool { let requested = caller_type_for_tool_use(caller); if let Some(def) = tool_def @@ -1179,6 +1251,11 @@ impl Engine { let shell_manager = new_shared_shell_manager(config.workspace.clone()); let capacity_controller = CapacityController::new(config.capacity.clone()); + let lsp_manager = Arc::new(match config.lsp_config.clone() { + Some(cfg) => crate::lsp::LspManager::new(cfg, config.workspace.clone()), + None => crate::lsp::LspManager::disabled(), + }); + let mut engine = Engine { config, deepseek_client, @@ -1198,6 +1275,8 @@ impl Engine { capacity_controller, coherence_state: CoherenceState::default(), turn_counter: 0, + lsp_manager, + pending_lsp_blocks: Vec::new(), }; engine.rehydrate_latest_canonical_state(); @@ -1413,6 +1492,57 @@ impl Engine { self.emit_session_updated().await; } + /// #136: post-edit hook. Inspects the tool name + input, derives the + /// edited file path, and asks the LSP manager for diagnostics. The + /// rendered block is queued in `pending_lsp_blocks` and flushed to the + /// session message stream just before the next API request. Failure is + /// silent by design — a missing/crashing LSP server must never block + /// the agent. + async fn run_post_edit_lsp_hook(&mut self, tool_name: &str, tool_input: &serde_json::Value) { + if !self.lsp_manager.config().enabled { + return; + } + let paths = edited_paths_for_tool(tool_name, tool_input); + for path in paths { + let absolute = if path.is_absolute() { + path.clone() + } else { + self.session.workspace.join(&path) + }; + // Use a short edit-sequence based on the existing turn counter so + // log output stays correlated even though we do not currently + // batch by sequence. + let seq = self.turn_counter; + if let Some(block) = self.lsp_manager.diagnostics_for(&absolute, seq).await { + self.pending_lsp_blocks.push(block); + } + } + } + + /// Drain `pending_lsp_blocks` into a single synthetic user message so the + /// model sees the diagnostics on its next request. Skips when nothing is + /// pending. The message uses the standard `text` content block shape + /// (the same shape as the post-tool steer messages) so we don't need to + /// invent a new envelope. + async fn flush_pending_lsp_diagnostics(&mut self) { + if self.pending_lsp_blocks.is_empty() { + return; + } + let blocks = std::mem::take(&mut self.pending_lsp_blocks); + let rendered = crate::lsp::render_blocks(&blocks); + if rendered.is_empty() { + return; + } + self.add_session_message(Message { + role: "user".to_string(), + content: vec![ContentBlock::Text { + text: rendered, + cache_control: None, + }], + }) + .await; + } + /// Handle a send message operation #[allow(clippy::too_many_arguments)] async fn handle_send_message( @@ -2424,6 +2554,11 @@ impl Engine { } } + // #136: drain any LSP diagnostics collected since the last + // request and inject them as a synthetic user message so the + // model sees compile errors before its next reasoning step. + self.flush_pending_lsp_diagnostics().await; + // Build the request let force_update_plan_this_step = force_update_plan_first && turn.tool_calls.is_empty(); let active_tools = if tool_catalog.is_empty() { @@ -3613,6 +3748,16 @@ impl Engine { Some(&output_for_context), &self.session.workspace, ); + + // #136: post-edit LSP diagnostics hook. We only run + // this on success — failed edits leave the file + // untouched, so polling for diagnostics would just + // surface stale state. + if output.success { + self.run_post_edit_lsp_hook(&outcome.name, &tool_input) + .await; + } + self.add_session_message(Message { role: "user".to_string(), content: vec![ContentBlock::ToolResult { diff --git a/crates/tui/src/core/engine/tests.rs b/crates/tui/src/core/engine/tests.rs index 43cd628b..d4194473 100644 --- a/crates/tui/src/core/engine/tests.rs +++ b/crates/tui/src/core/engine/tests.rs @@ -1118,3 +1118,164 @@ fn capacity_escalation_skips_pure_transient_categories() { }); assert!(only_transient); } + +// ── #136: post-edit LSP diagnostics hook ───────────────────────────────── + +#[test] +fn edited_paths_for_edit_file_returns_path() { + let input = json!({ "path": "src/foo.rs", "search": "x", "replace": "y" }); + let paths = edited_paths_for_tool("edit_file", &input); + assert_eq!(paths, vec![PathBuf::from("src/foo.rs")]); +} + +#[test] +fn edited_paths_for_write_file_returns_path() { + let input = json!({ "path": "src/bar.rs", "content": "fn main() {}" }); + let paths = edited_paths_for_tool("write_file", &input); + assert_eq!(paths, vec![PathBuf::from("src/bar.rs")]); +} + +#[test] +fn edited_paths_for_apply_patch_with_files_returns_each_path() { + let input = json!({ + "files": [ + { "path": "a.rs", "content": "" }, + { "path": "b.rs", "content": "" } + ] + }); + let paths = edited_paths_for_tool("apply_patch", &input); + assert_eq!(paths, vec![PathBuf::from("a.rs"), PathBuf::from("b.rs")]); +} + +#[test] +fn edited_paths_for_apply_patch_with_diff_text_extracts_paths() { + let input = json!({ + "patch": "--- a/foo.rs\n+++ b/foo.rs\n@@ -1 +1 @@\n-let x: i32 = 0;\n+let x: i32 = \"oops\";\n" + }); + let paths = edited_paths_for_tool("apply_patch", &input); + assert_eq!(paths, vec![PathBuf::from("foo.rs")]); +} + +#[test] +fn edited_paths_for_unknown_tool_returns_empty() { + let input = json!({ "path": "irrelevant.rs" }); + let paths = edited_paths_for_tool("read_file", &input); + assert!(paths.is_empty()); + let paths = edited_paths_for_tool("grep_files", &input); + assert!(paths.is_empty()); +} + +#[test] +fn parse_patch_paths_skips_dev_null() { + let patch = "--- a/keep.rs\n+++ b/keep.rs\n--- a/deleted.rs\n+++ /dev/null\n"; + let paths = parse_patch_paths(patch); + assert_eq!(paths, vec![PathBuf::from("keep.rs")]); +} + +#[tokio::test] +async fn post_edit_hook_injects_diagnostics_message_before_next_request() { + use crate::lsp::{Diagnostic, Language, Severity}; + use std::sync::Arc; + + let tmp = tempdir().expect("tempdir"); + let workspace = tmp.path().to_path_buf(); + let target = workspace.join("src").join("main.rs"); + fs::create_dir_all(workspace.join("src")).unwrap(); + fs::write(&target, "let x: i32 = \"not a number\";").unwrap(); + + let lsp_config = crate::lsp::LspConfig::default(); + let engine_config = EngineConfig { + workspace: workspace.clone(), + lsp_config: Some(lsp_config), + ..Default::default() + }; + let (mut engine, _handle) = Engine::new(engine_config, &Config::default()); + + // Install a fake transport that always reports a type error. + let fake = Arc::new(crate::lsp::tests::FakeTransport::new(vec![Diagnostic { + line: 1, + column: 14, + severity: Severity::Error, + message: "expected i32, found &str".to_string(), + }])); + engine + .lsp_manager + .install_test_transport(Language::Rust, fake) + .await; + + // Simulate the success path of an edit_file tool call. + let input = json!({ "path": "src/main.rs", "search": "0", "replace": "\"not a number\"" }); + engine.run_post_edit_lsp_hook("edit_file", &input).await; + assert_eq!(engine.pending_lsp_blocks.len(), 1); + + // Flush prepares the synthetic message. + let messages_before = engine.session.messages.len(); + engine.flush_pending_lsp_diagnostics().await; + assert_eq!(engine.session.messages.len(), messages_before + 1); + + let last = engine.session.messages.last().expect("message appended"); + assert_eq!(last.role, "user"); + let text = match &last.content[0] { + crate::models::ContentBlock::Text { text, .. } => text.clone(), + other => panic!("expected text block, got {other:?}"), + }; + assert!(text.contains(" Result>; + + /// Best-effort shutdown. Called via `LspManager::shutdown_all`. + #[allow(dead_code)] + async fn shutdown(&self); +} + +/// Stdio-backed transport. Spawns the LSP server as a child process and +/// pipes JSON-RPC over stdin/stdout. Stderr is captured into a buffer so +/// callers can include it in error messages without polluting our own stderr. +pub struct StdioLspTransport { + /// JoinHandle for the running server. Held so the child stays alive for + /// the transport's lifetime; consumed during `shutdown`. + #[allow(dead_code)] + child: AsyncMutex>, + /// Outgoing message sender to the writer task. + tx_outbound: mpsc::Sender>, + /// Inbound diagnostics queue. We push every `publishDiagnostics` + /// notification into here and the public API drains the relevant entries. + diagnostics_rx: AsyncMutex)>>, + /// Map of in-flight request id -> reply slot. We do not currently call + /// methods that need replies after `initialize`, but this is the hook + /// for it. + #[allow(dead_code)] + pending: Arc>>>, + /// Monotonic request id counter. Reserved for future LSP request/reply + /// methods (workspace symbol queries, etc.). + #[allow(dead_code)] + next_id: AsyncMutex, + /// Language id passed in `textDocument/didOpen` (e.g. "rust"). + language_id: &'static str, + /// Track which files we have opened so the second touch sends + /// `didChange` instead of `didOpen`. + opened: AsyncMutex>, +} + +impl StdioLspTransport { + /// Spawn `command args…` and run the LSP `initialize` handshake. Returns + /// `Err` immediately if the binary is not on PATH or `initialize` fails. + pub async fn spawn( + command: &str, + args: &[String], + language: Language, + workspace: PathBuf, + ) -> Result { + let mut cmd = Command::new(command); + cmd.args(args); + cmd.stdin(Stdio::piped()); + cmd.stdout(Stdio::piped()); + cmd.stderr(Stdio::piped()); + cmd.kill_on_drop(true); + + let mut child = cmd + .spawn() + .with_context(|| format!("failed to spawn LSP server `{command}`"))?; + + let stdin = child + .stdin + .take() + .context("LSP child has no stdin handle")?; + let stdout = child + .stdout + .take() + .context("LSP child has no stdout handle")?; + + let (tx_outbound, rx_outbound) = mpsc::channel::>(64); + let (tx_inbound, rx_inbound) = mpsc::channel::(64); + let (tx_diag, rx_diag) = mpsc::channel::<(PathBuf, Vec)>(64); + + // Writer task: drain outbound channel, frame with Content-Length, write to stdin. + tokio::spawn(writer_task(stdin, rx_outbound)); + // Reader task: parse Content-Length frames from stdout, push to inbound queue. + tokio::spawn(reader_task(stdout, tx_inbound)); + // Inbound dispatcher: routes notifications to `tx_diag`, replies to a + // pending map. We keep the pending map for completeness even though + // diagnostics polling itself does not reuse it. + let pending: Arc>>> = + Arc::new(AsyncMutex::new(HashMap::new())); + tokio::spawn(dispatcher_task(rx_inbound, tx_diag, pending.clone())); + + // Send `initialize` and wait for `initialized`. We synthesize id=1. + let init_payload = json!({ + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "processId": std::process::id(), + "rootUri": uri_from_path(&workspace), + "capabilities": { + "textDocument": { + "publishDiagnostics": { "relatedInformation": false } + } + }, + "workspaceFolders": [{ + "uri": uri_from_path(&workspace), + "name": "workspace" + }] + } + }); + send_message(&tx_outbound, &init_payload).await?; + + // We do not actually wait for the initialize response here in MVP — + // most servers buffer notifications until they are ready, and waiting + // for `initialize` reply doubles the latency of the first edit. Send + // `initialized` immediately and let publishDiagnostics arrive on its + // own clock. + let initialized = json!({ + "jsonrpc": "2.0", + "method": "initialized", + "params": {} + }); + send_message(&tx_outbound, &initialized).await?; + + Ok(Self { + child: AsyncMutex::new(Some(child)), + tx_outbound, + diagnostics_rx: AsyncMutex::new(rx_diag), + pending, + next_id: AsyncMutex::new(2), + language_id: language.language_id(), + opened: AsyncMutex::new(HashMap::new()), + }) + } +} + +#[async_trait] +impl LspTransport for StdioLspTransport { + async fn diagnostics_for( + &self, + path: &Path, + text: &str, + wait: Duration, + ) -> Result> { + let path_buf = path.to_path_buf(); + let uri = uri_from_path(&path_buf); + + // Either send didOpen (first time) or didChange (subsequent edits). + let mut opened = self.opened.lock().await; + let is_new = !opened.contains_key(&path_buf); + let new_version = opened.get(&path_buf).copied().unwrap_or(0) + 1; + opened.insert(path_buf.clone(), new_version); + drop(opened); + + let payload = if is_new { + json!({ + "jsonrpc": "2.0", + "method": "textDocument/didOpen", + "params": { + "textDocument": { + "uri": uri.clone(), + "languageId": self.language_id, + "version": new_version, + "text": text + } + } + }) + } else { + json!({ + "jsonrpc": "2.0", + "method": "textDocument/didChange", + "params": { + "textDocument": { + "uri": uri.clone(), + "version": new_version + }, + "contentChanges": [{ "text": text }] + } + }) + }; + send_message(&self.tx_outbound, &payload).await?; + + // Drain matching `publishDiagnostics` notifications until `wait` + // elapses. Servers typically publish within a few hundred ms; for + // initial cold-start (rust-analyzer) it can be many seconds — but + // the manager guards us with a separate timeout. + let deadline = tokio::time::Instant::now() + wait; + let mut latest: Option> = None; + + loop { + let now = tokio::time::Instant::now(); + if now >= deadline { + break; + } + let remaining = deadline - now; + let mut rx = self.diagnostics_rx.lock().await; + let next = match timeout(remaining, rx.recv()).await { + Ok(Some(item)) => item, + Ok(None) => break, // channel closed + Err(_) => break, // timed out + }; + drop(rx); + let (file, items) = next; + if file == path_buf { + latest = Some(items); + // We have a payload — return immediately. If the server + // re-publishes after rapid edits, the next call will sync. + break; + } + // Otherwise: notification was for a different file we previously + // opened. Discard and continue waiting. + } + Ok(latest.unwrap_or_default()) + } + + async fn shutdown(&self) { + let mut child = self.child.lock().await; + if let Some(mut c) = child.take() { + let _ = c.start_kill(); + let _ = c.wait().await; + } + } +} + +/// Send a JSON value as one Content-Length-framed JSON-RPC message. +async fn send_message(tx: &mpsc::Sender>, value: &Value) -> Result<()> { + let body = serde_json::to_vec(value).context("serialize LSP message")?; + let header = format!("Content-Length: {}\r\n\r\n", body.len()); + let mut frame = Vec::with_capacity(header.len() + body.len()); + frame.extend_from_slice(header.as_bytes()); + frame.extend_from_slice(&body); + tx.send(frame) + .await + .map_err(|_| anyhow!("LSP outbound channel closed"))?; + Ok(()) +} + +/// Background task that drains the outbound queue and writes each frame to +/// the LSP server's stdin. Exits cleanly when the channel closes. +async fn writer_task(mut stdin: tokio::process::ChildStdin, mut rx: mpsc::Receiver>) { + while let Some(frame) = rx.recv().await { + if stdin.write_all(&frame).await.is_err() { + break; + } + if stdin.flush().await.is_err() { + break; + } + } +} + +/// Background task that parses `Content-Length`-framed JSON-RPC frames from +/// the LSP server's stdout. Pushes each parsed JSON value to `tx`. Exits +/// when stdout closes or a frame is malformed (we choose to fail closed +/// rather than risk hanging). +async fn reader_task(mut stdout: tokio::process::ChildStdout, tx: mpsc::Sender) { + let mut buf: Vec = Vec::with_capacity(8 * 1024); + let mut tmp = [0u8; 4096]; + loop { + let n = match stdout.read(&mut tmp).await { + Ok(0) => return, + Ok(n) => n, + Err(_) => return, + }; + buf.extend_from_slice(&tmp[..n]); + // Try to parse as many frames as we can from the accumulated buffer. + while let Some((header_end, content_length)) = parse_header(&buf) { + if buf.len() < header_end + content_length { + break; // need more bytes + } + let body = &buf[header_end..header_end + content_length]; + let parsed = serde_json::from_slice::(body).ok(); + // Drop the consumed bytes regardless of parse result so a bad frame + // does not stall the loop. + buf.drain(..header_end + content_length); + if let Some(value) = parsed + && tx.send(value).await.is_err() + { + return; + } + } + } +} + +/// Parse a JSON-RPC header block. Returns `Some((header_end, content_length))` +/// where `header_end` is the byte offset of the first body byte. The header +/// terminator is `\r\n\r\n`. We require a `Content-Length` header. +fn parse_header(buf: &[u8]) -> Option<(usize, usize)> { + let term = b"\r\n\r\n"; + let pos = buf.windows(term.len()).position(|window| window == term)?; + let header = std::str::from_utf8(&buf[..pos]).ok()?; + let mut content_length: Option = None; + for line in header.split("\r\n") { + if let Some(rest) = line.strip_prefix("Content-Length:") { + content_length = rest.trim().parse::().ok(); + } + } + content_length.map(|cl| (pos + term.len(), cl)) +} + +/// Background task that consumes inbound JSON values, classifies them as +/// notifications/responses, and routes accordingly. +async fn dispatcher_task( + mut rx: mpsc::Receiver, + tx_diag: mpsc::Sender<(PathBuf, Vec)>, + pending: Arc>>>, +) { + while let Some(value) = rx.recv().await { + // Notifications have a `method` and no `id`. + let method = value.get("method").and_then(|v| v.as_str()); + if method == Some("textDocument/publishDiagnostics") { + if let Some((path, diags)) = parse_publish_diagnostics(&value) { + let _ = tx_diag.send((path, diags)).await; + } + continue; + } + // Replies have an `id` and a `result` or `error`. + if let Some(id) = value.get("id").and_then(|v| v.as_i64()) { + let mut map = pending.lock().await; + if let Some(slot) = map.remove(&id) { + let _ = slot.send(value); + } + } + } +} + +/// Decode a `textDocument/publishDiagnostics` notification. +fn parse_publish_diagnostics(value: &Value) -> Option<(PathBuf, Vec)> { + let params = value.get("params")?; + let uri = params.get("uri")?.as_str()?; + let path = path_from_uri(uri)?; + let raw = params.get("diagnostics")?.as_array()?; + let mut out = Vec::with_capacity(raw.len()); + for d in raw { + let range = d.get("range")?; + let start = range.get("start")?; + let line = start.get("line")?.as_u64()? as u32 + 1; + let column = start.get("character")?.as_u64()? as u32 + 1; + let severity = Severity::from_lsp(d.get("severity").and_then(|v| v.as_i64())) + .unwrap_or(Severity::Error); + let message = d + .get("message") + .and_then(|v| v.as_str()) + .unwrap_or("") + .to_string(); + out.push(Diagnostic { + line, + column, + severity, + message, + }); + } + Some((path, out)) +} + +/// Convert a filesystem path to a `file://` URI. Best-effort — we do not +/// support Windows drive letters perfectly, but the LSP servers in our +/// registry accept percent-encoded paths well enough for the post-edit +/// diagnostics use case. +fn uri_from_path(path: &Path) -> String { + let canonical = path.canonicalize().unwrap_or_else(|_| path.to_path_buf()); + let s = canonical.to_string_lossy(); + if s.starts_with('/') { + format!("file://{s}") + } else { + format!("file:///{}", s.trim_start_matches('/')) + } +} + +/// Inverse of [`uri_from_path`]. Returns `None` when the URI is not a `file://`. +fn path_from_uri(uri: &str) -> Option { + let stripped = uri.strip_prefix("file://")?; + Some(PathBuf::from(stripped)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parses_lsp_header() { + let frame = b"Content-Length: 5\r\n\r\nhello"; + let (end, len) = parse_header(frame).expect("header parses"); + assert_eq!(end, 21); + assert_eq!(len, 5); + } + + #[test] + fn parse_header_returns_none_when_truncated() { + let frame = b"Content-Length: 5\r\nMissingTerm"; + assert!(parse_header(frame).is_none()); + } + + #[test] + fn parses_publish_diagnostics_payload() { + let payload = json!({ + "jsonrpc": "2.0", + "method": "textDocument/publishDiagnostics", + "params": { + "uri": "file:///tmp/foo.rs", + "diagnostics": [ + { + "range": { + "start": { "line": 11, "character": 7 }, + "end": { "line": 11, "character": 8 } + }, + "severity": 1, + "message": "missing semicolon" + } + ] + } + }); + let (path, diags) = parse_publish_diagnostics(&payload).expect("parses"); + assert_eq!(path, PathBuf::from("/tmp/foo.rs")); + assert_eq!(diags.len(), 1); + assert_eq!(diags[0].line, 12); + assert_eq!(diags[0].column, 8); + assert_eq!(diags[0].severity, Severity::Error); + assert_eq!(diags[0].message, "missing semicolon"); + } + + #[test] + fn round_trips_uri_path() { + let path = PathBuf::from("/tmp/example/foo.rs"); + let uri = format!("file://{}", path.display()); + assert_eq!(path_from_uri(&uri), Some(path)); + } +} diff --git a/crates/tui/src/lsp/diagnostics.rs b/crates/tui/src/lsp/diagnostics.rs new file mode 100644 index 00000000..f45f81d3 --- /dev/null +++ b/crates/tui/src/lsp/diagnostics.rs @@ -0,0 +1,216 @@ +//! Diagnostic shape returned by the LSP transport, plus the renderer that +//! produces the `` block injected into the model +//! context after a file edit. +//! +//! Format (matches the spec given in issue #136): +//! +//! ```text +//! +//! ERROR [12:8] missing semicolon +//! ERROR [13:1] expected `,`, found `}` +//! +//! ``` +//! +//! Lines are 1-based. Columns are 1-based. We trim each diagnostic message +//! to a single line so the block stays compact. + +use std::path::PathBuf; + +/// Severity bucket used in the rendered block. Mirrors the LSP severity +/// codes (1 = Error, 2 = Warning, 3 = Information, 4 = Hint). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Severity { + Error, + Warning, + Information, + Hint, +} + +impl Severity { + /// Decode the LSP integer severity. Returns `None` when the integer is + /// missing or unrecognized — callers default to `Error` to err on the + /// side of surfacing the issue. + #[must_use] + pub fn from_lsp(code: Option) -> Option { + match code? { + 1 => Some(Severity::Error), + 2 => Some(Severity::Warning), + 3 => Some(Severity::Information), + 4 => Some(Severity::Hint), + _ => None, + } + } + + /// Uppercase label used in the rendered block. + #[must_use] + pub fn label(self) -> &'static str { + match self { + Severity::Error => "ERROR", + Severity::Warning => "WARNING", + Severity::Information => "INFO", + Severity::Hint => "HINT", + } + } +} + +/// One LSP diagnostic, normalized to 1-based line/col so we can render it +/// directly. The transport layer is responsible for the `0-based -> 1-based` +/// conversion. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Diagnostic { + pub line: u32, + pub column: u32, + pub severity: Severity, + pub message: String, +} + +impl Diagnostic { + /// Trim the message to a single line for compact rendering. + fn render_message(&self) -> String { + let first_line = self.message.lines().next().unwrap_or("").trim(); + first_line.to_string() + } +} + +/// One file's worth of diagnostics, ready to render. The renderer caps the +/// list to `max_per_file` items. +#[derive(Debug, Clone)] +pub struct DiagnosticBlock { + /// Path used inside the `file="…"` attribute. Should be relative to the + /// workspace root when possible (we use `path.file_name()` if relativizing + /// fails, per the issue's hard rule). + pub file: PathBuf, + pub items: Vec, +} + +impl DiagnosticBlock { + /// Render the block in the format pasted in the module docs. Returns the + /// empty string when `self.items` is empty so callers can `if !text.is_empty()` + /// before injecting. + #[must_use] + pub fn render(&self) -> String { + if self.items.is_empty() { + return String::new(); + } + let file_attr = self.file.display(); + let mut out = format!("\n"); + for item in &self.items { + out.push_str(&format!( + " {} [{}:{}] {}\n", + item.severity.label(), + item.line, + item.column, + item.render_message(), + )); + } + out.push_str(""); + out + } + + /// Truncate to at most `max_per_file` items, preserving order. The LSP + /// manager is responsible for sorting by severity before calling this so + /// errors are kept ahead of warnings when truncation happens. + pub fn truncate(&mut self, max_per_file: usize) { + if self.items.len() > max_per_file { + self.items.truncate(max_per_file); + } + } +} + +/// Format a list of [`DiagnosticBlock`]s as a single bundle. Used by the +/// engine when one turn touched several files. Empty blocks are skipped. +#[must_use] +pub fn render_blocks(blocks: &[DiagnosticBlock]) -> String { + let mut chunks = Vec::new(); + for block in blocks { + let rendered = block.render(); + if !rendered.is_empty() { + chunks.push(rendered); + } + } + chunks.join("\n") +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn severity_decodes_lsp_codes() { + assert_eq!(Severity::from_lsp(Some(1)), Some(Severity::Error)); + assert_eq!(Severity::from_lsp(Some(2)), Some(Severity::Warning)); + assert_eq!(Severity::from_lsp(Some(3)), Some(Severity::Information)); + assert_eq!(Severity::from_lsp(Some(4)), Some(Severity::Hint)); + assert_eq!(Severity::from_lsp(Some(99)), None); + assert_eq!(Severity::from_lsp(None), None); + } + + #[test] + fn renders_block_in_required_format() { + let block = DiagnosticBlock { + file: PathBuf::from("crates/tui/src/foo.rs"), + items: vec![ + Diagnostic { + line: 12, + column: 8, + severity: Severity::Error, + message: "missing semicolon".to_string(), + }, + Diagnostic { + line: 13, + column: 1, + severity: Severity::Error, + message: "expected `,`, found `}`".to_string(), + }, + ], + }; + let rendered = block.render(); + assert!(rendered.contains("")); + assert!(rendered.contains("ERROR [12:8] missing semicolon")); + assert!(rendered.contains("ERROR [13:1] expected `,`, found `}`")); + assert!(rendered.ends_with("")); + } + + #[test] + fn empty_block_renders_to_empty_string() { + let block = DiagnosticBlock { + file: PathBuf::from("foo.rs"), + items: Vec::new(), + }; + assert!(block.render().is_empty()); + } + + #[test] + fn truncate_caps_to_max() { + let mut block = DiagnosticBlock { + file: PathBuf::from("foo.rs"), + items: (0..30) + .map(|i| Diagnostic { + line: i, + column: 1, + severity: Severity::Error, + message: format!("err {i}"), + }) + .collect(), + }; + block.truncate(20); + assert_eq!(block.items.len(), 20); + } + + #[test] + fn renders_only_first_line_of_message() { + let block = DiagnosticBlock { + file: PathBuf::from("foo.rs"), + items: vec![Diagnostic { + line: 1, + column: 1, + severity: Severity::Error, + message: "first line\nsecond line\nthird".to_string(), + }], + }; + let rendered = block.render(); + assert!(rendered.contains("first line")); + assert!(!rendered.contains("second line")); + assert!(!rendered.contains("third")); + } +} diff --git a/crates/tui/src/lsp/mod.rs b/crates/tui/src/lsp/mod.rs new file mode 100644 index 00000000..1519e4a0 --- /dev/null +++ b/crates/tui/src/lsp/mod.rs @@ -0,0 +1,535 @@ +//! LSP integration: post-edit diagnostics injection (#136). +//! +//! After the agent performs a successful file edit (`edit_file`, +//! `apply_patch`, or `write_file`) the engine asks the [`LspManager`] for +//! diagnostics on that file. The manager spawns the appropriate LSP server +//! lazily on first use, sends `didOpen`/`didChange`, waits up to a bounded +//! timeout for `publishDiagnostics`, normalizes the result, and returns it +//! to the engine. +//! +//! Failure modes are non-blocking by design: a missing LSP binary, a +//! crashed server, or a timeout all degrade to "no diagnostics this turn" +//! rather than stalling the agent. We log a one-time warning per language +//! when the binary is missing. +//! +//! # Wiring +//! +//! ```text +//! Engine ── after successful edit ──▶ LspManager.diagnostics_for(path, seq) +//! │ +//! ▼ +//! per-language LspClient +//! │ +//! ▼ +//! LspTransport (stdio) +//! ``` +//! +//! # Configuration +//! +//! The `[lsp]` table in `~/.deepseek/config.toml` controls behavior: +//! `enabled`, `poll_after_edit_ms`, `max_diagnostics_per_file`, +//! `include_warnings`, and an optional `servers` override. See +//! [`LspConfig`] for defaults and `config.example.toml` for documentation. + +use std::collections::{HashMap, HashSet}; +use std::path::{Path, PathBuf}; +use std::sync::Arc; +use std::time::Duration; + +use serde::Deserialize; +use tokio::sync::Mutex as AsyncMutex; +use tokio::time::timeout; + +pub mod client; +pub mod diagnostics; +pub mod registry; + +pub use client::{LspTransport, StdioLspTransport}; +pub use diagnostics::{Diagnostic, DiagnosticBlock, Severity, render_blocks}; +pub use registry::Language; + +/// `[lsp]` config schema. Mirrors the TOML keys documented in +/// `config.example.toml`. Unknown keys are ignored. +#[derive(Debug, Clone, Deserialize, PartialEq, Eq)] +#[serde(default)] +pub struct LspConfig { + /// Master switch. When `false`, the manager skips every operation and + /// returns an empty diagnostics list. + pub enabled: bool, + /// Maximum time in milliseconds to wait for the LSP server to publish + /// diagnostics after a `didOpen`/`didChange`. Default 5000 ms. + pub poll_after_edit_ms: u64, + /// Maximum diagnostics to keep per file. Excess items are dropped after + /// sorting by severity. Default 20. + pub max_diagnostics_per_file: usize, + /// When `true`, warnings (severity 2) are kept in the output. When + /// `false` (default), only errors (severity 1) are surfaced. + pub include_warnings: bool, + /// Optional override for the `Language -> (cmd, args)` table. Keys use + /// [`Language::as_key`] (e.g. `"rust"`). + pub servers: HashMap>, +} + +impl Default for LspConfig { + fn default() -> Self { + Self { + enabled: true, + poll_after_edit_ms: 5_000, + max_diagnostics_per_file: 20, + include_warnings: false, + servers: HashMap::new(), + } + } +} + +impl LspConfig { + /// Resolve `(command, args)` for `lang`. User-supplied overrides take + /// precedence over the built-in registry. + fn resolve_command(&self, lang: Language) -> Option<(String, Vec)> { + if let Some(parts) = self.servers.get(lang.as_key()) + && let Some((first, rest)) = parts.split_first() + { + return Some((first.clone(), rest.to_vec())); + } + let (cmd, args) = registry::server_for(lang)?; + Some(( + cmd.to_string(), + args.iter().map(|a| (*a).to_string()).collect(), + )) + } +} + +/// The LspManager holds a lazily populated map of `Language -> Transport`. +/// One transport is reused across files of the same language for the +/// session's lifetime. +pub struct LspManager { + config: LspConfig, + workspace: PathBuf, + /// Per-language transports. Wrapped in `Arc` so we can release the outer + /// lock before driving I/O on a single transport. + transports: AsyncMutex>>, + /// Per-language "we already warned the user that the binary is missing" + /// guard so we do not spam the audit log on every edit. + missing_warned: AsyncMutex>, + /// Test seam: when set, `diagnostics_for` uses these instead of spawning + /// real LSP processes. Keyed by language. + test_transports: AsyncMutex>>, +} + +impl LspManager { + /// Build a new manager. Does not spawn any LSP servers — that is lazy. + #[must_use] + pub fn new(config: LspConfig, workspace: PathBuf) -> Self { + Self { + config, + workspace, + transports: AsyncMutex::new(HashMap::new()), + missing_warned: AsyncMutex::new(HashSet::new()), + test_transports: AsyncMutex::new(HashMap::new()), + } + } + + /// Read-only access to the resolved config. Used by the engine to skip + /// the post-edit hook entirely when `enabled = false`. + #[must_use] + pub fn config(&self) -> &LspConfig { + &self.config + } + + /// Inject a fake transport for a language. Used by tests so we never + /// fork a real LSP server in CI. + #[cfg(test)] + pub async fn install_test_transport(&self, lang: Language, transport: Arc) { + self.test_transports.lock().await.insert(lang, transport); + } + + /// Poll the LSP server for diagnostics on `file`. Returns the rendered + /// [`DiagnosticBlock`] (already truncated to the configured per-file + /// max) or `None` when the manager is disabled / has no server / the + /// poll times out. + /// + /// The `_edit_seq` argument is currently a no-op; it exists in the + /// signature so the engine can correlate diagnostics back to a specific + /// edit when we add request batching in v0.7.x. + pub async fn diagnostics_for(&self, file: &Path, _edit_seq: u64) -> Option { + if !self.config.enabled { + return None; + } + let lang = registry::detect_language(file); + if lang == Language::Other { + return None; + } + + let text = match tokio::fs::read_to_string(file).await { + Ok(text) => text, + Err(err) => { + tracing::debug!(?err, file = %file.display(), "lsp: read file failed"); + return None; + } + }; + + let transport = match self.transport_for(lang).await { + Some(t) => t, + None => return None, + }; + + let wait = Duration::from_millis(self.config.poll_after_edit_ms); + let inner_wait = wait; + let raw = match timeout(wait, transport.diagnostics_for(file, &text, inner_wait)).await { + Ok(Ok(items)) => items, + Ok(Err(err)) => { + tracing::debug!(?err, file = %file.display(), "lsp: diagnostics call failed"); + return None; + } + Err(_) => { + tracing::debug!(file = %file.display(), "lsp: diagnostics timed out"); + return None; + } + }; + + // Filter, sort, and truncate. + let include_warnings = self.config.include_warnings; + let mut items: Vec = raw + .into_iter() + .filter(|d| match d.severity { + Severity::Error => true, + Severity::Warning => include_warnings, + _ => false, + }) + .collect(); + items.sort_by_key(|d| match d.severity { + Severity::Error => 0u8, + Severity::Warning => 1u8, + Severity::Information => 2u8, + Severity::Hint => 3u8, + }); + let mut block = DiagnosticBlock { + file: relative_to_workspace(&self.workspace, file), + items, + }; + block.truncate(self.config.max_diagnostics_per_file); + if block.items.is_empty() { + None + } else { + Some(block) + } + } + + /// Resolve (and lazily spawn) the transport for `lang`. Tests can + /// short-circuit this via `install_test_transport` (cfg-test only). + async fn transport_for(&self, lang: Language) -> Option> { + if let Some(t) = self.test_transports.lock().await.get(&lang) { + return Some(t.clone()); + } + + if let Some(t) = self.transports.lock().await.get(&lang) { + return Some(t.clone()); + } + + let (cmd, args) = self.config.resolve_command(lang)?; + match StdioLspTransport::spawn(&cmd, &args, lang, self.workspace.clone()).await { + Ok(transport) => { + let arc: Arc = Arc::new(transport); + self.transports.lock().await.insert(lang, arc.clone()); + Some(arc) + } + Err(err) => { + self.warn_missing_once(lang, &cmd, &err).await; + None + } + } + } + + async fn warn_missing_once(&self, lang: Language, cmd: &str, err: &anyhow::Error) { + let mut warned = self.missing_warned.lock().await; + if warned.insert(lang) { + tracing::warn!( + language = %lang.as_key(), + command = %cmd, + error = %err, + "lsp: server unavailable; diagnostics disabled for this language" + ); + } + } + + /// Best-effort shutdown of every spawned transport. Called when the + /// session ends. + #[allow(dead_code)] + pub async fn shutdown_all(&self) { + let transports: Vec> = + self.transports.lock().await.values().cloned().collect(); + for transport in transports { + transport.shutdown().await; + } + } +} + +/// Render `path` relative to the workspace when possible. Falls back to +/// `path.file_name()` (per the issue's hard rule about not using +/// `display().to_string()` on the bare path) when relativization fails. +fn relative_to_workspace(workspace: &Path, path: &Path) -> PathBuf { + if let Ok(rel) = path.strip_prefix(workspace) { + return rel.to_path_buf(); + } + PathBuf::from( + path.file_name() + .map(|n| n.to_string_lossy().into_owned()) + .unwrap_or_else(|| String::from("unknown")), + ) +} + +/// Used for tests / no-op runs. Builds an empty manager that always returns +/// `None`. Needed because the engine constructs an `LspManager` even when +/// the user has disabled LSP, so the field is always present. +impl LspManager { + #[must_use] + pub fn disabled() -> Self { + Self::new( + LspConfig { + enabled: false, + ..LspConfig::default() + }, + PathBuf::new(), + ) + } +} + +#[cfg(test)] +pub(crate) mod tests { + use super::*; + use async_trait::async_trait; + use std::sync::atomic::{AtomicUsize, Ordering}; + + /// Fake transport: returns a fixed list of diagnostics. Used by + /// integration tests so we never spawn a real LSP server in CI. + pub(crate) struct FakeTransport { + items: Vec, + calls: AtomicUsize, + } + + impl FakeTransport { + pub(crate) fn new(items: Vec) -> Self { + Self { + items, + calls: AtomicUsize::new(0), + } + } + + pub(crate) fn call_count(&self) -> usize { + self.calls.load(Ordering::Relaxed) + } + } + + #[async_trait] + impl LspTransport for FakeTransport { + async fn diagnostics_for( + &self, + _path: &Path, + _text: &str, + _wait: Duration, + ) -> anyhow::Result> { + self.calls.fetch_add(1, Ordering::Relaxed); + Ok(self.items.clone()) + } + + async fn shutdown(&self) {} + } + + #[tokio::test] + async fn returns_none_when_disabled() { + let mgr = LspManager::new( + LspConfig { + enabled: false, + ..LspConfig::default() + }, + PathBuf::from("/tmp"), + ); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("foo.rs"); + tokio::fs::write(&path, b"fn main() {}").await.unwrap(); + assert!(mgr.diagnostics_for(&path, 1).await.is_none()); + } + + #[tokio::test] + async fn returns_none_for_unknown_language() { + let dir = tempfile::tempdir().unwrap(); + let mgr = LspManager::new(LspConfig::default(), dir.path().to_path_buf()); + let path = dir.path().join("notes.txt"); + tokio::fs::write(&path, b"hi").await.unwrap(); + assert!(mgr.diagnostics_for(&path, 1).await.is_none()); + } + + #[tokio::test] + async fn forwards_errors_through_fake_transport() { + let dir = tempfile::tempdir().unwrap(); + let mgr = LspManager::new(LspConfig::default(), dir.path().to_path_buf()); + let path = dir.path().join("foo.rs"); + tokio::fs::write(&path, b"let x: i32 = \"oops\";") + .await + .unwrap(); + + let fake = Arc::new(FakeTransport::new(vec![Diagnostic { + line: 1, + column: 14, + severity: Severity::Error, + message: "expected i32, found &str".to_string(), + }])); + mgr.install_test_transport(Language::Rust, fake.clone()) + .await; + + let block = mgr.diagnostics_for(&path, 1).await.expect("has block"); + let rendered = block.render(); + assert!(rendered.contains("ERROR [1:14] expected i32, found &str")); + assert!(rendered.contains("foo.rs")); + assert_eq!(fake.call_count(), 1); + } + + #[tokio::test] + async fn drops_warnings_by_default() { + let dir = tempfile::tempdir().unwrap(); + let mgr = LspManager::new(LspConfig::default(), dir.path().to_path_buf()); + let path = dir.path().join("foo.rs"); + tokio::fs::write(&path, b"fn main() {}").await.unwrap(); + + let fake = Arc::new(FakeTransport::new(vec![ + Diagnostic { + line: 1, + column: 1, + severity: Severity::Warning, + message: "unused import".to_string(), + }, + Diagnostic { + line: 2, + column: 1, + severity: Severity::Error, + message: "type error".to_string(), + }, + ])); + mgr.install_test_transport(Language::Rust, fake).await; + + let block = mgr.diagnostics_for(&path, 1).await.expect("has block"); + assert_eq!(block.items.len(), 1); + assert_eq!(block.items[0].severity, Severity::Error); + } + + #[tokio::test] + async fn keeps_warnings_when_opted_in() { + let dir = tempfile::tempdir().unwrap(); + let mgr = LspManager::new( + LspConfig { + include_warnings: true, + ..LspConfig::default() + }, + dir.path().to_path_buf(), + ); + let path = dir.path().join("foo.rs"); + tokio::fs::write(&path, b"fn main() {}").await.unwrap(); + + let fake = Arc::new(FakeTransport::new(vec![ + Diagnostic { + line: 1, + column: 1, + severity: Severity::Warning, + message: "unused".to_string(), + }, + Diagnostic { + line: 2, + column: 1, + severity: Severity::Error, + message: "broken".to_string(), + }, + ])); + mgr.install_test_transport(Language::Rust, fake).await; + + let block = mgr.diagnostics_for(&path, 1).await.expect("has block"); + assert_eq!(block.items.len(), 2); + // Errors come first after sorting. + assert_eq!(block.items[0].severity, Severity::Error); + assert_eq!(block.items[1].severity, Severity::Warning); + } + + #[tokio::test] + async fn truncates_to_max_per_file() { + let dir = tempfile::tempdir().unwrap(); + let mgr = LspManager::new( + LspConfig { + max_diagnostics_per_file: 3, + ..LspConfig::default() + }, + dir.path().to_path_buf(), + ); + let path = dir.path().join("foo.rs"); + tokio::fs::write(&path, b"fn main() {}").await.unwrap(); + + let fake = Arc::new(FakeTransport::new( + (0..10) + .map(|i| Diagnostic { + line: i + 1, + column: 1, + severity: Severity::Error, + message: format!("err {i}"), + }) + .collect(), + )); + mgr.install_test_transport(Language::Rust, fake).await; + + let block = mgr.diagnostics_for(&path, 1).await.expect("has block"); + assert_eq!(block.items.len(), 3); + } + + #[tokio::test] + async fn render_blocks_concatenates() { + let blocks = vec![ + DiagnosticBlock { + file: PathBuf::from("a.rs"), + items: vec![Diagnostic { + line: 1, + column: 1, + severity: Severity::Error, + message: "err in a".to_string(), + }], + }, + DiagnosticBlock { + file: PathBuf::from("b.rs"), + items: vec![Diagnostic { + line: 2, + column: 2, + severity: Severity::Error, + message: "err in b".to_string(), + }], + }, + ]; + let rendered = render_blocks(&blocks); + assert!(rendered.contains("file=\"a.rs\"")); + assert!(rendered.contains("file=\"b.rs\"")); + } + + #[test] + fn relative_path_falls_back_to_filename_when_outside_workspace() { + let workspace = PathBuf::from("/foo/bar"); + let path = PathBuf::from("/baz/qux.rs"); + assert_eq!( + relative_to_workspace(&workspace, &path), + PathBuf::from("qux.rs") + ); + } + + #[test] + fn config_resolve_uses_overrides() { + let mut cfg = LspConfig::default(); + cfg.servers.insert( + "rust".to_string(), + vec!["custom-rls".to_string(), "--lsp".to_string()], + ); + let (cmd, args) = cfg.resolve_command(Language::Rust).unwrap(); + assert_eq!(cmd, "custom-rls"); + assert_eq!(args, vec!["--lsp".to_string()]); + } + + #[test] + fn config_resolve_falls_back_to_registry() { + let cfg = LspConfig::default(); + let (cmd, _) = cfg.resolve_command(Language::Rust).unwrap(); + assert_eq!(cmd, "rust-analyzer"); + } +} diff --git a/crates/tui/src/lsp/registry.rs b/crates/tui/src/lsp/registry.rs new file mode 100644 index 00000000..c90834b9 --- /dev/null +++ b/crates/tui/src/lsp/registry.rs @@ -0,0 +1,146 @@ +//! Language detection + the fixed dictionary mapping a language to the LSP +//! server binary that handles it. +//! +//! Kept intentionally small: a dozen languages, a hard-coded executable name +//! per language, an optional list of args. Users can override the defaults +//! via `[lsp.servers]` in `~/.deepseek/config.toml` (handled by +//! [`super::LspConfig`], not this file). + +use std::path::Path; + +/// A language we know how to ask an LSP server about. Detected from the file +/// extension by [`detect_language`]. `Other` is a sentinel used when we do +/// not have an LSP for the file — the LSP manager treats it as "skip". +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum Language { + Rust, + Go, + Python, + TypeScript, + JavaScript, + C, + Cpp, + Other, +} + +impl Language { + /// Stable lowercase string used as the key in `[lsp.servers]` overrides + /// and in log lines. + #[must_use] + pub fn as_key(self) -> &'static str { + match self { + Language::Rust => "rust", + Language::Go => "go", + Language::Python => "python", + Language::TypeScript => "typescript", + Language::JavaScript => "javascript", + Language::C => "c", + Language::Cpp => "cpp", + Language::Other => "other", + } + } + + /// LSP `languageId` value used in `textDocument/didOpen`. We follow the + /// LSP-spec values: `rust`, `go`, `python`, `typescript`, `javascript`, + /// `c`, `cpp`. + #[must_use] + pub fn language_id(self) -> &'static str { + match self { + Language::Rust => "rust", + Language::Go => "go", + Language::Python => "python", + Language::TypeScript => "typescript", + Language::JavaScript => "javascript", + Language::C => "c", + Language::Cpp => "cpp", + Language::Other => "plaintext", + } + } +} + +/// Detect the language of `path` from its extension. Falls back to +/// `Language::Other` when the extension is unknown (or the file has none), +/// which signals "skip" to the manager. +#[must_use] +pub fn detect_language(path: &Path) -> Language { + let ext = match path.extension().and_then(|e| e.to_str()) { + Some(ext) => ext.to_ascii_lowercase(), + None => return Language::Other, + }; + match ext.as_str() { + "rs" => Language::Rust, + "go" => Language::Go, + "py" | "pyi" => Language::Python, + "ts" | "tsx" => Language::TypeScript, + "js" | "jsx" | "mjs" | "cjs" => Language::JavaScript, + "c" | "h" => Language::C, + "cpp" | "cc" | "cxx" | "hpp" | "hxx" | "hh" => Language::Cpp, + _ => Language::Other, + } +} + +/// Fixed default for "what executable + args do we run for `lang`?". +/// Returns `None` when no LSP server is wired for that language. The TUI +/// config layer can override this dictionary at runtime. +#[must_use] +pub fn server_for(lang: Language) -> Option<(&'static str, &'static [&'static str])> { + match lang { + Language::Rust => Some(("rust-analyzer", &[])), + Language::Go => Some(("gopls", &["serve"])), + Language::Python => Some(("pyright-langserver", &["--stdio"])), + Language::TypeScript | Language::JavaScript => { + Some(("typescript-language-server", &["--stdio"])) + } + Language::C | Language::Cpp => Some(("clangd", &[])), + Language::Other => None, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::path::PathBuf; + + #[test] + fn detects_rust_extension() { + assert_eq!(detect_language(&PathBuf::from("foo.rs")), Language::Rust); + assert_eq!(detect_language(&PathBuf::from("FOO.RS")), Language::Rust); + } + + #[test] + fn detects_unknown_as_other() { + assert_eq!( + detect_language(&PathBuf::from("notes.txt")), + Language::Other + ); + assert_eq!(detect_language(&PathBuf::from("README")), Language::Other); + } + + #[test] + fn detects_typescript_variants() { + assert_eq!( + detect_language(&PathBuf::from("foo.ts")), + Language::TypeScript + ); + assert_eq!( + detect_language(&PathBuf::from("foo.tsx")), + Language::TypeScript + ); + assert_eq!( + detect_language(&PathBuf::from("foo.js")), + Language::JavaScript + ); + } + + #[test] + fn server_for_rust_is_rust_analyzer() { + let (cmd, args) = server_for(Language::Rust).expect("rust has a server"); + assert_eq!(cmd, "rust-analyzer"); + assert!(args.is_empty()); + } + + #[test] + fn server_for_other_is_none() { + assert!(server_for(Language::Other).is_none()); + } +} diff --git a/crates/tui/src/main.rs b/crates/tui/src/main.rs index 88316221..595d1748 100644 --- a/crates/tui/src/main.rs +++ b/crates/tui/src/main.rs @@ -29,6 +29,7 @@ mod features; mod hooks; mod llm_client; mod logging; +mod lsp; mod mcp; mod mcp_server; mod models; @@ -2933,6 +2934,11 @@ async fn run_exec_agent( crate::network_policy::NetworkPolicyDecider::with_default_audit(toml_cfg.into_runtime()) }); + let lsp_config = config + .lsp + .clone() + .map(crate::config::LspConfigToml::into_runtime); + let engine_config = EngineConfig { model: model.to_string(), workspace: workspace.clone(), @@ -2950,6 +2956,7 @@ async fn run_exec_agent( plan_state: new_shared_plan_state(), max_spawn_depth: crate::tools::subagent::DEFAULT_MAX_SPAWN_DEPTH, network_policy, + lsp_config, }; let engine_handle = spawn_engine(engine_config, config); diff --git a/crates/tui/src/runtime_threads.rs b/crates/tui/src/runtime_threads.rs index ed942108..739fb940 100644 --- a/crates/tui/src/runtime_threads.rs +++ b/crates/tui/src/runtime_threads.rs @@ -1371,6 +1371,11 @@ impl RuntimeThreadManager { let network_policy = self.config.network.clone().map(|toml_cfg| { crate::network_policy::NetworkPolicyDecider::with_default_audit(toml_cfg.into_runtime()) }); + let lsp_config = self + .config + .lsp + .clone() + .map(crate::config::LspConfigToml::into_runtime); let engine_cfg = EngineConfig { model: thread.model.clone(), workspace: thread.workspace.clone(), @@ -1390,6 +1395,7 @@ impl RuntimeThreadManager { plan_state: new_shared_plan_state(), max_spawn_depth: crate::tools::subagent::DEFAULT_MAX_SPAWN_DEPTH, network_policy, + lsp_config, }; let engine = spawn_engine(engine_cfg, &self.config); diff --git a/crates/tui/src/tui/ui.rs b/crates/tui/src/tui/ui.rs index 3e50b995..a5f48c6f 100644 --- a/crates/tui/src/tui/ui.rs +++ b/crates/tui/src/tui/ui.rs @@ -336,6 +336,10 @@ fn build_engine_config(app: &App, config: &Config) -> EngineConfig { network_policy: config.network.clone().map(|toml_cfg| { crate::network_policy::NetworkPolicyDecider::with_default_audit(toml_cfg.into_runtime()) }), + lsp_config: config + .lsp + .clone() + .map(crate::config::LspConfigToml::into_runtime), } }