From a930537207ce37ed7e5eb99dea565c294ec4bca4 Mon Sep 17 00:00:00 2001 From: Erik Simon Date: Thu, 9 Jul 2026 07:25:14 +0200 Subject: [PATCH] WIP --- Cargo.lock | 5 + crates/harness-app/src/lib.rs | 21 ++ crates/harness-core/src/engine/loop.rs | 1 + crates/harness-core/src/engine/processor.rs | 3 + crates/harness-core/src/lib.rs | 1 + crates/harness-core/src/lsp.rs | 46 +++ crates/harness-core/src/tool/mod.rs | 3 + crates/harness-lsp/Cargo.toml | 7 + crates/harness-lsp/src/client.rs | 325 ++++++++++++++++++++ crates/harness-lsp/src/lib.rs | 239 +++++++++++++- crates/harness-lsp/tests/rust_analyzer.rs | 34 ++ crates/harness-tools/src/bash.rs | 1 + crates/harness-tools/src/diagnostics.rs | 68 ++++ crates/harness-tools/src/edit/mod.rs | 4 +- crates/harness-tools/src/glob.rs | 1 + crates/harness-tools/src/grep.rs | 1 + crates/harness-tools/src/lib.rs | 1 + crates/harness-tools/src/read.rs | 1 + crates/harness-tools/src/write.rs | 2 + 19 files changed, 762 insertions(+), 2 deletions(-) create mode 100644 crates/harness-core/src/lsp.rs create mode 100644 crates/harness-lsp/src/client.rs create mode 100644 crates/harness-lsp/tests/rust_analyzer.rs create mode 100644 crates/harness-tools/src/diagnostics.rs diff --git a/Cargo.lock b/Cargo.lock index 84b9211..80dac23 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -711,7 +711,12 @@ dependencies = [ name = "harness-lsp" version = "0.1.0" dependencies = [ + "async-trait", "harness-core", + "serde_json", + "tempfile", + "tokio", + "tracing", ] [[package]] diff --git a/crates/harness-app/src/lib.rs b/crates/harness-app/src/lib.rs index b339749..c24a065 100644 --- a/crates/harness-app/src/lib.rs +++ b/crates/harness-app/src/lib.rs @@ -16,6 +16,7 @@ use harness_core::config::{self, Config, ConfigError}; use harness_core::engine::jobs::{JobBoard, JobState, LaunchSpec}; use harness_core::engine::{run_session, RunConfig, StepContext}; use harness_core::event::{AppEvent, EventBus, RunOutcome}; +use harness_core::lsp::DiagnosticsSource; use harness_core::permission::{PermissionReply, PermissionService, Rule, Ruleset}; use harness_core::store::{Store, StoreError}; use harness_core::tool::{ @@ -90,6 +91,9 @@ struct EngineInner { providers: ProviderRegistry, catalog: ModelCatalog, agents: AgentRegistry, + /// LSP diagnostics pool (edit/write surface errors). Shared across sessions; servers spawn + /// lazily on first touch. + diagnostics: Option>, cwd: PathBuf, data_dir: PathBuf, runs: Mutex>, @@ -196,6 +200,20 @@ impl EngineHandle { // `init` warms the cache in the background for the next launch. let catalog = ModelCatalog::load_cached_or_baked(&ModelCatalog::default_cache_path()); + // LSP pool: built-ins present on PATH, plus any config-declared servers. + let lsp_servers: Vec = config + .lsp + .iter() + .map(|(name, c)| harness_lsp::ServerConfig { + name: name.clone(), + command: c.command.clone(), + args: c.args.clone(), + extensions: c.extensions.clone(), + }) + .collect(); + let diagnostics: Option> = + Some(Arc::new(harness_lsp::LspPool::new(cwd.clone(), lsp_servers))); + let inner = Arc::new(EngineInner { config, store, @@ -205,6 +223,7 @@ impl EngineHandle { providers, catalog, agents, + diagnostics, cwd, data_dir, runs: Mutex::new(HashMap::new()), @@ -328,6 +347,7 @@ impl EngineHandle { spawner: Some(self.inner.clone()), job_board: Some(board), context_reporter: None, // root session has no parent board to report to + diagnostics: self.inner.diagnostics.clone(), }; let run_config = RunConfig { agent_name: agent.name.clone(), @@ -633,6 +653,7 @@ impl EngineInner { spawner: Some(self.arc()), job_board: Some(board), context_reporter: reporter, + diagnostics: self.diagnostics.clone(), } } } diff --git a/crates/harness-core/src/engine/loop.rs b/crates/harness-core/src/engine/loop.rs index 97580f7..84d4ebd 100644 --- a/crates/harness-core/src/engine/loop.rs +++ b/crates/harness-core/src/engine/loop.rs @@ -434,6 +434,7 @@ mod tests { spawner: None, job_board: None, context_reporter: None, + diagnostics: None, } } diff --git a/crates/harness-core/src/engine/processor.rs b/crates/harness-core/src/engine/processor.rs index c29636c..8d169cb 100644 --- a/crates/harness-core/src/engine/processor.rs +++ b/crates/harness-core/src/engine/processor.rs @@ -72,6 +72,8 @@ pub struct StepContext { /// Present in subagent sessions: reports read files to this session's job on the parent /// board. `None` for root sessions (nothing to report to). pub context_reporter: Option>, + /// Language-server diagnostics source shared by the session's edit/write tool calls. + pub diagnostics: Option>, } struct FlushTracker { @@ -522,6 +524,7 @@ impl<'a> Run<'a> { metadata: metadata_sink, spawner: self.ctx.spawner.clone(), context_reporter: self.ctx.context_reporter.clone(), + diagnostics: self.ctx.diagnostics.clone(), }; let result = tokio::select! { diff --git a/crates/harness-core/src/lib.rs b/crates/harness-core/src/lib.rs index 253f0b4..4792490 100644 --- a/crates/harness-core/src/lib.rs +++ b/crates/harness-core/src/lib.rs @@ -3,6 +3,7 @@ pub mod config; pub mod engine; pub mod event; pub mod llm; +pub mod lsp; pub mod permission; pub mod store; pub mod tool; diff --git a/crates/harness-core/src/lsp.rs b/crates/harness-core/src/lsp.rs new file mode 100644 index 0000000..0f85f0d --- /dev/null +++ b/crates/harness-core/src/lsp.rs @@ -0,0 +1,46 @@ +//! Seam trait for language-server diagnostics, implemented by `harness-lsp` and consumed by +//! the edit/write tools. Kept in core so `harness-tools` never links the LSP crate directly. +//! See `docs/09-integrations.md`. + +use std::path::Path; +use std::time::Duration; + +use async_trait::async_trait; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Severity { + Error, + Warning, + Info, + Hint, +} + +/// One diagnostic reported by a language server. Line/character are 1-based for display. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Diagnostic { + pub line: u32, + pub character: u32, + pub severity: Severity, + pub message: String, + pub source: Option, +} + +impl Diagnostic { + /// `{file}: L{line}: {message}` — the one-line form appended to tool output. + pub fn display_line(&self, file: &str) -> String { + format!("{file}: L{}: {}", self.line, self.message) + } +} + +/// A source of file diagnostics (an LSP pool). All methods are best-effort: failures are +/// swallowed (logged) so a broken language server never fails a tool call. +#[async_trait] +pub trait DiagnosticsSource: Send + Sync { + /// Ensure a server for `path`'s language is running and told about the file's current + /// contents (spawn-if-needed + didOpen/didChange). + async fn touch(&self, path: &Path); + + /// Diagnostics for `path`, waiting up to `wait` for the server to (re)publish after a + /// change. Returns whatever is known on timeout. + async fn diagnostics(&self, path: &Path, wait: Duration) -> Vec; +} diff --git a/crates/harness-core/src/tool/mod.rs b/crates/harness-core/src/tool/mod.rs index 3ee7e17..82f695d 100644 --- a/crates/harness-core/src/tool/mod.rs +++ b/crates/harness-core/src/tool/mod.rs @@ -195,6 +195,9 @@ pub struct ToolCtx { pub spawner: Option>, /// Present in subagent sessions: lets the read tool report files to the job board. pub context_reporter: Option>, + /// Language-server diagnostics source (edit/write surface errors after a change). `None` + /// disables LSP integration. + pub diagnostics: Option>, } #[derive(Debug)] diff --git a/crates/harness-lsp/Cargo.toml b/crates/harness-lsp/Cargo.toml index 812bfed..465421b 100644 --- a/crates/harness-lsp/Cargo.toml +++ b/crates/harness-lsp/Cargo.toml @@ -6,6 +6,13 @@ license.workspace = true [dependencies] harness-core = { workspace = true } +tokio = { workspace = true } +async-trait = { workspace = true } +serde_json = { workspace = true } +tracing = { workspace = true } + +[dev-dependencies] +tempfile = { workspace = true } [lints] workspace = true diff --git a/crates/harness-lsp/src/client.rs b/crates/harness-lsp/src/client.rs new file mode 100644 index 0000000..94616f0 --- /dev/null +++ b/crates/harness-lsp/src/client.rs @@ -0,0 +1,325 @@ +//! Minimal JSON-RPC client over an LSP server's child stdio: `Content-Length` framing, a +//! request-id → oneshot map, and a diagnostics store keyed by document URI. Only the handful +//! of methods the edit/write flow needs are implemented (docs/09-integrations.md). + +use std::collections::HashMap; +use std::path::Path; +use std::sync::atomic::{AtomicI64, Ordering}; +use std::sync::{Arc, Mutex}; +use std::time::Duration; + +use harness_core::lsp::{Diagnostic, Severity}; +use serde_json::{json, Value}; +use tokio::io::{AsyncReadExt, AsyncWriteExt, BufReader}; +use tokio::process::{Child, Command}; +use tokio::sync::{oneshot, Notify}; + +type Pending = Arc>>>; +type DiagStore = Arc>>>; + +/// A running language server plus the state needed to talk to it. +pub struct LspClient { + outgoing: tokio::sync::mpsc::UnboundedSender, + next_id: AtomicI64, + pending: Pending, + diagnostics: DiagStore, + diag_notify: Arc, + // Kept alive so the child is killed on drop (`kill_on_drop`). + _child: Child, +} + +impl LspClient { + /// Spawns `command args`, performs the `initialize`/`initialized` handshake rooted at + /// `root`, and returns a ready client. + pub async fn spawn(command: &str, args: &[String], root: &Path) -> std::io::Result { + let mut child = Command::new(command) + .args(args) + .current_dir(root) + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::null()) + .kill_on_drop(true) + .spawn()?; + + let stdin = child.stdin.take().expect("piped stdin"); + let stdout = child.stdout.take().expect("piped stdout"); + + let (outgoing, mut out_rx) = tokio::sync::mpsc::unbounded_channel::(); + let pending: Pending = Arc::default(); + let diagnostics: DiagStore = Arc::default(); + let diag_notify = Arc::new(Notify::new()); + + // Writer task: frame and forward outgoing payloads. + tokio::spawn(async move { + let mut stdin = stdin; + while let Some(payload) = out_rx.recv().await { + let frame = format!("Content-Length: {}\r\n\r\n{}", payload.len(), payload); + if stdin.write_all(frame.as_bytes()).await.is_err() { + break; + } + let _ = stdin.flush().await; + } + }); + + // Reader task: parse frames, route responses, collect diagnostics, ack server requests. + { + let pending = pending.clone(); + let diagnostics = diagnostics.clone(); + let diag_notify = diag_notify.clone(); + let outgoing_ack = outgoing.clone(); + tokio::spawn(async move { + let mut reader = BufReader::new(stdout); + while let Some(msg) = read_message(&mut reader).await { + dispatch(msg, &pending, &diagnostics, &diag_notify, &outgoing_ack); + } + }); + } + + let client = Self { + outgoing, + next_id: AtomicI64::new(1), + pending, + diagnostics, + diag_notify, + _child: child, + }; + + client.initialize(root).await?; + Ok(client) + } + + async fn initialize(&self, root: &Path) -> std::io::Result<()> { + let root_uri = path_to_uri(root); + let params = json!({ + "processId": std::process::id(), + "rootUri": root_uri, + "workspaceFolders": [{ "uri": root_uri, "name": "root" }], + "capabilities": { + "textDocument": { + "publishDiagnostics": { "relatedInformation": false } + } + }, + "clientInfo": { "name": "ai-harness" } + }); + // A slow server (rust-analyzer indexing) can take a while to answer initialize. + let _ = self + .request("initialize", params, Duration::from_secs(30)) + .await; + self.notify("initialized", json!({})); + Ok(()) + } + + fn send(&self, payload: Value) { + if let Ok(text) = serde_json::to_string(&payload) { + let _ = self.outgoing.send(text); + } + } + + pub fn notify(&self, method: &str, params: Value) { + self.send(json!({ "jsonrpc": "2.0", "method": method, "params": params })); + } + + async fn request(&self, method: &str, params: Value, timeout: Duration) -> Option { + let id = self.next_id.fetch_add(1, Ordering::Relaxed); + let (tx, rx) = oneshot::channel(); + self.pending.lock().unwrap().insert(id, tx); + self.send(json!({ "jsonrpc": "2.0", "id": id, "method": method, "params": params })); + match tokio::time::timeout(timeout, rx).await { + Ok(Ok(value)) => Some(value), + _ => { + self.pending.lock().unwrap().remove(&id); + None + } + } + } + + pub fn did_open(&self, path: &Path, language_id: &str, version: i32, text: &str) { + self.notify( + "textDocument/didOpen", + json!({ + "textDocument": { + "uri": path_to_uri(path), + "languageId": language_id, + "version": version, + "text": text, + } + }), + ); + } + + pub fn did_change(&self, path: &Path, version: i32, text: &str) { + self.notify( + "textDocument/didChange", + json!({ + "textDocument": { "uri": path_to_uri(path), "version": version }, + "contentChanges": [{ "text": text }], + }), + ); + } + + /// Clears the stored diagnostics for `path` so the next `wait_diagnostics` observes a fresh + /// publish rather than a stale one. + pub fn clear(&self, path: &Path) { + self.diagnostics.lock().unwrap().remove(&path_to_uri(path)); + } + + /// Waits up to `wait` for the server to publish diagnostics for `path`, returning whatever + /// is stored on timeout (possibly empty). + pub async fn wait_diagnostics(&self, path: &Path, wait: Duration) -> Vec { + let uri = path_to_uri(path); + let deadline = tokio::time::Instant::now() + wait; + loop { + if let Some(diags) = self.diagnostics.lock().unwrap().get(&uri) { + return diags.clone(); + } + let notified = self.diag_notify.notified(); + let remaining = deadline.saturating_duration_since(tokio::time::Instant::now()); + if remaining.is_zero() { + break; + } + if tokio::time::timeout(remaining, notified).await.is_err() { + break; + } + } + self.diagnostics + .lock() + .unwrap() + .get(&uri) + .cloned() + .unwrap_or_default() + } +} + +fn dispatch( + msg: Value, + pending: &Pending, + diagnostics: &DiagStore, + diag_notify: &Arc, + outgoing: &tokio::sync::mpsc::UnboundedSender, +) { + let method = msg.get("method").and_then(|m| m.as_str()); + let id = msg.get("id"); + + match (method, id) { + // Server → client request: ack with a null result so the server can proceed + // (e.g. client/registerCapability, window/workDoneProgress/create). + (Some(_), Some(id)) => { + let reply = json!({ "jsonrpc": "2.0", "id": id, "result": Value::Null }); + if let Ok(text) = serde_json::to_string(&reply) { + let _ = outgoing.send(text); + } + } + // Notification from the server. + (Some("textDocument/publishDiagnostics"), None) => { + if let Some(params) = msg.get("params") { + if let Some((uri, diags)) = parse_diagnostics(params) { + diagnostics.lock().unwrap().insert(uri, diags); + diag_notify.notify_waiters(); + } + } + } + (Some(_), None) => {} + // Response to one of our requests. + (None, Some(id)) => { + if let Some(id) = id.as_i64() { + if let Some(tx) = pending.lock().unwrap().remove(&id) { + let result = msg.get("result").cloned().unwrap_or(Value::Null); + let _ = tx.send(result); + } + } + } + (None, None) => {} + } +} + +fn parse_diagnostics(params: &Value) -> Option<(String, Vec)> { + let uri = params.get("uri")?.as_str()?.to_string(); + let items = params.get("diagnostics")?.as_array()?; + let diags = items + .iter() + .filter_map(|d| { + let start = d.get("range")?.get("start")?; + Some(Diagnostic { + line: start.get("line")?.as_u64().unwrap_or(0) as u32 + 1, + character: start.get("character")?.as_u64().unwrap_or(0) as u32 + 1, + severity: match d.get("severity").and_then(|s| s.as_u64()) { + Some(1) => Severity::Error, + Some(2) => Severity::Warning, + Some(3) => Severity::Info, + _ => Severity::Hint, + }, + message: d.get("message")?.as_str().unwrap_or("").to_string(), + source: d + .get("source") + .and_then(|s| s.as_str()) + .map(|s| s.to_string()), + }) + }) + .collect(); + Some((uri, diags)) +} + +/// Reads one `Content-Length`-framed JSON-RPC message, or `None` at EOF. +async fn read_message(reader: &mut R) -> Option { + use tokio::io::AsyncBufReadExt; + let mut content_length: Option = None; + loop { + let mut line = String::new(); + let n = reader.read_line(&mut line).await.ok()?; + if n == 0 { + return None; // EOF + } + let trimmed = line.trim_end(); + if trimmed.is_empty() { + break; // end of headers + } + if let Some(value) = trimmed.strip_prefix("Content-Length:") { + content_length = value.trim().parse().ok(); + } + } + let len = content_length?; + let mut buf = vec![0u8; len]; + reader.read_exact(&mut buf).await.ok()?; + serde_json::from_slice(&buf).ok() +} + +/// `file://` URI for an absolute path (best-effort; assumes UTF-8, no percent-encoding needed +/// for the local paths we handle). +fn path_to_uri(path: &Path) -> String { + let s = path.to_string_lossy(); + if s.starts_with('/') { + format!("file://{s}") + } else { + format!("file:///{s}") + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parses_publish_diagnostics_into_one_based_positions() { + let params = json!({ + "uri": "file:///tmp/a.rs", + "diagnostics": [{ + "range": {"start": {"line": 4, "character": 8}, "end": {"line": 4, "character": 12}}, + "severity": 1, + "message": "cannot find value `x`", + "source": "rustc" + }] + }); + let (uri, diags) = parse_diagnostics(¶ms).unwrap(); + assert_eq!(uri, "file:///tmp/a.rs"); + assert_eq!(diags.len(), 1); + assert_eq!(diags[0].line, 5); // 0-based 4 → 1-based 5 + assert_eq!(diags[0].character, 9); + assert_eq!(diags[0].severity, Severity::Error); + assert_eq!(diags[0].source.as_deref(), Some("rustc")); + } + + #[test] + fn path_to_uri_prefixes_file_scheme() { + assert_eq!(path_to_uri(Path::new("/tmp/a.rs")), "file:///tmp/a.rs"); + } +} diff --git a/crates/harness-lsp/src/lib.rs b/crates/harness-lsp/src/lib.rs index a033f68..efe8dee 100644 --- a/crates/harness-lsp/src/lib.rs +++ b/crates/harness-lsp/src/lib.rs @@ -1 +1,238 @@ -// LSP client pool + diagnostics service land here in M5. +//! LSP diagnostics pool: maps a file extension to a language server, lazily spawns one server +//! per language, and answers the core `DiagnosticsSource` seam. Built-in servers are only +//! offered when their binary is on `PATH`; config can add or override them. +//! See `docs/09-integrations.md`. + +mod client; + +use std::collections::HashMap; +use std::path::{Path, PathBuf}; +use std::sync::{Arc, Mutex}; +use std::time::Duration; + +use async_trait::async_trait; +use harness_core::lsp::{Diagnostic, DiagnosticsSource}; + +use client::LspClient; + +/// One language server: which binary to run and which extensions it handles. +#[derive(Debug, Clone)] +pub struct ServerConfig { + pub name: String, + pub command: String, + pub args: Vec, + pub extensions: Vec, +} + +/// Built-in servers, tried only when their binary exists on `PATH`. +fn builtin_servers() -> Vec { + vec![ + ServerConfig { + name: "rust".into(), + command: "rust-analyzer".into(), + args: vec![], + extensions: vec!["rs".into()], + }, + ServerConfig { + name: "typescript".into(), + command: "typescript-language-server".into(), + args: vec!["--stdio".into()], + extensions: vec!["ts".into(), "tsx".into(), "js".into(), "jsx".into()], + }, + ServerConfig { + name: "go".into(), + command: "gopls".into(), + args: vec![], + extensions: vec!["go".into()], + }, + ServerConfig { + name: "python".into(), + command: "pyright-langserver".into(), + args: vec!["--stdio".into()], + extensions: vec!["py".into()], + }, + ] +} + +/// LSP `languageId` for a file extension (falls back to the extension itself). +fn language_id_for_ext(ext: &str) -> &str { + match ext { + "rs" => "rust", + "ts" => "typescript", + "tsx" => "typescriptreact", + "js" => "javascript", + "jsx" => "javascriptreact", + "go" => "go", + "py" => "python", + other => other, + } +} + +/// Returns true if `command` is an absolute existing file or resolvable on `PATH`. +fn binary_exists(command: &str) -> bool { + let p = Path::new(command); + if p.is_absolute() { + return p.is_file(); + } + let Some(paths) = std::env::var_os("PATH") else { + return false; + }; + std::env::split_paths(&paths).any(|dir| dir.join(command).is_file()) +} + +enum Slot { + Ready(Arc), + Failed, +} + +pub struct LspPool { + root: PathBuf, + servers: Vec, + /// One slot per server name; absent until first spawn attempt. + clients: tokio::sync::Mutex>, + /// Open documents → last-sent version, so `didChange` bumps monotonically. + open: Mutex>, +} + +impl LspPool { + /// Builds a pool: available built-ins plus any `config` servers (which override built-ins + /// by name; an empty command disables a built-in). + pub fn new(root: PathBuf, config: Vec) -> Self { + let mut by_name: HashMap = HashMap::new(); + for server in builtin_servers() { + if binary_exists(&server.command) { + by_name.insert(server.name.clone(), server); + } + } + for server in config { + if server.command.is_empty() { + by_name.remove(&server.name); + } else { + by_name.insert(server.name.clone(), server); + } + } + Self { + root, + servers: by_name.into_values().collect(), + clients: tokio::sync::Mutex::new(HashMap::new()), + open: Mutex::new(HashMap::new()), + } + } + + fn server_for(&self, path: &Path) -> Option<&ServerConfig> { + let ext = path.extension().and_then(|e| e.to_str())?; + self.servers + .iter() + .find(|s| s.extensions.iter().any(|e| e == ext)) + } + + /// Gets or lazily spawns the client for `server`. Caches a failure so we don't respawn a + /// broken server on every edit. + async fn client_for(&self, server: &ServerConfig) -> Option> { + let mut clients = self.clients.lock().await; + match clients.get(&server.name) { + Some(Slot::Ready(c)) => return Some(c.clone()), + Some(Slot::Failed) => return None, + None => {} + } + match LspClient::spawn(&server.command, &server.args, &self.root).await { + Ok(client) => { + let client = Arc::new(client); + clients.insert(server.name.clone(), Slot::Ready(client.clone())); + Some(client) + } + Err(e) => { + tracing::warn!(server = %server.name, error = %e, "LSP server failed to start"); + clients.insert(server.name.clone(), Slot::Failed); + None + } + } + } +} + +#[async_trait] +impl DiagnosticsSource for LspPool { + async fn touch(&self, path: &Path) { + let Some(server) = self.server_for(path).cloned() else { + return; + }; + let Ok(text) = tokio::fs::read_to_string(path).await else { + return; + }; + let Some(client) = self.client_for(&server).await else { + return; + }; + let ext = path.extension().and_then(|e| e.to_str()).unwrap_or_default(); + let language_id = language_id_for_ext(ext); + + let existing_version = { + let mut open = self.open.lock().unwrap(); + match open.get_mut(path) { + Some(version) => { + *version += 1; + Some(*version) + } + None => { + open.insert(path.to_path_buf(), 1); + None + } + } + }; + + // Fresh diagnostics only reflect the current contents — drop any stale set first. + client.clear(path); + match existing_version { + None => client.did_open(path, language_id, 1, &text), + Some(version) => client.did_change(path, version, &text), + } + } + + async fn diagnostics(&self, path: &Path, wait: Duration) -> Vec { + let Some(server) = self.server_for(path).cloned() else { + return Vec::new(); + }; + let Some(client) = self.client_for(&server).await else { + return Vec::new(); + }; + client.wait_diagnostics(path, wait).await + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn language_ids_map_common_extensions() { + assert_eq!(language_id_for_ext("rs"), "rust"); + assert_eq!(language_id_for_ext("tsx"), "typescriptreact"); + assert_eq!(language_id_for_ext("unknown"), "unknown"); + } + + #[test] + fn config_server_overrides_builtin_by_name() { + let pool = LspPool::new( + std::env::temp_dir(), + vec![ServerConfig { + name: "rust".into(), + command: "my-custom-ra".into(), + args: vec!["--flag".into()], + extensions: vec!["rs".into()], + }], + ); + let server = pool.server_for(Path::new("/x/a.rs")).unwrap(); + assert_eq!(server.command, "my-custom-ra"); + } + + #[test] + fn no_server_for_unknown_extension() { + let pool = LspPool::new(std::env::temp_dir(), vec![]); + assert!(pool.server_for(Path::new("/x/a.zzz")).is_none()); + } + + #[test] + fn binary_exists_finds_a_known_tool() { + assert!(binary_exists("sh")); + assert!(!binary_exists("definitely-not-a-real-binary-xyz")); + } +} diff --git a/crates/harness-lsp/tests/rust_analyzer.rs b/crates/harness-lsp/tests/rust_analyzer.rs new file mode 100644 index 0000000..aa24d89 --- /dev/null +++ b/crates/harness-lsp/tests/rust_analyzer.rs @@ -0,0 +1,34 @@ +//! End-to-end LSP test against a real `rust-analyzer`. Ignored by default (requires the binary +//! on PATH and is slow — RA indexes the project). Run with: +//! cargo test -p harness-lsp --test rust_analyzer -- --ignored --nocapture + +use std::time::Duration; + +use harness_core::lsp::{DiagnosticsSource, Severity}; +use harness_lsp::LspPool; + +#[tokio::test] +#[ignore = "requires rust-analyzer on PATH; slow"] +async fn rust_analyzer_reports_a_type_error() { + // Minimal cargo project with a deliberate type error. + let dir = tempfile::tempdir().unwrap(); + std::fs::write( + dir.path().join("Cargo.toml"), + "[package]\nname = \"probe\"\nversion = \"0.1.0\"\nedition = \"2021\"\n", + ) + .unwrap(); + std::fs::create_dir_all(dir.path().join("src")).unwrap(); + let main_rs = dir.path().join("src/main.rs"); + std::fs::write(&main_rs, "fn main() {\n let x: i32 = \"not an integer\";\n let _ = x;\n}\n").unwrap(); + + let pool = LspPool::new(dir.path().to_path_buf(), vec![]); + pool.touch(&main_rs).await; + // Generous wait: rust-analyzer must index the workspace before it reports anything. + let diags = pool.diagnostics(&main_rs, Duration::from_secs(60)).await; + + println!("diagnostics: {diags:#?}"); + assert!( + diags.iter().any(|d| d.severity == Severity::Error), + "expected at least one error diagnostic, got: {diags:?}" + ); +} diff --git a/crates/harness-tools/src/bash.rs b/crates/harness-tools/src/bash.rs index b9dae14..b28c7a0 100644 --- a/crates/harness-tools/src/bash.rs +++ b/crates/harness-tools/src/bash.rs @@ -151,6 +151,7 @@ mod tests { metadata, spawner: None, context_reporter: None, + diagnostics: None, } } diff --git a/crates/harness-tools/src/diagnostics.rs b/crates/harness-tools/src/diagnostics.rs new file mode 100644 index 0000000..8d1263d --- /dev/null +++ b/crates/harness-tools/src/diagnostics.rs @@ -0,0 +1,68 @@ +//! Shared LSP-diagnostics reporting for the edit/write tools. After a file is written we ask +//! the (optional) diagnostics source to re-analyze it and append any error-severity items to +//! the tool output so the model sees mistakes it just introduced. Best-effort: no source, a +//! slow server, or a timeout all just mean "no diagnostics" — never a tool failure. + +use std::path::Path; +use std::time::Duration; + +use harness_core::lsp::Severity; +use harness_core::tool::{ToolCtx, ToolOutput}; + +/// docs/09-integrations.md: wait up to 1.5s for the server to (re)publish after the change. +const DIAGNOSTICS_WAIT: Duration = Duration::from_millis(1500); + +/// Touches `path` in the language server and appends error-severity diagnostics to `output` +/// (both as a human-readable block in the text and the full set in metadata under `diagnostics`). +pub async fn append_diagnostics(ctx: &ToolCtx, path: &Path, display_name: &str, output: &mut ToolOutput) { + let Some(source) = &ctx.diagnostics else { + return; + }; + source.touch(path).await; + let diagnostics = source.diagnostics(path, DIAGNOSTICS_WAIT).await; + if diagnostics.is_empty() { + return; + } + + let errors: Vec<_> = diagnostics + .iter() + .filter(|d| d.severity == Severity::Error) + .collect(); + if !errors.is_empty() { + output + .output + .push_str("\n\nLSP errors detected in this file, please fix:"); + for diag in &errors { + output.output.push('\n'); + output.output.push_str(&diag.display_line(display_name)); + } + } + + // Full set (all severities) into metadata for the TUI. + let items: Vec = diagnostics + .iter() + .map(|d| { + serde_json::json!({ + "line": d.line, + "character": d.character, + "severity": severity_str(d.severity), + "message": d.message, + "source": d.source, + }) + }) + .collect(); + if let serde_json::Value::Object(map) = &mut output.metadata { + map.insert("diagnostics".into(), serde_json::Value::Array(items)); + } else { + output.metadata = serde_json::json!({ "diagnostics": items }); + } +} + +fn severity_str(severity: Severity) -> &'static str { + match severity { + Severity::Error => "error", + Severity::Warning => "warning", + Severity::Info => "info", + Severity::Hint => "hint", + } +} diff --git a/crates/harness-tools/src/edit/mod.rs b/crates/harness-tools/src/edit/mod.rs index 654ca8d..62865be 100644 --- a/crates/harness-tools/src/edit/mod.rs +++ b/crates/harness-tools/src/edit/mod.rs @@ -196,8 +196,9 @@ impl Tool for EditTool { .map_err(|e| ToolError::Other(format!("{}: {e}", path.display())))?; let (added, removed) = diff_stats(&content_old, &content_new); - let mut output = ToolOutput::new(pattern, "Edit applied successfully.".to_string()); + let mut output = ToolOutput::new(pattern.clone(), "Edit applied successfully.".to_string()); output.metadata = serde_json::json!({"diff": diff, "added": added, "removed": removed}); + crate::diagnostics::append_diagnostics(&ctx, &path, &pattern, &mut output).await; Ok(output) } } @@ -234,6 +235,7 @@ mod tests { metadata, spawner: None, context_reporter: None, + diagnostics: None, } } diff --git a/crates/harness-tools/src/glob.rs b/crates/harness-tools/src/glob.rs index 1600f5f..1c2e72d 100644 --- a/crates/harness-tools/src/glob.rs +++ b/crates/harness-tools/src/glob.rs @@ -132,6 +132,7 @@ mod tests { metadata, spawner: None, context_reporter: None, + diagnostics: None, } } diff --git a/crates/harness-tools/src/grep.rs b/crates/harness-tools/src/grep.rs index 15cf098..7229661 100644 --- a/crates/harness-tools/src/grep.rs +++ b/crates/harness-tools/src/grep.rs @@ -161,6 +161,7 @@ mod tests { metadata, spawner: None, context_reporter: None, + diagnostics: None, } } diff --git a/crates/harness-tools/src/lib.rs b/crates/harness-tools/src/lib.rs index 175d96a..085fb52 100644 --- a/crates/harness-tools/src/lib.rs +++ b/crates/harness-tools/src/lib.rs @@ -1,4 +1,5 @@ mod bash; +mod diagnostics; mod edit; mod glob; mod grep; diff --git a/crates/harness-tools/src/read.rs b/crates/harness-tools/src/read.rs index 03f26f4..d9680fc 100644 --- a/crates/harness-tools/src/read.rs +++ b/crates/harness-tools/src/read.rs @@ -148,6 +148,7 @@ mod tests { metadata, spawner: None, context_reporter: None, + diagnostics: None, } } diff --git a/crates/harness-tools/src/write.rs b/crates/harness-tools/src/write.rs index 6a07da7..6e9231e 100644 --- a/crates/harness-tools/src/write.rs +++ b/crates/harness-tools/src/write.rs @@ -80,6 +80,7 @@ impl Tool for WriteTool { format!("wrote {} bytes", params.content.len()), ); output.metadata = serde_json::json!({"diff": diff, "added": added, "removed": removed}); + crate::diagnostics::append_diagnostics(&ctx, &path, ¶ms.file_path, &mut output).await; Ok(output) } } @@ -116,6 +117,7 @@ mod tests { metadata, spawner: None, context_reporter: None, + diagnostics: None, } }