Merge pull request #26 from levineuwirth/lsp-uri-and-init-race-fixes

Fix LSP transport: absolute-path URIs + defer pre-init notifications

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<PathBuf>) {
+        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<Component> = 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()));

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<u64>,
+    /// 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(),
         }
     }
@@ -1198,6 +1209,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.
@@ -1291,9 +1307,7 @@ impl LspManager {
             .clients
             .get_mut(&id)
             .ok_or_else(|| format!("unknown server: {id}"))?;
-        // Notifications during init are allowed (the spec actually
+        // Disallowed after Stopped/Crashed since stdin is gone.
-        // *requires* `initialized` during the initializing phase),
-        // but disallowed after Stopped/Crashed since stdin is gone.
         if matches!(
             client.state,
             LspClientState::Stopped { .. } | LspClientState::Crashed { .. }
@@ -1303,11 +1317,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 {
@@ -2383,6 +2433,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;
         }