From 3f03ef2ae257fe556fa4e2b02db3011e2f22d5bb Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Tue, 19 May 2026 13:58:21 -0400 Subject: [PATCH 1/2] Fix: normalize buffer paths to absolute before they become identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A buffer opened by a relative or `~`-prefixed path (e.g. `pmacs ipc.cpp` from within the project) kept that literal string as its file_path. The LSP layer turns file_path into a `file://` URI by straight prefixing, so `ipc.cpp` became `file://ipc.cpp` — host `ipc.cpp`, empty path — which clangd rejects with `-32602 unresolvable URI at (root).textDocument.uri`, breaking every request for the buffer. Normalize at the single chokepoint EditorCore::set_buffer_path (CLI open, Lua find-file, WorkspaceEdit rename ops all flow through it): expand a leading `~`/`~/…` against $HOME, join onto cwd if relative, then fold `.`/`..` lexically. No fs access / no symlink resolution, so a not-yet-created "[new file]" buffer and already-absolute tempdir paths are unchanged (acceptance tests' exact-path asserts still hold). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/editor_core.rs | 132 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 132 insertions(+) diff --git a/src/editor_core.rs b/src/editor_core.rs index 1ea8cc3..af54432 100644 --- a/src/editor_core.rs +++ b/src/editor_core.rs @@ -333,7 +333,18 @@ impl EditorCore { /// Bind a path (and clear metadata) on a specific buffer. Used by /// file open / `pmacs.buffer.from_file`. + /// + /// The path is normalized to an absolute, lexically-clean form + /// first ([`normalize_buffer_path`]). This is the single seam + /// every buffer identity flows through (CLI open, Lua find-file, + /// `WorkspaceEdit` rename ops), so doing it here keeps the invariant + /// "a buffer's `file_path` is always absolute" — which the LSP + /// layer relies on to build a resolvable `file:///…` URI (a + /// relative or `~`-prefixed path produced `file://ipc.cpp`, which + /// clangd rejected with `-32602 unresolvable URI`) and which + /// cross-file navigation relies on for buffer-identity matching. pub fn set_buffer_path(&mut self, id: BufferId, path: Option) { + let path = path.map(normalize_buffer_path); if let Ok(b) = self.registry.borrow_mut().get_mut(id) { b.set_file_path(path); } @@ -1479,6 +1490,79 @@ fn backward_word(buf: &Buffer, mut pos: Position) -> Position { pos } +/// Normalize a buffer path to an absolute, lexically-clean form: +/// +/// 1. expand a leading `~` / `~/…` against `$HOME`, +/// 2. join onto the process cwd if still relative, +/// 3. fold `.` / `..` purely lexically. +/// +/// No filesystem access and no symlink resolution (unlike +/// [`std::fs::canonicalize`]): the result is correct for a +/// not-yet-created "[new file]" buffer and never silently rewrites a +/// path's on-disk identity. Every step is best-effort — if `$HOME` +/// or the cwd is unavailable the path is returned as far as it could +/// be resolved rather than panicking. +fn normalize_buffer_path(path: PathBuf) -> PathBuf { + let path = expand_tilde(path); + let abs = if path.is_absolute() { + path + } else if let Ok(cwd) = std::env::current_dir() { + cwd.join(path) + } else { + path + }; + lexical_normalize(&abs) +} + +/// Expand a leading `~` (whole component only) using `$HOME`. A bare +/// `~` becomes `$HOME`; `~/x` becomes `$HOME/x`. `~user` is left +/// untouched (no passwd lookup). Returns the input unchanged if it +/// has no leading `~`, isn't valid UTF-8, or `$HOME` is unset. +fn expand_tilde(path: PathBuf) -> PathBuf { + let Some(s) = path.to_str() else { + return path; + }; + if s == "~" { + return std::env::var_os("HOME").map_or(path, PathBuf::from); + } + if let Some(rest) = s.strip_prefix("~/") + && let Some(home) = std::env::var_os("HOME") + { + return Path::new(&home).join(rest); + } + path +} + +/// Fold `.` and `..` components without touching the filesystem. +/// `..` pops a preceding normal segment; against the root (or a +/// Windows prefix) it is dropped, since you cannot ascend past it. +fn lexical_normalize(path: &Path) -> PathBuf { + use std::path::Component; + let mut stack: Vec = Vec::new(); + for comp in path.components() { + match comp { + Component::CurDir => {} + Component::ParentDir => match stack.last() { + Some(Component::Normal(_)) => { + stack.pop(); + } + Some(Component::RootDir | Component::Prefix(_)) => {} + _ => stack.push(Component::ParentDir), + }, + c => stack.push(c), + } + } + let mut out = PathBuf::new(); + for c in stack { + out.push(c.as_os_str()); + } + if out.as_os_str().is_empty() { + PathBuf::from(".") + } else { + out + } +} + // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- @@ -1489,6 +1573,54 @@ mod tests { use std::cell::RefCell; use std::rc::Rc; + #[test] + fn lexical_normalize_folds_dot_and_dotdot() { + assert_eq!( + lexical_normalize(Path::new("/a/./b/../c")), + PathBuf::from("/a/c") + ); + // `..` cannot ascend past the root. + assert_eq!( + lexical_normalize(Path::new("/../../x")), + PathBuf::from("/x") + ); + // Already clean ⇒ unchanged (keeps tempdir paths stable so + // the LSP acceptance tests' exact-path asserts still hold). + assert_eq!( + lexical_normalize(Path::new("/tmp/quickshell/ipc.cpp")), + PathBuf::from("/tmp/quickshell/ipc.cpp") + ); + } + + #[test] + fn expand_tilde_only_at_leading_component() { + // `~user` (no passwd lookup) and a non-leading `~` are left + // exactly as-is, independent of `$HOME`. + assert_eq!( + expand_tilde(PathBuf::from("~bob/x")), + PathBuf::from("~bob/x") + ); + assert_eq!(expand_tilde(PathBuf::from("a/~/b")), PathBuf::from("a/~/b")); + // With `$HOME` set (the case in any normal test environment) + // a leading `~` / `~/…` expands against its real value. + if let Some(home) = std::env::var_os("HOME") { + assert_eq!(expand_tilde(PathBuf::from("~")), PathBuf::from(&home)); + assert_eq!( + expand_tilde(PathBuf::from("~/src/ipc.cpp")), + Path::new(&home).join("src/ipc.cpp") + ); + } + } + + #[test] + fn normalize_buffer_path_yields_absolute() { + // A relative path becomes absolute (joined onto cwd) — this + // is exactly what made clangd reject `file://ipc.cpp`. + let p = normalize_buffer_path(PathBuf::from("ipc.cpp")); + assert!(p.is_absolute(), "expected absolute, got {p:?}"); + assert!(p.ends_with("ipc.cpp")); + } + fn fresh() -> EditorCore { let reg: SharedRegistry = Rc::new(RefCell::new(crate::buffer_registry::BufferRegistry::new())); From 56a57d3c7fa4b1ee4604a970816db1a7cdf11f93 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Tue, 19 May 2026 13:58:21 -0400 Subject: [PATCH 2/2] Fix: defer pre-init LSP notifications until the server is Initialized send_notification wrote frames straight to stdin regardless of lifecycle state. At CLI startup the buffer.after-load hook fires did_open while clangd's initialize is still in flight; the LSP spec lets a server discard any notification before the initialize/initialized handshake, and clangd does. The document is then never "added", so every later request fails with `-32602 trying to get AST for non-added document` (and no diagnostics ever appear). Lenient servers (rust-analyzer, gopls) queue internally, which is why the M4.5 arc didn't catch it. Buffer notifications issued while Starting/Initializing on the client and replay them, in issue order, immediately after the `initialized` notification goes out (flush_deferred_notifications, from the initialize-response handler). `initialized`/`exit` are sent directly by the lifecycle handler and never pass through send_notification, so they bypass the gate. The queue is cleared on (re)start_generation since the reattach path re-sends fresh did_opens against the new process. This also makes the stale lsp.lua:211 comment ("the manager queues it cleanly even while starting/initializing") finally true. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/lsp.rs | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 57 insertions(+), 3 deletions(-) diff --git a/src/lsp.rs b/src/lsp.rs index e87e17f..a15269e 100644 --- a/src/lsp.rs +++ b/src/lsp.rs @@ -666,6 +666,16 @@ pub struct LspClient { /// "response for unknown request id" `ProtocolError`. Mirrors /// `mcp.rs`'s `cancelled_rids`. cancelled_rids: HashSet, + /// Notifications issued before the server reached `Initialized`. + /// The LSP lifecycle requires `initialize` / `initialized` to + /// complete before any other notification; a strict server + /// (clangd) silently discards a pre-init `textDocument/didOpen`, + /// after which every later request fails with + /// `-32602 trying to get AST for non-added document`. These are + /// held in issue order while `Starting` / `Initializing` and + /// replayed the instant the server reaches `Initialized` (right + /// after the `initialized` notification is sent). + deferred_notifications: Vec<(String, Value)>, /// T M4.5 Option B: encoding the server negotiated for `Position` /// `character` counts. Set from the `initialize` response; /// defaults to the LSP spec default (UTF-16) until then. @@ -684,6 +694,7 @@ impl LspClient { attempt: 0, next_restart_at: None, cancelled_rids: HashSet::new(), + deferred_notifications: Vec::new(), position_encoding: PositionEncoding::default(), } } @@ -1193,6 +1204,11 @@ impl LspManager { // space; stale cancelled ids can never collide, so reset to // keep the set from growing across restarts. client.cancelled_rids.clear(); + // Drop notifications buffered against the dead generation: + // they reference its (now-gone) document state, and the + // editor's reattach path issues fresh `did_open`s after the + // new generation finishes initializing. + client.deferred_notifications.clear(); // T M4.7: drop any pending response routes for this server. // Their request ids belong to the previous generation; the // new server starts request id numbering fresh. @@ -1286,9 +1302,7 @@ impl LspManager { .clients .get_mut(&id) .ok_or_else(|| format!("unknown server: {id}"))?; - // Notifications during init are allowed (the spec actually - // *requires* `initialized` during the initializing phase), - // but disallowed after Stopped/Crashed since stdin is gone. + // Disallowed after Stopped/Crashed since stdin is gone. if matches!( client.state, LspClientState::Stopped { .. } | LspClientState::Crashed { .. } @@ -1298,11 +1312,47 @@ impl LspManager { state_label(&client.state) )); } + // Before the `initialize` / `initialized` handshake completes + // the only notification the spec permits is `initialized` + // itself — and that one is sent directly by `handle_response`, + // never through here. Everything else issued this early + // (`textDocument/didOpen` from the editor's attach hook is the + // common one) is buffered and replayed in order once the + // server reaches `Initialized`; sending it now would be + // discarded by a strict server, breaking every later request + // with "non-added document". + if matches!( + client.state, + LspClientState::Starting | LspClientState::Initializing { .. } + ) { + client.deferred_notifications.push((method, params)); + return Ok(()); + } let body = make_notification(&method, params); send_frame_to(&self.supervisor, client, &body)?; Ok(()) } + /// Replay, in issue order, the notifications buffered by + /// [`Self::send_notification`] while `sid` was still initializing. + /// Called once, from the `initialize`-response handler, right + /// after the `initialized` notification is sent. + fn flush_deferred_notifications(&mut self, sid: LspServerId) { + let deferred = self + .clients + .get_mut(&sid) + .map(|c| std::mem::take(&mut c.deferred_notifications)) + .unwrap_or_default(); + if !deferred.is_empty() + && let Some(client) = self.clients.get(&sid) + { + for (method, params) in deferred { + let body = make_notification(&method, params); + let _ = send_frame_to(&self.supervisor, client, &body); + } + } + } + /// The position encoding `sid` negotiated, or the spec default /// (UTF-16) if the server is unknown / not yet initialized. fn position_encoding(&self, sid: LspServerId) -> PositionEncoding { @@ -2326,6 +2376,10 @@ impl LspManager { initialized_at: now, }; } + // The handshake is complete: replay every notification + // buffered while the server was still initializing, after + // the `initialized` notification that went out above. + self.flush_deferred_notifications(sid); self.push_event(sid, now, LspEventKind::Initialized { capabilities: caps }); return; }