Skip to content

Commit bba4943

Browse files
author
Yogthos
committed
LSP Phase 7: CLI + config plumbing through build_channels
src/config/mod.rs - LspConfig: untagged enum, accepts `true`/`false` OR a map of per-server overrides. - LspServerConfig: all-optional fields (command, extensions, env, initialization, disabled). Missing fields fall back to built-in. - Config gains an optional `lsp` field; default-None (which the CLI resolver treats as enabled-with-built-ins). - 4 config-parse tests: bool form, per-server map form, absent-is-None, mixed command/disabled entries. src/cli.rs - `--no-lsp` flag. - `resolve_lsp_enabled(cli, cfg)` resolver: no-tools or no-lsp turns it off; otherwise the config bool (default true) decides. src/lsp/spawn.rs - ProcessSpawner::default_commands(): the 4 built-in commands for v1 servers (rust-analyzer, typescript-language-server --stdio, pyright-langserver --stdio, clojure-lsp). User config overrides are merged in main::compile_lsp_commands. src/main.rs - build_channels grows a 10th return slot: Option<Arc<LspManager>>. Constructed when lsp_enabled, via ProcessSpawner with merged defaults + user overrides. - compile_lsp_commands: starts from defaults, applies per-server overrides (disabled removes, command replaces, env/init merge). - Threaded through all 3 `build_agent` call sites + run_interactive. src/provider.rs + src/agent/builder.rs - New `lsp_manager: Option<Arc<LspManager>>` arg on build_agent / build_agent_inner. Threaded down to the WriteTool / EditTool / ReadTool constructors (which Phase 6 prepared the field for) AND to the LspTool registration (Phase 5's tool now actually gets attached to the agent when lsp_manager is present). src/ui/mod.rs + src/ui/slash.rs + src/extras/acp/mod.rs - run_interactive accepts and forwards lsp_manager. - Plan-switch rebuild passes the live lsp_manager.clone() (so the rebuilt agent still has LSP tools — same pattern as the bg-store fix from Phase 5). - 7 slash.rs sub-rebuild sites + 2 ui/mod.rs prompt-switch sites pass `None` for lsp_manager (intentional — these rebuild for /model, /context-reset etc., which don't need to re-attach LSP). - ACP path passes `None`. src/agent/tools/lsp.rs - Drops the Phase-5 `#![allow(dead_code)]` now that builder.rs wires it. Phase 6: 116, Phase 7: +4 config tests -> 120 LSP tests. Suite: 422 -> 426.
1 parent 0fb964e commit bba4943

11 files changed

Lines changed: 282 additions & 15 deletions

File tree

