diff --git a/Cargo.lock b/Cargo.lock index 84b9211..9a654b2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -35,6 +35,15 @@ version = "0.2.21" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "683d7910e743518b0e34f1186f92494becacb047c7b6bf616c96772180fef923" +[[package]] +name = "android_system_properties" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "819e7219dbd41043ac279b19830f2efc897156490d7fd6ea916720117ee66311" +dependencies = [ + "libc", +] + [[package]] name = "anyhow" version = "1.0.103" @@ -92,6 +101,18 @@ version = "1.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1505bd5d3d116872e7271a6d4e16d81d0c8570876c8de68093a09ac269d8aac0" +[[package]] +name = "autocfg" +version = "1.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" + +[[package]] +name = "base64" +version = "0.21.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9d297deb1925b89f2ccc13d7635fa0714f12c87adce1c75356b39ca9b7178567" + [[package]] name = "base64" version = "0.22.1" @@ -175,6 +196,20 @@ dependencies = [ "rand_core 0.10.1", ] +[[package]] +name = "chrono" +version = "0.4.45" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1aa79e62e7697b8e29b513a68abacf485adcd1fe8284a4316c5ae868e6633327" +dependencies = [ + "iana-time-zone", + "js-sys", + "num-traits", + "serde", + "wasm-bindgen", + "windows-link", +] + [[package]] name = "compact_str" version = "0.8.2" @@ -217,6 +252,12 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "core-foundation-sys" +version = "0.8.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "773648b94d0e5d620f64f280777445740e61fe701025087ec8b57f45c791888b" + [[package]] name = "cpufeatures" version = "0.3.0" @@ -711,14 +752,28 @@ dependencies = [ name = "harness-lsp" version = "0.1.0" dependencies = [ + "async-trait", "harness-core", + "serde_json", + "tempfile", + "tokio", + "tracing", ] [[package]] name = "harness-mcp" version = "0.1.0" dependencies = [ + "async-trait", "harness-core", + "rmcp", + "serde", + "serde_json", + "tempfile", + "thiserror 2.0.18", + "tokio", + "tokio-util", + "tracing", ] [[package]] @@ -909,7 +964,7 @@ version = "0.1.20" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "96547c2556ec9d12fb1578c4eaf448b04993e7fb79cbaad930a656880a6bdfa0" dependencies = [ - "base64", + "base64 0.22.1", "bytes", "futures-channel", "futures-util", @@ -926,6 +981,30 @@ dependencies = [ "tracing", ] +[[package]] +name = "iana-time-zone" +version = "0.1.65" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e31bc9ad994ba00e440a8aa5c9ef0ec67d5cb5e5cb0cc7f8b744a35b389cc470" +dependencies = [ + "android_system_properties", + "core-foundation-sys", + "iana-time-zone-haiku", + "js-sys", + "log", + "wasm-bindgen", + "windows-core", +] + +[[package]] +name = "iana-time-zone-haiku" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f31827a206f56af32e590ba56d5d2d085f558508192593743f16b2306495269f" +dependencies = [ + "cc", +] + [[package]] name = "icu_collections" version = "2.2.0" @@ -1293,6 +1372,15 @@ version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "521739c6d2bac4aa25192232afe6841231376b2b26d4d9fae5ecf8ca5772e441" +[[package]] +name = "num-traits" +version = "0.2.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "071dfc062690e90b734c0b2273ce72ad0ffa95f0c74596bc250dcfd960262841" +dependencies = [ + "autocfg", +] + [[package]] name = "once_cell" version = "1.21.4" @@ -1600,7 +1688,7 @@ version = "0.12.28" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "eddd3ca559203180a307f12d114c268abf583f59b03cb906fd0b3ff8646c1147" dependencies = [ - "base64", + "base64 0.22.1", "bytes", "futures-core", "futures-util", @@ -1649,6 +1737,38 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "rmcp" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "33a0110d28bd076f39e14bfd5b0340216dd18effeb5d02b43215944cc3e5c751" +dependencies = [ + "base64 0.21.7", + "chrono", + "futures", + "paste", + "pin-project-lite", + "rmcp-macros", + "schemars", + "serde", + "serde_json", + "thiserror 2.0.18", + "tokio", + "tokio-util", + "tracing", +] + +[[package]] +name = "rmcp-macros" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a6e2b2fd7497540489fa2db285edd43b7ed14c49157157438664278da6e42a7a" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "rusqlite" version = "0.32.1" @@ -2600,12 +2720,65 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" +[[package]] +name = "windows-core" +version = "0.62.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b8e83a14d34d0623b51dce9581199302a221863196a1dde71a7663a4c2be9deb" +dependencies = [ + "windows-implement", + "windows-interface", + "windows-link", + "windows-result", + "windows-strings", +] + +[[package]] +name = "windows-implement" +version = "0.60.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "053e2e040ab57b9dc951b72c264860db7eb3b0200ba345b4e4c3b14f67855ddf" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "windows-interface" +version = "0.59.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f316c4a2570ba26bbec722032c4099d8c8bc095efccdc15688708623367e358" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "windows-link" version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" +[[package]] +name = "windows-result" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7781fa89eaf60850ac3d2da7af8e5242a5ea78d1a11c49bf2910bb5a73853eb5" +dependencies = [ + "windows-link", +] + +[[package]] +name = "windows-strings" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7837d08f69c77cf6b07689544538e017c1bfcf57e34b4c0ff58e6c2cd3b37091" +dependencies = [ + "windows-link", +] + [[package]] name = "windows-sys" version = "0.48.0" diff --git a/crates/harness-app/src/lib.rs b/crates/harness-app/src/lib.rs index b339749..2603f6c 100644 --- a/crates/harness-app/src/lib.rs +++ b/crates/harness-app/src/lib.rs @@ -16,10 +16,11 @@ 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::{ - ContextReporter, SpawnError, SpawnOutcome, SpawnRequest, SubagentSpawner, ToolRegistry, + ContextReporter, SpawnError, SpawnOutcome, SpawnRequest, SubagentSpawner, Tool, ToolRegistry, }; use harness_core::types::{ Message, MessageId, ModelRef, Part, PartBody, PartId, Session, SessionId, @@ -75,6 +76,71 @@ fn db_path(cwd: &Path) -> PathBuf { base.join(format!("{}.sqlite", slug)) } +/// Builds the provider registry from config: every provider with an API key is registered. +fn providers_from_config(config: &Config) -> ProviderRegistry { + let mut providers = ProviderRegistry::new(); + if let Some(key) = config + .providers + .get("anthropic") + .and_then(|p| p.api_key.clone()) + { + providers.register(Arc::new(AnthropicProvider::new(key))); + } + if let Some(key) = config + .providers + .get("openai") + .and_then(|p| p.api_key.clone()) + { + let provider = match config + .providers + .get("openai") + .and_then(|p| p.base_url.clone()) + { + Some(base_url) => OpenAiProvider::with_base_url(key, base_url), + None => OpenAiProvider::new(key), + }; + providers.register(Arc::new(provider)); + } + // OpenCode Zen: OpenAI-compatible chat gateway under its own `opencode` provider id, so it + // coexists with a real `openai` provider. `base_url` defaults to Zen's endpoint. + if let Some(key) = config + .providers + .get("opencode") + .and_then(|p| p.api_key.clone()) + { + let base_url = config + .providers + .get("opencode") + .and_then(|p| p.base_url.clone()); + providers.register(Arc::new(OpenAiProvider::opencode(key, base_url))); + } + providers +} + +/// Connects every enabled MCP server declared in config and returns their tool adapters. A +/// server that fails to start is logged and skipped inside `harness_mcp` — never fatal. +async fn connect_mcp_tools(config: &Config) -> Vec> { + let servers: HashMap = config + .mcp + .iter() + .filter(|(_, c)| c.enabled.unwrap_or(true) && !c.command.is_empty()) + .map(|(name, c)| { + ( + name.clone(), + harness_mcp::ServerConfig { + command: c.command.clone(), + args: c.args.clone(), + env: c.env.clone(), + }, + ) + }) + .collect(); + if servers.is_empty() { + return Vec::new(); + } + harness_mcp::connect_all(servers).await +} + /// Non-blocking, multi-turn engine API used by the TUI. #[derive(Clone)] pub struct EngineHandle { @@ -90,6 +156,14 @@ 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>, + /// Pre-rendered "## Skills" system block (name + description), injected into every run. + /// `None` when no skills are configured. + skills_prompt: Option, + /// Slash commands by name, expanded into the user message by the TUI input layer. + commands: HashMap, cwd: PathBuf, data_dir: PathBuf, runs: Mutex>, @@ -107,69 +181,36 @@ struct RunHandle { impl EngineHandle { /// Persistent SQLite-backed store. Used by the TUI and the default headless `App`. - pub fn init(cwd: PathBuf) -> Result { + /// Async because it connects any configured MCP servers (spawn + initialize + list tools) + /// before the first turn so their tools are advertised to the model. + pub async fn init(cwd: PathBuf) -> Result { let path = db_path(&cwd); let store = Store::open(&path)?; - let handle = Self::new(cwd, store)?; + let config = config::load(&cwd)?; + let providers = providers_from_config(&config); + let mcp_tools = connect_mcp_tools(&config).await; + let handle = Self::build(cwd, store, config, providers, mcp_tools)?; handle.spawn_catalog_refresh(); Ok(handle) } - /// In-memory store — useful for tests and ephemeral sessions. + /// In-memory store — useful for tests and ephemeral sessions. Skips MCP (no external + /// servers in tests) and stays synchronous. pub fn init_in_memory(cwd: PathBuf) -> Result { let store = Store::open_in_memory()?; - Self::new(cwd, store) - } - - fn new(cwd: PathBuf, store: Store) -> Result { let config = config::load(&cwd)?; - - let mut providers = ProviderRegistry::new(); - if let Some(key) = config - .providers - .get("anthropic") - .and_then(|p| p.api_key.clone()) - { - providers.register(Arc::new(AnthropicProvider::new(key))); - } - if let Some(key) = config - .providers - .get("openai") - .and_then(|p| p.api_key.clone()) - { - let provider = match config - .providers - .get("openai") - .and_then(|p| p.base_url.clone()) - { - Some(base_url) => OpenAiProvider::with_base_url(key, base_url), - None => OpenAiProvider::new(key), - }; - providers.register(Arc::new(provider)); - } - // OpenCode Zen: OpenAI-compatible chat gateway under its own `opencode` provider id, - // so it coexists with a real `openai` provider. `base_url` defaults to Zen's endpoint. - if let Some(key) = config - .providers - .get("opencode") - .and_then(|p| p.api_key.clone()) - { - let base_url = config - .providers - .get("opencode") - .and_then(|p| p.base_url.clone()); - providers.register(Arc::new(OpenAiProvider::opencode(key, base_url))); - } - - Self::build(cwd, store, config, providers) + let providers = providers_from_config(&config); + Self::build(cwd, store, config, providers, Vec::new()) } - /// Shared construction over an explicit provider set — the seam tests use to inject a mock. + /// Shared construction over an explicit provider set and pre-connected extra tools (MCP) — + /// the seam the tests use to inject a mock provider. fn build( cwd: PathBuf, store: Store, config: Config, providers: ProviderRegistry, + extra_tools: Vec>, ) -> Result { let bus = EventBus::new(); let permissions = Arc::new(PermissionService::new(bus.clone())); @@ -177,6 +218,24 @@ impl EngineHandle { let mut tools = ToolRegistry::new(); harness_tools::register_builtins(&mut tools); harness_tools::register_task_tool(&mut tools); + for tool in extra_tools { + tools.register(tool); + } + + // Skills: layered global + project markdown, advertised in the system prompt and pulled + // on demand by the `skill` tool (only registered when at least one skill exists). + let global_skill_dir = dirs::config_dir().map(|d| d.join("ai-harness").join("skill")); + let project_skill_dir = cwd.join(".harness").join("skill"); + let skills = config::load_skills(global_skill_dir.as_deref(), Some(&project_skill_dir)); + harness_tools::register_skill_tool(&mut tools, &skills); + let skills_prompt = config::skills_prompt(&skills); + + // Slash commands: layered global + project markdown, expanded into the user message by + // the TUI input layer (project wins by name). + let global_command_dir = dirs::config_dir().map(|d| d.join("ai-harness").join("command")); + let project_command_dir = cwd.join(".harness").join("command"); + let commands = + config::load_commands(global_command_dir.as_deref(), Some(&project_command_dir)); // Layered agent registry: bundled markdown → global dir → project dir → config patches. let global_agent_dir = dirs::config_dir().map(|d| d.join("ai-harness").join("agent")); @@ -196,6 +255,21 @@ 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 +279,9 @@ impl EngineHandle { providers, catalog, agents, + diagnostics, + skills_prompt, + commands, cwd, data_dir, runs: Mutex::new(HashMap::new()), @@ -236,6 +313,11 @@ impl EngineHandle { self.inner.config.clone() } + /// A loaded slash command by name (without the leading `/`), if defined. + pub fn command(&self, name: &str) -> Option { + self.inner.commands.get(name).cloned() + } + pub async fn new_session(&self, agent: &str, model_ref: &str) -> Result { let (provider_id, model_id) = model_ref .split_once('/') @@ -254,6 +336,18 @@ impl EngineHandle { session_id: SessionId, text: String, model_ref: &str, + ) -> Result<(), AppError> { + self.prompt_with(session_id, text, model_ref, None).await + } + + /// Like [`prompt`](Self::prompt) but lets a slash command override the agent for this one + /// run (`agent_override`) without mutating the stored session agent. + pub async fn prompt_with( + &self, + session_id: SessionId, + text: String, + model_ref: &str, + agent_override: Option<&str>, ) -> Result<(), AppError> { let (provider_id, model_id) = model_ref .split_once('/') @@ -300,14 +394,18 @@ impl EngineHandle { .bus .publish(AppEvent::PartUpdated { part: user_part }); - // Resolve the session's agent from the registry (falls back to a generic assistant). - let session_agent = self - .inner - .store - .session(session_id.clone()) - .await? - .map(|s| s.agent) - .unwrap_or_else(|| "orchestrator".to_string()); + // Resolve the agent: a command's `agent:` override for this run, else the session's + // stored agent (falling back to a generic assistant). + let session_agent = match agent_override { + Some(name) => name.to_string(), + None => self + .inner + .store + .session(session_id.clone()) + .await? + .map(|s| s.agent) + .unwrap_or_else(|| "orchestrator".to_string()), + }; let agent = self.inner.agent_def(&session_agent); let inject_job_board = self.inner.config.orchestration.job_board && agent.mode.is_primary(); let board = self.inner.board_for(&session_id).await?; @@ -328,6 +426,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(), @@ -340,6 +439,7 @@ impl EngineHandle { inject_job_board, reminder_turn_start: self.inner.reminder("turn_start"), reminder_after_file_tool: self.inner.reminder("after_file_tool"), + skills_prompt: self.inner.skills_prompt.clone(), }; // Reserve the run slot *before* spawning. If we inserted after spawning, a run that @@ -633,6 +733,7 @@ impl EngineInner { spawner: Some(self.arc()), job_board: Some(board), context_reporter: reporter, + diagnostics: self.diagnostics.clone(), } } } @@ -751,6 +852,7 @@ impl SubagentSpawner for EngineInner { inject_job_board: agent.mode.is_primary(), reminder_turn_start: self.reminder("turn_start"), reminder_after_file_tool: self.reminder("after_file_tool"), + skills_prompt: self.skills_prompt.clone(), }; // Register the launch on the parent board (also makes foreground results reusable). @@ -867,8 +969,8 @@ pub struct App { impl App { /// Persistent SQLite-backed store with an auto-approve permission frontend — the default /// headless configuration used by `harness run -p`. - pub fn init(cwd: PathBuf) -> Result { - let engine = EngineHandle::init(cwd)?; + pub async fn init(cwd: PathBuf) -> Result { + let engine = EngineHandle::init(cwd).await?; let _auto_approve_handle = Some(spawn_auto_approve_task(&engine)); Ok(Self { engine, @@ -997,6 +1099,7 @@ mod tests { Store::open_in_memory().unwrap(), Config::default(), providers, + Vec::new(), ) .unwrap() } @@ -1086,6 +1189,7 @@ mod tests { Store::open_in_memory().unwrap(), Config::default(), providers, + Vec::new(), ) .unwrap(); // Auto-approve permission asks (the child's read tool gates on `read`). @@ -1263,6 +1367,82 @@ mod tests { assert_eq!(app.engine.inner.tools.all().len(), 7); } + #[tokio::test] + async fn project_skill_registers_the_skill_tool_and_advertises_it() { + let dir = tempfile::tempdir().unwrap(); + let skill_dir = dir.path().join(".harness").join("skill").join("fmt"); + std::fs::create_dir_all(&skill_dir).unwrap(); + std::fs::write( + skill_dir.join("SKILL.md"), + "---\ndescription: format the code\n---\nRun cargo fmt.", + ) + .unwrap(); + + let engine = EngineHandle::init_in_memory(dir.path().to_path_buf()).unwrap(); + // 6 built-ins + task + skill. + assert_eq!(engine.inner.tools.all().len(), 8); + assert!(engine.inner.tools.get("skill").is_some()); + let prompt = engine + .inner + .skills_prompt + .clone() + .expect("skills advertised"); + assert!(prompt.contains("**fmt** — format the code")); + } + + #[tokio::test] + async fn project_command_loads_and_expands() { + let dir = tempfile::tempdir().unwrap(); + let cmd_dir = dir.path().join(".harness").join("command"); + std::fs::create_dir_all(&cmd_dir).unwrap(); + std::fs::write( + cmd_dir.join("greet.md"), + "---\ndescription: greet\nagent: explorer\n---\nHello $1, welcome.", + ) + .unwrap(); + + let engine = EngineHandle::init_in_memory(dir.path().to_path_buf()).unwrap(); + let cmd = engine.command("greet").expect("command loaded"); + assert_eq!(cmd.agent.as_deref(), Some("explorer")); + assert_eq!(cmd.expand("world"), "Hello world, welcome."); + assert!(engine.command("missing").is_none()); + } + + #[tokio::test] + async fn command_run_reaches_engine_with_agent_override() { + let dir = tempfile::tempdir().unwrap(); + let engine = mock_engine(dir.path().to_path_buf()); + spawn_auto_approve_task(&engine); + let session = engine + .new_session("orchestrator", "mock/mock-model") + .await + .unwrap(); + + // Drive the command path directly: expand + agent override for this run only. + engine + .prompt_with( + session.clone(), + "do the thing".into(), + "mock/mock-model", + Some("explorer"), + ) + .await + .unwrap(); + + // Wait for the run to finish. + let mut rx = engine.bus().subscribe(); + loop { + if let Ok(AppEvent::RunFinished { session_id, .. }) = rx.recv().await { + if session_id == session { + break; + } + } + } + // The stored session agent is untouched by the per-run override. + let stored = engine.get_session(session).await.unwrap().unwrap(); + assert_eq!(stored.agent, "orchestrator"); + } + #[tokio::test] async fn run_prompt_errors_on_unregistered_provider() { let dir = tempfile::tempdir().unwrap(); diff --git a/crates/harness-core/src/config/markdown.rs b/crates/harness-core/src/config/markdown.rs new file mode 100644 index 0000000..9ce54d6 --- /dev/null +++ b/crates/harness-core/src/config/markdown.rs @@ -0,0 +1,296 @@ +//! Markdown-defined commands and skills (M5). Both are YAML-frontmatter + body files loaded +//! from a global dir (`~/.config/ai-harness//`) and a project dir (`/.harness/ +//! /`), project winning by name. See `docs/06-config.md` and `docs/09-integrations.md`. +//! +//! - Commands (`command/*.md`) are a pure input-layer concern: `/name args` expands the body +//! template (`$ARGUMENTS`, `$1..$9`) into the user message, optionally switching agent/model. +//! - Skills (`skill//SKILL.md`) advertise `name + description` in the system prompt; the +//! model pulls a skill's body on demand via the built-in `skill` tool. + +use std::collections::HashMap; +use std::path::Path; + +use serde::Deserialize; + +/// A slash command: a named prompt template with an optional agent/model override. +#[derive(Debug, Clone, PartialEq)] +pub struct CommandDef { + /// Invocation name (the file stem); used as `/name`. + pub name: String, + pub description: String, + /// Run the expanded prompt under this agent instead of the session's default. + pub agent: Option, + /// Run under this `provider/model` instead of the session's default. + pub model: Option, + /// The body, with `$ARGUMENTS` / `$1..$9` placeholders. + pub template: String, +} + +impl CommandDef { + /// Substitutes `$ARGUMENTS` (the whole argument string) and `$1..$9` (whitespace-split + /// positionals; missing ones become empty) into the template. + pub fn expand(&self, arguments: &str) -> String { + let positionals: Vec<&str> = arguments.split_whitespace().collect(); + let mut out = self.template.replace("$ARGUMENTS", arguments); + for i in 1..=9 { + let value = positionals.get(i - 1).copied().unwrap_or(""); + out = out.replace(&format!("${i}"), value); + } + out + } +} + +/// A skill: advertised by `name + description`, body loaded on demand by the `skill` tool. +#[derive(Debug, Clone, PartialEq)] +pub struct SkillDef { + /// Skill name (the containing directory name); used as the `skill` tool argument. + pub name: String, + pub description: String, + pub body: String, +} + +/// Frontmatter fields shared by commands (all optional). +#[derive(Debug, Default, Deserialize)] +#[serde(default)] +struct CommandFrontmatter { + description: Option, + agent: Option, + model: Option, +} + +#[derive(Debug, Default, Deserialize)] +#[serde(default)] +struct SkillFrontmatter { + description: Option, +} + +/// Splits `---\n…\n---\n` frontmatter from the body. A file with no leading fence is all body. +fn split_frontmatter(content: &str) -> (&str, &str) { + let rest = match content + .strip_prefix("---\n") + .or_else(|| content.strip_prefix("---\r\n")) + { + Some(r) => r, + None => return ("", content), + }; + for delim in ["\n---\n", "\n---\r\n"] { + if let Some(idx) = rest.find(delim) { + return (&rest[..idx], &rest[idx + delim.len()..]); + } + } + if rest.ends_with("\n---") { + return (rest.trim_end_matches("\n---"), ""); + } + ("", content) +} + +fn parse_command(name: &str, content: &str) -> CommandDef { + let (fm_raw, body) = split_frontmatter(content); + let fm: CommandFrontmatter = if fm_raw.trim().is_empty() { + CommandFrontmatter::default() + } else { + serde_yaml_ng::from_str(fm_raw).unwrap_or_default() + }; + CommandDef { + name: name.to_string(), + description: fm.description.unwrap_or_default(), + agent: fm.agent, + model: fm.model, + template: body.trim().to_string(), + } +} + +fn parse_skill(name: &str, content: &str) -> SkillDef { + let (fm_raw, body) = split_frontmatter(content); + let fm: SkillFrontmatter = if fm_raw.trim().is_empty() { + SkillFrontmatter::default() + } else { + serde_yaml_ng::from_str(fm_raw).unwrap_or_default() + }; + SkillDef { + name: name.to_string(), + description: fm.description.unwrap_or_default(), + body: body.trim().to_string(), + } +} + +/// Loads `command/*.md` from global then project dirs (project wins by name). +pub fn load_commands( + global_dir: Option<&Path>, + project_dir: Option<&Path>, +) -> HashMap { + let mut commands = HashMap::new(); + for dir in [global_dir, project_dir].into_iter().flatten() { + let Ok(entries) = std::fs::read_dir(dir) else { + continue; + }; + for entry in entries.flatten() { + let path = entry.path(); + if path.extension().and_then(|e| e.to_str()) != Some("md") { + continue; + } + let Some(name) = path.file_stem().and_then(|s| s.to_str()) else { + continue; + }; + if let Ok(content) = std::fs::read_to_string(&path) { + commands.insert(name.to_string(), parse_command(name, &content)); + } + } + } + commands +} + +/// Loads `skill//SKILL.md` from global then project dirs (project wins by name), +/// returned name-sorted for a stable system-prompt listing. +pub fn load_skills(global_dir: Option<&Path>, project_dir: Option<&Path>) -> Vec { + let mut by_name: HashMap = HashMap::new(); + for dir in [global_dir, project_dir].into_iter().flatten() { + let Ok(entries) = std::fs::read_dir(dir) else { + continue; + }; + for entry in entries.flatten() { + let path = entry.path(); + if !path.is_dir() { + continue; + } + let Some(name) = path.file_name().and_then(|s| s.to_str()) else { + continue; + }; + let skill_file = path.join("SKILL.md"); + if let Ok(content) = std::fs::read_to_string(&skill_file) { + by_name.insert(name.to_string(), parse_skill(name, &content)); + } + } + } + let mut skills: Vec = by_name.into_values().collect(); + skills.sort_by(|a, b| a.name.cmp(&b.name)); + skills +} + +/// The system-prompt section advertising available skills (name + description). `None` when +/// there are no skills, so no empty section is injected. +pub fn skills_prompt(skills: &[SkillDef]) -> Option { + if skills.is_empty() { + return None; + } + let mut section = String::from( + "## Skills\n\nThese skills are available. Load a skill's full instructions on demand \ + by calling the `skill` tool with its name before doing the related work:\n", + ); + for skill in skills { + section.push_str(&format!("- **{}** — {}\n", skill.name, skill.description)); + } + Some(section.trim_end().to_string()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn expand_substitutes_arguments_and_positionals() { + let cmd = CommandDef { + name: "greet".into(), + description: String::new(), + agent: None, + model: None, + template: "Say $1 to $2. All: $ARGUMENTS".into(), + }; + assert_eq!(cmd.expand("hi there"), "Say hi to there. All: hi there"); + // Missing positionals collapse to empty. + assert_eq!(cmd.expand("solo"), "Say solo to . All: solo"); + } + + #[test] + fn parse_command_reads_frontmatter_and_body() { + let md = "---\ndescription: review a PR\nagent: oracle\nmodel: openai/gpt-5\n---\nReview $ARGUMENTS please.\n"; + let cmd = parse_command("review", md); + assert_eq!(cmd.description, "review a PR"); + assert_eq!(cmd.agent.as_deref(), Some("oracle")); + assert_eq!(cmd.model.as_deref(), Some("openai/gpt-5")); + assert_eq!(cmd.template, "Review $ARGUMENTS please."); + } + + #[test] + fn parse_command_without_frontmatter_is_all_template() { + let cmd = parse_command("x", "just do $1"); + assert_eq!(cmd.template, "just do $1"); + assert!(cmd.agent.is_none()); + } + + #[test] + fn parse_skill_reads_description_and_body() { + let md = "---\ndescription: format code\n---\nRun the formatter.\n"; + let skill = parse_skill("formatter", md); + assert_eq!(skill.name, "formatter"); + assert_eq!(skill.description, "format code"); + assert_eq!(skill.body, "Run the formatter."); + } + + #[test] + fn skills_prompt_lists_each_and_is_none_when_empty() { + assert!(skills_prompt(&[]).is_none()); + let skills = vec![ + SkillDef { + name: "a".into(), + description: "does a".into(), + body: "…".into(), + }, + SkillDef { + name: "b".into(), + description: "does b".into(), + body: "…".into(), + }, + ]; + let prompt = skills_prompt(&skills).unwrap(); + assert!(prompt.contains("- **a** — does a")); + assert!(prompt.contains("- **b** — does b")); + } + + #[test] + fn load_skills_reads_skill_dirs_and_project_wins() { + let dir = tempfile::tempdir().unwrap(); + let global = dir.path().join("global"); + let project = dir.path().join("project"); + std::fs::create_dir_all(global.join("fmt")).unwrap(); + std::fs::create_dir_all(project.join("fmt")).unwrap(); + std::fs::create_dir_all(global.join("lint")).unwrap(); + std::fs::write( + global.join("fmt/SKILL.md"), + "---\ndescription: global fmt\n---\nglobal body", + ) + .unwrap(); + std::fs::write( + project.join("fmt/SKILL.md"), + "---\ndescription: project fmt\n---\nproject body", + ) + .unwrap(); + std::fs::write( + global.join("lint/SKILL.md"), + "---\ndescription: lint\n---\nlint body", + ) + .unwrap(); + + let skills = load_skills(Some(&global), Some(&project)); + assert_eq!(skills.len(), 2); + // Sorted by name: fmt, lint. + assert_eq!(skills[0].name, "fmt"); + assert_eq!(skills[0].description, "project fmt"); // project overrode global + assert_eq!(skills[0].body, "project body"); + assert_eq!(skills[1].name, "lint"); + } + + #[test] + fn load_commands_project_overrides_global() { + let dir = tempfile::tempdir().unwrap(); + let global = dir.path().join("g"); + let project = dir.path().join("p"); + std::fs::create_dir_all(&global).unwrap(); + std::fs::create_dir_all(&project).unwrap(); + std::fs::write(global.join("deploy.md"), "global deploy").unwrap(); + std::fs::write(project.join("deploy.md"), "project deploy").unwrap(); + let commands = load_commands(Some(&global), Some(&project)); + assert_eq!(commands.len(), 1); + assert_eq!(commands["deploy"].template, "project deploy"); + } +} diff --git a/crates/harness-core/src/config/mod.rs b/crates/harness-core/src/config/mod.rs index 825e4e6..77796ac 100644 --- a/crates/harness-core/src/config/mod.rs +++ b/crates/harness-core/src/config/mod.rs @@ -1,7 +1,9 @@ pub mod load; +pub mod markdown; pub mod schema; pub use load::{load, ConfigError}; +pub use markdown::{load_commands, load_skills, skills_prompt, CommandDef, SkillDef}; pub use schema::{ AgentPatch, Config, LspServerConfig, McpServerConfig, OrchestrationConfig, ProviderConfig, TuiConfig, diff --git a/crates/harness-core/src/engine/loop.rs b/crates/harness-core/src/engine/loop.rs index 97580f7..16097b6 100644 --- a/crates/harness-core/src/engine/loop.rs +++ b/crates/harness-core/src/engine/loop.rs @@ -28,6 +28,8 @@ pub struct RunConfig { pub reminder_turn_start: Option, /// Optional user-provided reminder injected on the turn after a file tool ran. pub reminder_after_file_tool: Option, + /// Pre-rendered "## Skills" system block advertising loadable skills. `None` = no skills. + pub skills_prompt: Option, } /// Adds a step's usage/cost onto the persisted session and republishes it. Cost accounting is @@ -240,6 +242,7 @@ pub async fn run_session( let system_blocks = system::assemble( system::env_header(&ctx.cwd), &run_config.agent_prompt, + run_config.skills_prompt.as_deref(), &run_config.instructions, ); let tools: Vec = ctx @@ -434,6 +437,7 @@ mod tests { spawner: None, job_board: None, context_reporter: None, + diagnostics: None, } } @@ -518,6 +522,7 @@ mod tests { inject_job_board: false, reminder_turn_start: None, reminder_after_file_tool: None, + skills_prompt: None, }; let outcome = run_session(Arc::new(provider), ctx, &run_config, || 2).await; @@ -628,6 +633,7 @@ mod tests { inject_job_board: false, reminder_turn_start: None, reminder_after_file_tool: None, + skills_prompt: None, }; let outcome = run_session(Arc::new(provider), ctx, &run_config, || 2).await; @@ -735,6 +741,7 @@ mod tests { inject_job_board: true, reminder_turn_start: None, reminder_after_file_tool: None, + skills_prompt: None, }; let provider = std::sync::Arc::new(CapturingProvider { @@ -802,6 +809,7 @@ mod tests { inject_job_board: false, reminder_turn_start: Some("REMEMBER: stay on task.".into()), reminder_after_file_tool: None, + skills_prompt: None, }; let provider = std::sync::Arc::new(CapturingProvider { 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/engine/system.rs b/crates/harness-core/src/engine/system.rs index 22b0349..700764b 100644 --- a/crates/harness-core/src/engine/system.rs +++ b/crates/harness-core/src/engine/system.rs @@ -25,10 +25,18 @@ pub fn env_header(cwd: &Path) -> String { header } -/// Ordered system prompt blocks: environment header, agent prompt, then project -/// instructions (e.g. AGENTS.md contents). Order matches `02-engine.md`. -pub fn assemble(env_header: String, agent_prompt: &str, instructions: &[String]) -> Vec { +/// Ordered system prompt blocks: environment header, agent prompt, the optional skills +/// listing, then project instructions (e.g. AGENTS.md contents). Order matches `02-engine.md`. +pub fn assemble( + env_header: String, + agent_prompt: &str, + skills: Option<&str>, + instructions: &[String], +) -> Vec { let mut blocks = vec![env_header, agent_prompt.to_string()]; + if let Some(skills) = skills { + blocks.push(skills.to_string()); + } blocks.extend(instructions.iter().cloned()); blocks } @@ -49,8 +57,20 @@ mod tests { let blocks = assemble( "ENV".to_string(), "AGENT", + None, &["AGENTS.md contents".to_string()], ); assert_eq!(blocks, vec!["ENV", "AGENT", "AGENTS.md contents"]); } + + #[test] + fn assemble_inserts_skills_after_agent_before_instructions() { + let blocks = assemble( + "ENV".to_string(), + "AGENT", + Some("SKILLS"), + &["INSTR".to_string()], + ); + assert_eq!(blocks, vec!["ENV", "AGENT", "SKILLS", "INSTR"]); + } } 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..e8c54a2 100644 --- a/crates/harness-lsp/src/lib.rs +++ b/crates/harness-lsp/src/lib.rs @@ -1 +1,241 @@ -// 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..e259608 --- /dev/null +++ b/crates/harness-lsp/tests/rust_analyzer.rs @@ -0,0 +1,38 @@ +//! 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-mcp/Cargo.toml b/crates/harness-mcp/Cargo.toml index 86e02af..f21f755 100644 --- a/crates/harness-mcp/Cargo.toml +++ b/crates/harness-mcp/Cargo.toml @@ -6,6 +6,17 @@ license.workspace = true [dependencies] harness-core = { workspace = true } +rmcp = { workspace = true, features = ["client", "transport-child-process"] } +tokio = { workspace = true } +async-trait = { workspace = true } +serde = { workspace = true } +serde_json = { workspace = true } +tracing = { workspace = true } +thiserror = { workspace = true } + +[dev-dependencies] +tokio-util = { workspace = true } +tempfile = { workspace = true } [lints] workspace = true diff --git a/crates/harness-mcp/src/lib.rs b/crates/harness-mcp/src/lib.rs index f2e32d3..1818905 100644 --- a/crates/harness-mcp/src/lib.rs +++ b/crates/harness-mcp/src/lib.rs @@ -1 +1,264 @@ -// MCP stdio client → Tool adapters land here in M5. +//! MCP stdio client → `Tool` adapters (M5). For each configured server we spawn the child +//! over rmcp's `TokioChildProcess` transport, `initialize`, `list_tools`, and wrap every +//! remote tool as an [`McpTool`] named `{server}_{tool}`. Servers are gated behind the `mcp` +//! permission key. See `docs/09-integrations.md`. +//! +//! Out of scope for v1 (matching the doc): resources, prompts, sampling, and non-stdio +//! transports. + +use std::collections::HashMap; +use std::path::PathBuf; +use std::sync::Arc; + +use async_trait::async_trait; +use harness_core::tool::{Tool, ToolCtx, ToolError, ToolOutput}; +use rmcp::model::{CallToolRequestParam, RawContent}; +use rmcp::service::RunningService; +use rmcp::transport::TokioChildProcess; +use rmcp::{RoleClient, ServiceExt}; +use tokio::process::Command; + +/// Sanitized-name cap so a `{server}_{tool}` name stays a legal tool identifier. +const MAX_NAME_LEN: usize = 64; + +/// One configured MCP server: the child command plus its environment. +#[derive(Debug, Clone)] +pub struct ServerConfig { + pub command: String, + pub args: Vec, + pub env: HashMap, +} + +#[derive(Debug, thiserror::Error)] +enum ConnectError { + #[error("spawn/transport failed: {0}")] + Transport(#[from] std::io::Error), + #[error("MCP service error: {0}")] + Service(#[from] rmcp::service::ServiceError), +} + +/// Connects every configured server and returns ready tool adapters. A server that fails to +/// start (or list its tools) is logged and skipped — the rest are unaffected, and the session +/// still runs with whatever connected. `named_servers` is `{server_name: config}`. +pub async fn connect_all(named_servers: HashMap) -> Vec> { + let mut tools: Vec> = Vec::new(); + for (name, config) in named_servers { + match connect(&name, &config).await { + Ok(mut server_tools) => { + tracing::info!(server = %name, count = server_tools.len(), "MCP server connected"); + tools.append(&mut server_tools); + } + Err(e) => { + tracing::warn!(server = %name, error = %e, "MCP server failed to start; skipping"); + } + } + } + tools +} + +async fn connect(name: &str, config: &ServerConfig) -> Result>, ConnectError> { + let mut command = Command::new(&config.command); + command.args(&config.args); + for (key, value) in &config.env { + command.env(key, value); + } + // rmcp sets stdin/stdout to piped and kill-on-drop; the child dies with the `RunningService`. + let transport = TokioChildProcess::new(&mut command)?; + let service = Arc::new(().serve(transport).await?); + + let remote_tools = service.peer().list_all_tools().await?; + let adapters = remote_tools + .into_iter() + .map(|tool| { + let full_name = qualified_name(name, &tool.name); + let parameters = serde_json::Value::Object((*tool.input_schema).clone()); + Arc::new(McpTool { + full_name, + remote_name: tool.name.to_string(), + description: tool.description.to_string(), + parameters, + service: service.clone(), + }) as Arc + }) + .collect(); + Ok(adapters) +} + +/// `{server}_{tool}` sanitized to `[a-zA-Z0-9_-]` and capped at [`MAX_NAME_LEN`] chars. +fn qualified_name(server: &str, tool: &str) -> String { + let mut name: String = format!("{server}_{tool}") + .chars() + .map(|c| { + if c.is_ascii_alphanumeric() || c == '_' || c == '-' { + c + } else { + '_' + } + }) + .collect(); + name.truncate(MAX_NAME_LEN); + name +} + +/// A single remote MCP tool exposed to the engine as a `Tool`. Holds a shared handle to the +/// server's `RunningService` (kept alive for the whole session so the child stays up). +struct McpTool { + /// Engine-facing name: sanitized `{server}_{tool}`; also the permission pattern. + full_name: String, + /// The server's own tool name, sent back verbatim in `call_tool`. + remote_name: String, + description: String, + parameters: serde_json::Value, + service: Arc>, +} + +#[async_trait] +impl Tool for McpTool { + fn name(&self) -> &str { + &self.full_name + } + + fn description(&self) -> &str { + &self.description + } + + fn parameters(&self) -> serde_json::Value { + self.parameters.clone() + } + + async fn execute( + &self, + input: serde_json::Value, + ctx: ToolCtx, + ) -> Result { + ctx.ask + .ask( + "mcp", + self.full_name.clone(), + self.full_name.clone(), + input.clone(), + ) + .await?; + + let arguments = match input { + serde_json::Value::Object(map) => Some(map), + serde_json::Value::Null => None, + other => { + return Err(ToolError::Invalid(format!( + "MCP tool arguments must be a JSON object, got {other}" + ))) + } + }; + + let result = self + .service + .peer() + .call_tool(CallToolRequestParam { + name: self.remote_name.clone().into(), + arguments, + }) + .await + .map_err(|e| ToolError::Other(e.to_string()))?; + + let mut text = String::new(); + let mut image_index = 0; + for content in &result.content { + match &content.raw { + RawContent::Text(t) => { + if !text.is_empty() { + text.push('\n'); + } + text.push_str(&t.text); + } + RawContent::Image(image) => { + let note = save_image(&ctx.data_dir, &self.full_name, image_index, image).await; + if !text.is_empty() { + text.push('\n'); + } + text.push_str(¬e); + image_index += 1; + } + RawContent::Resource(resource) => { + let embedded = resource_text(resource); + if !embedded.is_empty() { + if !text.is_empty() { + text.push('\n'); + } + text.push_str(&embedded); + } + } + } + } + + // MCP surfaces tool-level failures as `is_error` with the message in `content`; map + // that to a tool error so the model sees it as a failed call rather than a result. + if result.is_error.unwrap_or(false) { + return Err(ToolError::Other(if text.is_empty() { + "MCP tool reported an error".to_string() + } else { + text + })); + } + + Ok(ToolOutput::new(self.full_name.clone(), text)) + } +} + +/// Writes an image payload to the session data dir and returns a one-line note for the tool +/// output. Best-effort: a write failure still yields a note (without a path). +async fn save_image( + data_dir: &PathBuf, + tool_name: &str, + index: usize, + image: &rmcp::model::RawImageContent, +) -> String { + let ext = image.mime_type.rsplit('/').next().unwrap_or("bin"); + let file_name = format!("{tool_name}-image-{index}.{ext}.b64"); + let path = data_dir.join(&file_name); + let saved = tokio::fs::create_dir_all(data_dir).await.is_ok() + && tokio::fs::write(&path, &image.data).await.is_ok(); + if saved { + format!( + "[image: {} ({} base64 bytes) saved to {}]", + image.mime_type, + image.data.len(), + path.display() + ) + } else { + format!( + "[image: {} ({} base64 bytes, not saved)]", + image.mime_type, + image.data.len() + ) + } +} + +/// Best-effort text extraction from an embedded resource (text resources only in v1). +fn resource_text(resource: &rmcp::model::RawEmbeddedResource) -> String { + match &resource.resource { + rmcp::model::ResourceContents::TextResourceContents { text, .. } => text.clone(), + _ => String::new(), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn qualified_name_prefixes_and_sanitizes() { + assert_eq!(qualified_name("fs", "read_file"), "fs_read_file"); + assert_eq!( + qualified_name("my.server", "do/thing"), + "my_server_do_thing" + ); + } + + #[test] + fn qualified_name_caps_length() { + let long_tool = "t".repeat(100); + let name = qualified_name("srv", &long_tool); + assert_eq!(name.len(), MAX_NAME_LEN); + assert!(name.starts_with("srv_t")); + } +} diff --git a/crates/harness-mcp/tests/echo.rs b/crates/harness-mcp/tests/echo.rs new file mode 100644 index 0000000..526a44e --- /dev/null +++ b/crates/harness-mcp/tests/echo.rs @@ -0,0 +1,153 @@ +//! End-to-end MCP integration test: spawn a real stdio MCP server (a small Python fixture), +//! connect through the real rmcp client, and verify a discovered tool is callable and gated +//! behind the `mcp` permission key. This is the M5 milestone's ✅ for MCP. + +use std::collections::HashMap; +use std::sync::{Arc, Mutex}; + +use harness_core::event::{AppEvent, EventBus}; +use harness_core::permission::{PermissionReply, PermissionService}; +use harness_core::tool::{MetadataSink, PermissionHandle, ToolCtx, ToolError}; +use harness_core::types::{MessageId, SessionId}; +use harness_mcp::{connect_all, ServerConfig}; +use tokio_util::sync::CancellationToken; + +/// A permission frontend that replies `Once` to every ask and records the `permission`/`pattern` +/// of each, so a test can assert the call was actually gated. +fn recording_auto_approve( + bus: EventBus, + service: Arc, +) -> Arc>> { + let asks = Arc::new(Mutex::new(Vec::new())); + let asks_task = asks.clone(); + // Subscribe before spawning: a subscription created inside the task could miss the ask + // (tokio broadcast only delivers to receivers that exist at publish time). + let mut rx = bus.subscribe(); + tokio::spawn(async move { + while let Ok(event) = rx.recv().await { + if let AppEvent::PermissionAsked { request } = event { + asks_task + .lock() + .unwrap() + .push((request.permission.clone(), request.pattern.clone())); + service.reply(&request.id, PermissionReply::Once); + } + } + }); + asks +} + +struct Harness { + ctx_data_dir: std::path::PathBuf, + service: Arc, +} + +impl Harness { + fn ctx(&self) -> ToolCtx { + let (metadata, _rx) = MetadataSink::channel(); + ToolCtx { + session_id: SessionId::new(), + message_id: MessageId::new(), + call_id: "call_1".into(), + data_dir: self.ctx_data_dir.clone(), + cwd: std::env::temp_dir(), + cancel: CancellationToken::new(), + ask: PermissionHandle::new( + self.service.clone(), + SessionId::new(), + Vec::new(), + Arc::new(Mutex::new(Vec::new())), + CancellationToken::new(), + ), + metadata, + spawner: None, + context_reporter: None, + diagnostics: None, + } + } +} + +fn fixture_server() -> HashMap { + let script = concat!(env!("CARGO_MANIFEST_DIR"), "/tests/fixtures/echo_server.py"); + HashMap::from([( + "fix".to_string(), + ServerConfig { + command: "python3".to_string(), + args: vec![script.to_string()], + env: HashMap::new(), + }, + )]) +} + +#[tokio::test] +async fn discovers_and_calls_a_real_mcp_tool_with_permission() { + let tools = connect_all(fixture_server()).await; + let names: Vec<_> = tools.iter().map(|t| t.name().to_string()).collect(); + assert!( + names.contains(&"fix_echo".to_string()), + "expected fix_echo among {names:?}" + ); + assert!(names.contains(&"fix_boom".to_string())); + + let echo = tools.iter().find(|t| t.name() == "fix_echo").unwrap(); + // Schema passes through untouched from the server. + assert_eq!(echo.parameters()["properties"]["text"]["type"], "string"); + + let bus = EventBus::new(); + let service = Arc::new(PermissionService::new(bus.clone())); + let asks = recording_auto_approve(bus, service.clone()); + let dir = tempfile::tempdir().unwrap(); + let harness = Harness { + ctx_data_dir: dir.path().to_path_buf(), + service, + }; + + let out = echo + .execute(serde_json::json!({"text": "hi there"}), harness.ctx()) + .await + .expect("echo call succeeds"); + assert_eq!(out.output, "hi there"); + + // The call was gated on the `mcp` key with the qualified tool name as the pattern. + let recorded = asks.lock().unwrap().clone(); + assert_eq!(recorded, vec![("mcp".to_string(), "fix_echo".to_string())]); +} + +#[tokio::test] +async fn tool_error_result_maps_to_tool_error() { + let tools = connect_all(fixture_server()).await; + let boom = tools.iter().find(|t| t.name() == "fix_boom").unwrap(); + + let bus = EventBus::new(); + let service = Arc::new(PermissionService::new(bus.clone())); + let _asks = recording_auto_approve(bus, service.clone()); + let dir = tempfile::tempdir().unwrap(); + let harness = Harness { + ctx_data_dir: dir.path().to_path_buf(), + service, + }; + + let err = boom + .execute(serde_json::json!({}), harness.ctx()) + .await + .expect_err("boom reports an error result"); + match err { + ToolError::Other(msg) => assert_eq!(msg, "kaboom"), + other => panic!("expected ToolError::Other, got {other:?}"), + } +} + +#[tokio::test] +async fn a_failed_server_is_skipped_not_fatal() { + let servers = HashMap::from([( + "broken".to_string(), + ServerConfig { + command: "definitely-not-a-real-binary-xyz".to_string(), + args: vec![], + env: HashMap::new(), + }, + )]); + // No panic, no tools — the missing server is logged and skipped. + let tools = connect_all(servers).await; + assert!(tools.is_empty()); +} diff --git a/crates/harness-mcp/tests/fixtures/echo_server.py b/crates/harness-mcp/tests/fixtures/echo_server.py new file mode 100644 index 0000000..13253c4 --- /dev/null +++ b/crates/harness-mcp/tests/fixtures/echo_server.py @@ -0,0 +1,86 @@ +#!/usr/bin/env python3 +"""Minimal MCP stdio server fixture for harness-mcp integration tests. + +Speaks newline-delimited JSON-RPC (the framing rmcp's child-process transport uses) and +implements just enough of the protocol to be discovered and called: `initialize`, +`notifications/initialized`, `tools/list`, and `tools/call`. Exposes one tool, `echo`, +which returns its `text` argument, plus `boom`, which returns an error result. +""" +import json +import sys + +PROTOCOL_VERSION = "2024-11-05" + +TOOLS = [ + { + "name": "echo", + "description": "Returns the text it is given.", + "inputSchema": { + "type": "object", + "properties": {"text": {"type": "string"}}, + "required": ["text"], + }, + }, + { + "name": "boom", + "description": "Always fails.", + "inputSchema": {"type": "object", "properties": {}}, + }, +] + + +def reply(msg_id, result): + sys.stdout.write(json.dumps({"jsonrpc": "2.0", "id": msg_id, "result": result}) + "\n") + sys.stdout.flush() + + +def main(): + # readline() rather than `for line in sys.stdin`: the latter's read-ahead buffer blocks + # until it fills, which would stall the JSON-RPC handshake line-by-line. + while True: + line = sys.stdin.readline() + if line == "": # EOF: parent closed stdin + break + line = line.strip() + if not line: + continue + msg = json.loads(line) + method = msg.get("method") + msg_id = msg.get("id") + + if method == "initialize": + reply(msg_id, { + "protocolVersion": PROTOCOL_VERSION, + "capabilities": {"tools": {}}, + "serverInfo": {"name": "echo-fixture", "version": "0.1.0"}, + }) + elif method == "notifications/initialized": + pass # notification: no response + elif method == "tools/list": + reply(msg_id, {"tools": TOOLS}) + elif method == "tools/call": + params = msg.get("params") or {} + name = params.get("name") + args = params.get("arguments") or {} + if name == "echo": + reply(msg_id, { + "content": [{"type": "text", "text": args.get("text", "")}], + "isError": False, + }) + elif name == "boom": + reply(msg_id, { + "content": [{"type": "text", "text": "kaboom"}], + "isError": True, + }) + else: + reply(msg_id, { + "content": [{"type": "text", "text": f"unknown tool {name}"}], + "isError": True, + }) + elif msg_id is not None: + # Unknown request: empty result keeps the client happy. + reply(msg_id, {}) + + +if __name__ == "__main__": + main() 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..a4b3e3a --- /dev/null +++ b/crates/harness-tools/src/diagnostics.rs @@ -0,0 +1,73 @@ +//! 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..ec8ed6c 100644 --- a/crates/harness-tools/src/lib.rs +++ b/crates/harness-tools/src/lib.rs @@ -1,9 +1,11 @@ mod bash; +mod diagnostics; mod edit; mod glob; mod grep; mod paths; mod read; +mod skill; mod task; mod write; @@ -12,6 +14,7 @@ pub use edit::EditTool; pub use glob::GlobTool; pub use grep::GrepTool; pub use read::ReadTool; +pub use skill::SkillTool; pub use task::TaskTool; pub use write::WriteTool; @@ -35,3 +38,11 @@ pub fn register_builtins(registry: &mut ToolRegistry) { pub fn register_task_tool(registry: &mut ToolRegistry) { registry.register(Arc::new(TaskTool)); } + +/// Registers the `skill` tool (M5) over a loaded skill set. No-op when there are no skills, +/// so the tool is only advertised when something can be loaded. +pub fn register_skill_tool(registry: &mut ToolRegistry, skills: &[harness_core::config::SkillDef]) { + if !skills.is_empty() { + registry.register(Arc::new(SkillTool::new(skills))); + } +} 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/skill.rs b/crates/harness-tools/src/skill.rs new file mode 100644 index 0000000..518e65c --- /dev/null +++ b/crates/harness-tools/src/skill.rs @@ -0,0 +1,154 @@ +//! The `skill` tool (M5): the system prompt advertises each skill's name + description; when +//! the model decides a skill is relevant it calls this tool with the skill name to pull the +//! full instructions on demand. See `docs/09-integrations.md`. + +use std::collections::HashMap; + +use async_trait::async_trait; +use harness_core::config::SkillDef; +use harness_core::tool::{Tool, ToolCtx, ToolError, ToolOutput}; +use schemars::JsonSchema; +use serde::Deserialize; + +#[derive(Debug, Deserialize, JsonSchema)] +struct SkillParams { + /// The name of the skill to load, as advertised in the system prompt. + name: String, +} + +/// Serves skill bodies by name. Built from the loaded skill set; if there are no skills the +/// caller simply doesn't register the tool. +pub struct SkillTool { + /// name → (description, body). + skills: HashMap, + /// Sorted names, for a stable "unknown skill" hint. + names: Vec, + description: String, +} + +impl SkillTool { + pub fn new(skills: &[SkillDef]) -> Self { + let mut names: Vec = skills.iter().map(|s| s.name.clone()).collect(); + names.sort(); + let map = skills + .iter() + .map(|s| (s.name.clone(), (s.description.clone(), s.body.clone()))) + .collect(); + let description = format!( + "Load the full instructions for a named skill before doing the related work. \ + Available skills: {}.", + names.join(", ") + ); + Self { + skills: map, + names, + description, + } + } +} + +#[async_trait] +impl Tool for SkillTool { + fn name(&self) -> &str { + "skill" + } + + fn description(&self) -> &str { + &self.description + } + + fn parameters(&self) -> serde_json::Value { + serde_json::to_value(schemars::schema_for!(SkillParams)).unwrap() + } + + async fn execute( + &self, + input: serde_json::Value, + _ctx: ToolCtx, + ) -> Result { + let params: SkillParams = + serde_json::from_value(input).map_err(|e| ToolError::Invalid(e.to_string()))?; + match self.skills.get(¶ms.name) { + Some((_description, body)) => Ok(ToolOutput::new( + format!("skill: {}", params.name), + body.clone(), + )), + None => Err(ToolError::Invalid(format!( + "unknown skill {:?}; available: {}", + params.name, + self.names.join(", ") + ))), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use harness_core::event::EventBus; + use harness_core::permission::{spawn_auto_approve, PermissionService}; + use harness_core::tool::{MetadataSink, PermissionHandle}; + use harness_core::types::SessionId; + use std::sync::{Arc, Mutex}; + use tokio_util::sync::CancellationToken; + + fn ctx() -> ToolCtx { + let bus = EventBus::new(); + let service = Arc::new(PermissionService::new(bus.clone())); + spawn_auto_approve(bus, service.clone()); + let (metadata, _rx) = MetadataSink::channel(); + ToolCtx { + session_id: SessionId::new(), + message_id: harness_core::types::MessageId::new(), + call_id: "c1".into(), + data_dir: std::env::temp_dir(), + cwd: std::env::temp_dir(), + cancel: CancellationToken::new(), + ask: PermissionHandle::new( + service, + SessionId::new(), + Vec::new(), + Arc::new(Mutex::new(Vec::new())), + CancellationToken::new(), + ), + metadata, + spawner: None, + context_reporter: None, + diagnostics: None, + } + } + + fn sample() -> Vec { + vec![SkillDef { + name: "formatter".into(), + description: "format code".into(), + body: "Run cargo fmt.".into(), + }] + } + + #[tokio::test] + async fn returns_skill_body_by_name() { + let tool = SkillTool::new(&sample()); + let out = tool + .execute(serde_json::json!({"name": "formatter"}), ctx()) + .await + .unwrap(); + assert_eq!(out.output, "Run cargo fmt."); + } + + #[tokio::test] + async fn unknown_skill_is_an_input_error() { + let tool = SkillTool::new(&sample()); + let err = tool + .execute(serde_json::json!({"name": "nope"}), ctx()) + .await + .unwrap_err(); + assert!(matches!(err, ToolError::Invalid(_))); + } + + #[test] + fn description_lists_available_skills() { + let tool = SkillTool::new(&sample()); + assert!(tool.description().contains("formatter")); + } +} 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, } } diff --git a/crates/harness-tui/src/app.rs b/crates/harness-tui/src/app.rs index 8449bc3..d79adb3 100644 --- a/crates/harness-tui/src/app.rs +++ b/crates/harness-tui/src/app.rs @@ -31,7 +31,7 @@ pub struct App { impl App { pub async fn new(cwd: PathBuf) -> anyhow::Result { - let engine = EngineHandle::init(cwd)?; + let engine = EngineHandle::init(cwd).await?; let bus_rx = engine.bus().subscribe(); let config = engine.config(); diff --git a/crates/harness-tui/src/input.rs b/crates/harness-tui/src/input.rs index e4309f8..7e9fd4b 100644 --- a/crates/harness-tui/src/input.rs +++ b/crates/harness-tui/src/input.rs @@ -9,7 +9,16 @@ use crate::state::{AppState, ModalState}; #[derive(Debug)] pub enum InputAction { None, - Submit { text: String }, + Submit { + text: String, + }, + /// A user-defined slash command: `name` (no leading `/`), its `args`, and the `raw` input + /// to fall back to submitting verbatim if no such command is defined. + RunCommand { + name: String, + args: String, + raw: String, + }, Abort, Quit, LoadSessions, @@ -176,7 +185,13 @@ fn parse_slash_command(text: &str) -> Option { "/sessions" => Some(InputAction::LoadSessions), "/jobs" => Some(InputAction::OpenJobs), "/quit" => Some(InputAction::Quit), - _ => None, + // Any other `/word` is treated as a user-defined command, resolved against the engine's + // loaded commands when applied; if none matches, the raw text is submitted as-is. + other => Some(InputAction::RunCommand { + name: other.trim_start_matches('/').to_string(), + args: rest, + raw: trimmed.to_string(), + }), } } @@ -190,6 +205,30 @@ pub async fn apply_action(action: InputAction, state: &mut AppState, engine: &En } } } + InputAction::RunCommand { name, args, raw } => { + let Some(session_id) = state.session_id.clone() else { + return; + }; + match engine.command(&name) { + Some(cmd) => { + let text = cmd.expand(&args); + // A command may switch model/agent for this one run only. + let model_ref = cmd.model.clone().unwrap_or_else(|| state.model_ref.clone()); + if let Err(e) = engine + .prompt_with(session_id, text, &model_ref, cmd.agent.as_deref()) + .await + { + tracing::error!(error = %e, "command prompt failed"); + } + } + // Unknown command: submit the original text as an ordinary message. + None => { + if let Err(e) = engine.prompt(session_id, raw, &state.model_ref).await { + tracing::error!(error = %e, "prompt failed"); + } + } + } + } InputAction::Abort => { if let Some(session_id) = state.session_id.clone() { engine.abort(&session_id); @@ -317,4 +356,33 @@ mod tests { assert!(state.dirty, "a keystroke must request a redraw"); assert_eq!(state.input.lines().join("\n"), "x"); } + + #[test] + fn builtin_slash_commands_still_parse() { + assert!(matches!( + parse_slash_command("/new"), + Some(InputAction::NewSession) + )); + assert!(matches!( + parse_slash_command("/model openai/gpt-5"), + Some(InputAction::SetModel(m)) if m == "openai/gpt-5" + )); + } + + #[test] + fn unknown_slash_becomes_a_run_command() { + match parse_slash_command("/deploy prod now") { + Some(InputAction::RunCommand { name, args, raw }) => { + assert_eq!(name, "deploy"); + assert_eq!(args, "prod now"); + assert_eq!(raw, "/deploy prod now"); + } + other => panic!("expected RunCommand, got {other:?}"), + } + } + + #[test] + fn non_slash_text_is_not_a_command() { + assert!(parse_slash_command("hello world").is_none()); + } } diff --git a/crates/harness-tui/src/main.rs b/crates/harness-tui/src/main.rs index 7b65de4..07c7a5c 100644 --- a/crates/harness-tui/src/main.rs +++ b/crates/harness-tui/src/main.rs @@ -50,7 +50,7 @@ async fn run_headless(args: &[String]) -> i32 { } }; - let app = match harness_app::App::init(cwd) { + let app = match harness_app::App::init(cwd).await { Ok(app) => app, Err(e) => { eprintln!("error: {e}");