diff --git a/builtin/runtime/lsp.lua b/builtin/runtime/lsp.lua index 3aca1ac..6021134 100644 --- a/builtin/runtime/lsp.lua +++ b/builtin/runtime/lsp.lua @@ -540,7 +540,7 @@ end -- resolver can never serve a root the previous one computed. local root_resolver_memo = setmetatable({}, { __mode = "k" }) -local function resolve_root_fn(resolver, path) +local function resolve_root_fn(language, resolver, path) local dir = dir_of(path) if not dir then return nil end local memo = root_resolver_memo[resolver] @@ -555,7 +555,36 @@ local function resolve_root_fn(resolver, path) return hit or nil end local ok, resolved = pcall(resolver, path) - if not ok or type(resolved) ~= "string" then resolved = nil end + -- COHERENCE §1.2: background wiring must not DISCARD a failure. A + -- resolver that raises, or that returns something other than a string + -- or nil, is a config bug — and the memo below would otherwise bury + -- it permanently for this directory, so it is never observed again. + -- Returning nil is the documented decline and stays silent. + local failure + if not ok then + failure = "raised: " .. tostring(resolved) + elseif resolved ~= nil and type(resolved) ~= "string" then + failure = "returned a " .. type(resolved) .. "; want string or nil" + end + if failure then + local msg = string.format( + "LSP: %s root resolver for %s %s", language, dir, failure) + -- Report on the channel that EXISTS. `pmacs.error` is referenced by + -- fifteen guarded call sites across the runtime and is defined + -- nowhere in production (only by a test stub in `src/editor.rs`), so + -- `if pmacs.error then ...` alone would be a sixteenth report that + -- never fires — the unwired-guard shape, not a fix for it. The + -- status line is what lsp.lua already uses for every other LSP + -- error. The `pmacs.error` arm rides along so this upgrades for free + -- if that channel is ever built. + -- + -- Both reports are pcall'd: a broken reporting channel must not turn + -- a declined root into a failed attach. + pcall(pmacs.editor.set_status, msg) + if pmacs.error then pcall(pmacs.error, msg) end + resolved = nil + end + if type(resolved) ~= "string" then resolved = nil end memo[dir] = resolved or false return resolved end @@ -570,7 +599,7 @@ local function project_root_for(language, path) end if not path then return nil, nil end if configured then - local resolved = resolve_root_fn(configured, path) + local resolved = resolve_root_fn(language, configured, path) if resolved then return resolved, "config" end end local ok, det = pcall(pmacs.project.detect, path) diff --git a/tests/lsp_multi_root_acceptance.rs b/tests/lsp_multi_root_acceptance.rs index 518c964..39ac68f 100644 --- a/tests/lsp_multi_root_acceptance.rs +++ b/tests/lsp_multi_root_acceptance.rs @@ -156,6 +156,10 @@ fn rows(state: &EditorState) -> Vec { } } +fn status(state: &EditorState) -> String { + state.core.borrow().status.clone() +} + fn count(state: &EditorState) -> usize { let n: i64 = eval(state, "return #pmacs.lsp.list()"); usize::try_from(n).expect("server count is non-negative") @@ -613,3 +617,88 @@ fn config_root_false_reads_as_unset_and_detection_still_wins() { "`false` must not become a root; detection still wins" ); } + +/// COHERENCE §1.2: background wiring must leave an attributed trace +/// rather than discard a failure. A throwing root resolver is a config +/// bug, and the per-directory memo would otherwise bury it permanently. +/// +/// The bite: drop the reporting arm and `*errors*` stays empty while the +/// attach still succeeds — the exact silence §1.2 names as the canonical +/// anti-pattern, in the function it cites. +#[test] +fn a_throwing_root_resolver_leaves_an_attributed_trace() { + let fx = Fixture::new(); + fx.write("proj/Cargo.toml", "[package]\nname = \"p\"\n"); + let file = fx.write("proj/src/main.rs", "fn main() {}\n"); + let mut state = editor(); + fx.bind(&state); + exec( + &state, + &format!( + r#" + pmacs.lsp.config.rust = {{ + command = "{}", + root = function(_) error("resolver blew up") end, + }} + "#, + fake_lsp_path() + ), + ); + open(&state, &file); + settle(&mut state); + + let msg = status(&state); + assert!( + msg.contains("root resolver"), + "a raising resolver must leave an attributed trace; got: {msg:?}" + ); + assert!( + msg.contains("rust"), + "the trace must name the language that owns it; got: {msg:?}" + ); + assert!( + msg.contains("resolver blew up"), + "the underlying error text must survive; got: {msg:?}" + ); + + // ...and the failure must degrade to a decline, not a failed attach: + // detection still wins and the buffer still gets its server. + let rows = rows(&state); + assert_eq!(rows.len(), 1, "the attach must still succeed: {rows:?}"); + assert_eq!( + rows[0].split('|').nth(1).unwrap(), + file_uri(&fx.dir("proj")), + "a declining resolver falls through to the marker walk" + ); +} + +/// The decline path stays silent. Without this, "report failures" could +/// be satisfied by reporting *every* resolution, which would spam +/// `*errors*` on every attach in a Lean project. +#[test] +fn a_resolver_returning_nil_declines_silently() { + let fx = Fixture::new(); + fx.write("proj/Cargo.toml", "[package]\nname = \"p\"\n"); + let file = fx.write("proj/src/main.rs", "fn main() {}\n"); + let mut state = editor(); + fx.bind(&state); + exec( + &state, + &format!( + "pmacs.lsp.config.rust = {{ command = \"{}\", root = function(_) return nil end }}", + fake_lsp_path() + ), + ); + open(&state, &file); + settle(&mut state); + + let msg = status(&state); + assert!( + !msg.contains("root resolver"), + "returning nil is the documented decline, not a failure; got: {msg:?}" + ); + assert_eq!( + rows(&state)[0].split('|').nth(1).unwrap(), + file_uri(&fx.dir("proj")) + ); +}