src/agent/builder.rs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ pub async fn build_agent_inner<M: CompletionModel + 'static>(
3737
question_tx: Option<QuestionSender>,
3838
plan_tx: Option<PlanSwitchSender>,
3939
bg_store: Option<BackgroundStore>,
40+
lsp_manager: Option<std::sync::Arc<crate::lsp::manager::LspManager>>,
4041
sandbox: Sandbox,
4142
parent_model: Option<AnyModel>,
4243
#[cfg(feature = "mcp")] mcp_manager: Option<&McpClientManager>,
@@ -140,22 +141,21 @@ pub async fn build_agent_inner<M: CompletionModel + 'static>(
140141
permission.clone(),
141142
ask_tx.clone(),
142143
cache.clone(),
143-
// Phase 7 will populate this from build_channels.
144-
None,
144+
lsp_manager.clone(),
145145
)),
146146
Box::new(tools::WriteTool::with_cache(
147147
permission.clone(),
148148
ask_tx.clone(),
149149
plan_file.clone(),
150150
cache.clone(),
151-
None,
151+
lsp_manager.clone(),
152152
)),
153153
Box::new(tools::EditTool::with_cache(
154154
permission.clone(),
155155
ask_tx.clone(),
156156
plan_file.clone(),
157157
cache.clone(),
158-
None,
158+
lsp_manager.clone(),
159159
)),
160160
Box::new(tools::BashTool::with_cache(
161161
permission.clone(),
@@ -262,6 +262,17 @@ pub async fn build_agent_inner<M: CompletionModel + 'static>(
262262
builder = builder.tools(vec![task_tool, status_tool]);
263263
}
264264

265+
if let Some(manager) = &lsp_manager {
266+
let cwd = std::env::current_dir().unwrap_or_else(|_| ".".into());
267+
let lsp_tool = Box::new(tools::LspTool::new(
268+
permission.clone(),
269+
ask_tx.clone(),
270+
manager.clone(),
271+
cwd,
272+
)) as Box<dyn rig::tool::ToolDyn>;
273+
builder = builder.tools(vec![lsp_tool]);
274+
}
275+
265276
#[cfg(feature = "mcp")]
266277
if let Some(manager) = &mcp_manager {
267278
let mcp_tools = manager

src/agent/tools/lsp.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,6 @@
44
//! tool, one `operation` parameter; the agent picks which LSP capability to
55
//! invoke. Mirrors opencode's `tool/lsp.ts` surface so the agent's mental
66
//! model carries between the two.
7-
//!
8-
//! Phase 5 lands the tool + its tests. The builder.rs wiring that actually
9-
//! attaches it to a running agent comes in Phase 7 (config + CLI plumbing).
10-
//! Until then the symbols here are unused outside the test module — the
11-
//! crate-wide `#[allow(dead_code)]` in `lsp/mod.rs` doesn't extend here, so
12-
//! the tool's items carry their own `#[allow(dead_code)]` to keep the
13-
//! warning surface clean.
14-
#![allow(dead_code)]
157
168
use std::path::{Path, PathBuf};
179
use std::sync::Arc;

src/agent/tools/mod.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,6 @@ pub use find_files::FindFilesTool;
3030
pub use glob::GlobTool;
3131
pub use grep::GrepTool;
3232
pub use list_dir::ListDirTool;
33-
// Phase 5 lands the tool; builder.rs wiring is Phase 7.
34-
#[allow(unused_imports)]
3533
pub use lsp::LspTool;
3634
pub use memory::MemoryTool;
3735
pub use plan::{PlanEnterTool, PlanExitTool};

src/cli.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,12 @@ pub struct Cli {
4848
#[arg(long = "no-tools", help = "Disable all tools")]
4949
pub no_tools: bool,
5050

51+
#[arg(
52+
long = "no-lsp",
53+
help = "Disable LSP integration (no diagnostics on edit/write, no `lsp` agent tool)"
54+
)]
55+
pub no_lsp: bool,
56+
5157
#[arg(long = "no-color", help = "Disable colored TUI output")]
5258
pub no_color: bool,
5359

@@ -161,6 +167,16 @@ impl Cli {
161167
self.no_tools || cfg.no_tools.unwrap_or(false)
162168
}
163169

170+
pub fn resolve_lsp_enabled(&self, cfg: &config::Config) -> bool {
171+
if self.no_lsp || self.no_tools {
172+
return false;
173+
}
174+
match &cfg.lsp {
175+
Some(c) => c.is_enabled(),
176+
None => true, // default-on
177+
}
178+
}
179+
164180
pub fn resolve_sandbox(&self, cfg: &config::Config) -> bool {
165181
self.sandbox || cfg.sandbox.unwrap_or(false)
166182
}

src/config/mod.rs

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,57 @@ pub struct ToolsConfig {
2525
pub webfetch: Option<bool>,
2626
}
2727

28+
/// Per-server LSP configuration. All fields optional — unspecified fields
29+
/// fall back to the built-in defaults for the given `server_id`.
30+
///
31+
/// Two forms are accepted:
32+
/// - `{ "disabled": true }` to turn off a built-in server entirely.
33+
/// - any subset of `{ command, extensions, env, initialization, disabled }`
34+
/// to override pieces of the default.
35+
#[derive(Debug, Default, Clone, Deserialize)]
36+
#[serde(default)]
37+
pub struct LspServerConfig {
38+
pub command: Option<Vec<String>>,
39+
pub extensions: Option<Vec<String>>,
40+
pub env: Option<HashMap<String, String>>,
41+
pub initialization: Option<serde_json::Value>,
42+
pub disabled: Option<bool>,
43+
}
44+
45+
/// `lsp = true` → enable built-in servers with default commands.
46+
/// `lsp = false` → disable LSP entirely.
47+
/// `lsp = { server-id = { … } }` → enable defaults, overriding the named
48+
/// servers with the provided config.
49+
#[derive(Debug, Clone, Deserialize)]
50+
#[serde(untagged)]
51+
pub enum LspConfig {
52+
Enabled(bool),
53+
Servers(HashMap<String, LspServerConfig>),
54+
}
55+
56+
impl LspConfig {
57+
/// `true` when LSP should be on. Defaults to enabled.
58+
pub fn is_enabled(&self) -> bool {
59+
match self {
60+
LspConfig::Enabled(b) => *b,
61+
LspConfig::Servers(_) => true,
62+
}
63+
}
64+
65+
/// Per-server overrides keyed by server id. Empty when LSP is a bool.
66+
pub fn server_overrides(&self) -> &HashMap<String, LspServerConfig> {
67+
match self {
68+
LspConfig::Enabled(_) => {
69+
// Empty borrow without allocating per-call.
70+
static EMPTY: std::sync::OnceLock<HashMap<String, LspServerConfig>> =
71+
std::sync::OnceLock::new();
72+
EMPTY.get_or_init(HashMap::new)
73+
}
74+
LspConfig::Servers(map) => map,
75+
}
76+
}
77+
}
78+
2879
#[derive(Debug, Default, Deserialize)]
2980
#[serde(default)]
3081
pub struct Config {
@@ -51,6 +102,7 @@ pub struct Config {
51102
pub tool_result_max_chars: Option<usize>,
52103
pub default_prompt: Option<String>,
53104
pub tools: Option<ToolsConfig>,
105+
pub lsp: Option<LspConfig>,
54106
#[cfg(feature = "mcp")]
55107
pub mcp_servers: Option<HashMap<String, McpServerConfig>>,
56108

@@ -141,3 +193,69 @@ pub fn load() -> Config {
141193

142194
cfg
143195
}
196+
197+
#[cfg(test)]
198+
mod tests {
199+
use super::*;
200+
201+
#[test]
202+
fn lsp_config_parses_as_bool() {
203+
let cfg: Config = serde_json::from_str(r#"{"lsp": true}"#).unwrap();
204+
assert!(cfg.lsp.unwrap().is_enabled());
205+
206+
let cfg: Config = serde_json::from_str(r#"{"lsp": false}"#).unwrap();
207+
assert!(!cfg.lsp.unwrap().is_enabled());
208+
}
209+
210+
#[test]
211+
fn lsp_config_parses_as_per_server_map() {
212+
let raw = r#"{
213+
"lsp": {
214+
"rust": { "command": ["my-rust-analyzer", "--my-arg"] },
215+
"typescript": { "disabled": true }
216+
}
217+
}"#;
218+
let cfg: Config = serde_json::from_str(raw).unwrap();
219+
let overrides = cfg.lsp.as_ref().unwrap().server_overrides();
220+
assert_eq!(overrides.len(), 2);
221+
assert_eq!(
222+
overrides["rust"].command.as_ref().unwrap(),
223+
&vec!["my-rust-analyzer".to_string(), "--my-arg".to_string()]
224+
);
225+
assert_eq!(overrides["typescript"].disabled, Some(true));
226+
}
227+
228+
// Regression: when lsp is omitted entirely, default is "enabled with
229+
// built-in commands" — the CLI's resolve_lsp_enabled handles that.
230+
// Config-side, an absent value parses to `None`.
231+
#[test]
232+
fn absent_lsp_config_is_none() {
233+
let cfg: Config = serde_json::from_str(r#"{"model": "foo"}"#).unwrap();
234+
assert!(cfg.lsp.is_none());
235+
}
236+
237+
// Regression: a config that mixes overrides for valid server ids
238+
// (rust) with disabled-only entries (typescript) must parse cleanly.
239+
#[test]
240+
fn lsp_config_mixes_command_and_disabled_entries() {
241+
let raw = r#"{
242+
"lsp": {
243+
"rust": { "command": ["rust-analyzer"], "env": {"RUST_LOG": "info"} },
244+
"typescript": { "disabled": true }
245+
}
246+
}"#;
247+
let cfg: Config = serde_json::from_str(raw).unwrap();
248+
let overrides = cfg.lsp.as_ref().unwrap().server_overrides();
249+
assert!(overrides["rust"].command.is_some());
250+
assert_eq!(
251+
overrides["rust"]
252+
.env
253+
.as_ref()
254+
.unwrap()
255+
.get("RUST_LOG")
256+
.unwrap(),
257+
"info"
258+
);
259+
assert_eq!(overrides["typescript"].disabled, Some(true));
260+
}
261+
}

src/extras/acp/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@ async fn run_prompt(
170170
None,
171171
None,
172172
None,
173+
None,
173174
sandbox,
174175
#[cfg(feature = "mcp")]
175176
None::<&crate::extras::mcp::McpClientManager>,

src/lsp/spawn.rs

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,50 @@ impl ProcessSpawner {
6262
pub fn new(commands: std::collections::HashMap<String, ProcessCommand>) -> Self {
6363
Self { commands }
6464
}
65+
66+
/// Built-in command defaults for the v1 server set. Each entry is the
67+
/// program name + argv to launch. The actual `tokio::process::Command`
68+
/// resolves `program` via PATH at spawn time.
69+
pub fn default_commands() -> std::collections::HashMap<String, ProcessCommand> {
70+
let mut m = std::collections::HashMap::new();
71+
m.insert(
72+
"rust".to_string(),
73+
ProcessCommand {
74+
program: PathBuf::from("rust-analyzer"),
75+
args: vec![],
76+
env: vec![],
77+
init_options: Value::Null,
78+
},
79+
);
80+
m.insert(
81+
"typescript".to_string(),
82+
ProcessCommand {
83+
program: PathBuf::from("typescript-language-server"),
84+
args: vec!["--stdio".to_string()],
85+
env: vec![],
86+
init_options: Value::Null,
87+
},
88+
);
89+
m.insert(
90+
"pyright".to_string(),
91+
ProcessCommand {
92+
program: PathBuf::from("pyright-langserver"),
93+
args: vec!["--stdio".to_string()],
94+
env: vec![],
95+
init_options: Value::Null,
96+
},
97+
);
98+
m.insert(
99+
"clojure-lsp".to_string(),
100+
ProcessCommand {
101+
program: PathBuf::from("clojure-lsp"),
102+
args: vec![],
103+
env: vec![],
104+
init_options: Value::Null,
105+
},
106+
);
107+
m
108+
}
65109
}
66110

67111
impl Spawner for ProcessSpawner {

0 commit comments

Comments
 (0)