diff --git a/builtin/runtime/lsp.lua b/builtin/runtime/lsp.lua index 6a837f6..fd44f1a 100644 --- a/builtin/runtime/lsp.lua +++ b/builtin/runtime/lsp.lua @@ -1584,10 +1584,10 @@ local function server_attempt(sid) local skey = tostring(sid) for _, info in ipairs(pmacs.lsp.list()) do if tostring(info.id) == skey then - return info.attempt or 0, info.state and info.state.kind + return info.attempt or 0 end end - return nil, nil + return nil end -- fn(sid, params); persistent, fires for every server. @@ -1605,6 +1605,15 @@ end -- fn(result, err); ONE-SHOT, keyed to the exact request. -- `request_id` is what `pmacs.lsp.send_request` returned. +-- +-- **Register only against a server with an attached buffer.** The drain +-- that delivers replies visits only sids present in `attachments`, so a +-- one-shot on an unattached server will not fire on its reply — the +-- reply sits in that server's queue and the handler is invoked only when +-- the purge below decides the server is gone. That is fire-on-death, not +-- fire-on-reply, and it looks exactly like a hung request while +-- debugging. The attach path is the ordinary way to get a sid; a +-- hand-spawned one from `init.lua` is the case to watch. function pmacs.lsp.on_response(sid, request_id, fn) if not sid or type(request_id) ~= "number" or type(fn) ~= "function" then error("pmacs.lsp.on_response(sid, request_id, fn): want sid, number, function") @@ -1641,8 +1650,13 @@ local function deliver_response(sid, ev) if not pend then return end local entry = pend[ev.request_id] if not entry then return end - -- Removed BEFORE invocation: a handler that raises must not be - -- re-entered by a later event carrying the same id. + -- Removed UNCONDITIONALLY, so a handler that raises is still retired + -- and cannot be invoked a second time by the purge. Removing first is + -- the defensive order and costs nothing, but it is not what defends + -- against re-invocation: `pcall` catches the raise either way, so + -- before-vs-after is unobservable without a re-entrant drain. The + -- reachable bug is gating removal on a clean return, which acceptance + -- 32 bites (2 != 1). pend[ev.request_id] = nil if next(pend) == nil then pending_responses[skey] = nil end local ok, err = pcall(entry.fn, ev.result, ev.error) diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index 369b810..ac33537 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -6550,9 +6550,18 @@ pub fn install_async( // symlink is ordinary, and raising would surface through // `resolve_root_fn`'s pcall as a config bug, which it is // not. + // + // `to_str`, NOT `display()`. A resolution that lands on + // non-UTF-8 bytes has no faithful string form, and + // `display()` would substitute U+FFFD and hand back a + // path that does not exist on disk — strictly worse than + // nil here, because this value becomes a server-affinity + // key via `file_uri_for` and would silently fail to + // round-trip. Unrepresentable is a decline, matching how + // the fs layer already treats non-UTF-8 symlink targets. Ok(std::fs::canonicalize(&path) .ok() - .map(|p| p.display().to_string())) + .and_then(|p| p.to_str().map(str::to_owned))) })?, )?; pmacs.set("_fs", fs_priv)?; diff --git a/tests/lsp_dispatch_seams_acceptance.rs b/tests/lsp_dispatch_seams_acceptance.rs index cb9acff..bc0b40d 100644 --- a/tests/lsp_dispatch_seams_acceptance.rs +++ b/tests/lsp_dispatch_seams_acceptance.rs @@ -493,12 +493,6 @@ fn acc34_purge_reaches_a_server_that_is_in_no_attachment() { ); settle(&mut state); - let attached: i64 = eval( - &state, - "local n = 0 for _ in pairs(_G) do n = n + 1 end return n", - ); - assert!(attached > 0, "lua globals are readable"); - exec( &state, r#" @@ -528,6 +522,7 @@ fn acc34_purge_reaches_a_server_that_is_in_no_attachment() { // --------------------------------------------------------------------------- #[test] +#[cfg(unix)] fn acc34a_canonicalize_resolves_symlinks_and_dot_segments() { let fx = Fixture::new(); fx.write("pkg/sub/a.txt", "x\n"); @@ -576,6 +571,7 @@ fn server_count(state: &EditorState) -> i64 { } #[test] +#[cfg(unix)] fn acc34b_canonicalizing_resolver_reuses_one_server_across_a_symlink() { let fx = Fixture::new(); fx.write("proj/Cargo.toml", "[package]\nname = \"p\"\n"); @@ -627,6 +623,7 @@ fn acc34b_canonicalizing_resolver_reuses_one_server_across_a_symlink() { } #[test] +#[cfg(unix)] fn acc34b_falsified_by_a_resolver_that_skips_canonicalization() { let fx = Fixture::new(); fx.write("proj/Cargo.toml", "[package]\nname = \"p\"\n"); @@ -671,3 +668,47 @@ fn acc34b_falsified_by_a_resolver_that_skips_canonicalization() { spawn two servers — this is what 34b's positive case rules out" ); } + +// --------------------------------------------------------------------------- +// Acceptance 34a, non-UTF-8 arm — an unrepresentable resolution declines +// rather than returning a lossy string. +// +// Review finding on PR #167: `display().to_string()` substitutes U+FFFD, +// which would hand back a path that does not exist on disk. That is +// strictly worse than nil here, because the value becomes a +// server-affinity key via `file_uri_for` and would silently fail to +// round-trip. Bites against the `display()` form, which returns a +// non-nil string for this fixture. +// --------------------------------------------------------------------------- + +#[test] +#[cfg(unix)] +fn acc34a_canonicalize_declines_a_non_utf8_resolution() { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt as _; + + let fx = Fixture::new(); + // 0xFF is not valid UTF-8 in any position. + let raw = OsStr::from_bytes(b"bad-\xffname"); + let target = fx.root.join(raw); + std::fs::write(&target, "x\n").unwrap(); + // Reached through an ASCII symlink, so the *input* is representable + // and only the resolved output is not — which is the case + // `to_str()` has to catch and a UTF-8-only input check would miss. + let link = fx.dir("ascii-link"); + std::os::unix::fs::symlink(&target, &link).unwrap(); + + let state = editor(); + let got: String = eval( + &state, + &format!( + "return tostring(pmacs.fs.canonicalize(\"{}\"))", + lua_str(&link) + ), + ); + assert_eq!( + got, "nil", + "a resolution that lands on non-UTF-8 bytes must decline, not \ + return a U+FFFD-substituted path that exists nowhere" + ); +}