diff --git a/crates/terraphim_lsp/Cargo.lock b/crates/terraphim_lsp/Cargo.lock index 011111d14..20a2e4680 100644 --- a/crates/terraphim_lsp/Cargo.lock +++ b/crates/terraphim_lsp/Cargo.lock @@ -1597,7 +1597,7 @@ dependencies = [ [[package]] name = "terraphim_lsp" -version = "1.22.0-rc.6" +version = "1.22.0-rc.7" dependencies = [ "criterion", "env_logger", diff --git a/crates/terraphim_lsp/Cargo.toml b/crates/terraphim_lsp/Cargo.toml index 292967c3b..fe697a785 100644 --- a/crates/terraphim_lsp/Cargo.toml +++ b/crates/terraphim_lsp/Cargo.toml @@ -50,7 +50,7 @@ resolver = "2" members = [".", "../terraphim_lsp_core"] [workspace.package] -version = "1.22.0-rc.6" +version = "1.22.0-rc.7" edition = "2024" rust-version = "1.91" diff --git a/crates/terraphim_lsp/src/server.rs b/crates/terraphim_lsp/src/server.rs index bf3b443dc..6c7d385bc 100644 --- a/crates/terraphim_lsp/src/server.rs +++ b/crates/terraphim_lsp/src/server.rs @@ -90,6 +90,9 @@ pub struct TerraphimLspServer { kg_loads: AtomicU64, cut_operations: Arc, documents: Arc>>, + /// JSON buffers are tracked only to protect export destinations. When both + /// maps are needed, always lock documents before passive_buffers. + passive_buffers: RwLock>, /// Last acknowledged generated text, used to refuse changed output buffers. exported: RwLock>, /// Whether the client accepts `WorkspaceEdit.documentChanges` @@ -157,6 +160,40 @@ impl Kg { } } +/// Raw text used for export safety, never for Markdown analysis. +#[derive(Debug, Clone)] +struct ExportBuffer { + text: String, + version: i32, + incarnation: Arc<()>, + kind: BufferKind, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum BufferKind { + Markdown, + Json, + Jsonc, +} + +impl ExportBuffer { + fn markdown(document: &OpenDocument) -> Self { + Self { + text: document.text.clone(), + version: document.version, + incarnation: Arc::clone(&document.incarnation), + kind: BufferKind::Markdown, + } + } + + fn unchanged(&self, other: &Self) -> bool { + self.kind == other.kind + && self.version == other.version + && self.text == other.text + && Arc::ptr_eq(&self.incarnation, &other.incarnation) + } +} + /// An open document: its full text, the version the client last sent, and /// the Lab and trim results computed for that text. #[derive(Debug, Clone, Default)] @@ -394,6 +431,7 @@ impl TerraphimLspServer { kg_loads: AtomicU64::new(0), cut_operations: Arc::new(AtomicU64::new(0)), documents: Arc::new(RwLock::new(HashMap::new())), + passive_buffers: RwLock::new(HashMap::new()), exported: RwLock::new(HashMap::new()), versioned_edits: Arc::new(AtomicBool::new(false)), inlay_hint_refresh: AtomicBool::new(false), @@ -627,19 +665,9 @@ impl TerraphimLspServer { .map(|document| document.text.clone()) } - /// Record the latest text and version of a document. Requested Lab - /// actions and the trim level survive; their results, computed for the - /// old text, are dropped. - async fn store_document(&self, uri: &Url, text: &str, version: i32) { - let mut documents = self.documents.write().await; - if documents - .get(uri) - .is_some_and(|document| version <= document.version) - { - log::debug!("terraphim_lsp: ignored non-increasing document version {version}"); - return; - } - let document = documents.entry(uri.clone()).or_default(); + /// Update known Markdown state while its map is locked. Callers enforce + /// monotonic changes; didOpen may start a new version sequence at zero. + fn update_document(document: &mut OpenDocument, text: &str, version: i32) { document.text = text.to_string(); document.version = version; document.reanchoring = None; @@ -658,10 +686,68 @@ impl TerraphimLspServer { document.trim = None; document.trim_made = None; document.trim_walk = None; + } + + #[cfg(test)] + async fn store_document(&self, uri: &Url, text: &str, version: i32) { + let mut documents = self.documents.write().await; + if documents + .get(uri) + .is_some_and(|document| version <= document.version) + { + return; + } + Self::update_document(documents.entry(uri.clone()).or_default(), text, version); drop(documents); self.changed.notify_waiters(); } + fn export_buffer_from( + documents: &HashMap, + passive: &HashMap, + uri: &Url, + ) -> Option { + documents + .get(uri) + .map(ExportBuffer::markdown) + .or_else(|| passive.get(uri).cloned()) + } + + async fn export_buffer(&self, uri: &Url) -> Option { + let documents = self.documents.read().await; + let passive = self.passive_buffers.read().await; + Self::export_buffer_from(&documents, &passive, uri) + } + + /// Revalidate both lifecycles together after filesystem inspection yields. + async fn export_snapshots_current( + &self, + source: &Url, + snapshot: &OpenDocument, + target: &Url, + original_target: Option<&ExportBuffer>, + ) -> Result { + let documents = self.documents.read().await; + let passive = self.passive_buffers.read().await; + if !documents.get(source).is_some_and(|current| { + current.version == snapshot.version + && current.text == snapshot.text + && Arc::ptr_eq(¤t.incarnation, &snapshot.incarnation) + }) { + return Err(tower_lsp::jsonrpc::Error { + code: tower_lsp::jsonrpc::ErrorCode::ContentModified, + message: "Source document changed or closed while preparing export".into(), + data: None, + }); + } + let current = Self::export_buffer_from(&documents, &passive, target); + Ok(match (original_target, current.as_ref()) { + (None, None) => true, + (Some(before), Some(after)) => before.unchanged(after), + _ => false, + }) + } + /// What a Lab run needs from a document, taken under a brief lock so /// the (whole-document) computation runs without holding any lock. fn lab_job(&self, document: &OpenDocument) -> LabJob { @@ -1399,14 +1485,16 @@ impl TerraphimLspServer { let Ok(target_path) = target.to_file_path() else { return false; }; - let mut protected: Vec = self - .documents - .read() - .await - .keys() - .filter(|uri| *uri != target) - .filter_map(|uri| uri.to_file_path().ok()) - .collect(); + let mut protected: Vec = { + let documents = self.documents.read().await; + let passive = self.passive_buffers.read().await; + documents + .keys() + .chain(passive.keys()) + .filter(|uri| *uri != target) + .filter_map(|uri| uri.to_file_path().ok()) + .collect() + }; if let Ok(path) = source.to_file_path() { protected.push(path); } @@ -1456,12 +1544,12 @@ impl TerraphimLspServer { target: &Url, markdown: &str, ) -> Result { + let open = self.export_buffer(target).await; if !self.export_target_is_separate(source, target).await { self.client.show_message(MessageType::WARNING, "terraphim-lsp: choose an independent export file; the destination may alias the source or another open document.").await; return Ok(false); } - let open = self.documents.read().await.get(target).cloned(); let disk = if let Ok(path) = target.to_file_path() { match tokio::fs::read_to_string(path).await { Ok(text) => Some(text), @@ -1525,23 +1613,13 @@ impl TerraphimLspServer { if !self.export_target_is_separate(source, target).await { return Ok(false); } - // Filesystem checks above yield to didChange/close/open notifications. - // Check both version and text immediately before dispatch; reopening a - // different document at the same URI can restart its version sequence. if !self - .documents - .read() - .await - .get(source) - .is_some_and(|current| { - current.version == snapshot.version && current.text == snapshot.text - }) + .export_snapshots_current(source, snapshot, target, open.as_ref()) + .await? { - return Err(tower_lsp::jsonrpc::Error { - code: tower_lsp::jsonrpc::ErrorCode::ContentModified, - message: "Source document changed or closed while preparing export".into(), - data: None, - }); + self.client.show_message(MessageType::WARNING, + "terraphim-lsp: the export buffer changed while preparing export; try again after saving or closing it.").await; + return Ok(false); } let applied = match self.client.apply_edit(edit).await { Ok(response) => response.applied, @@ -2226,12 +2304,43 @@ impl LanguageServer for TerraphimLspServer { } async fn did_open(&self, params: DidOpenTextDocumentParams) { - let uri = params.text_document.uri; - let text = params.text_document.text; - // Opening starts a new client version sequence, including version 0. - self.documents.write().await.remove(&uri); - self.store_document(&uri, &text, params.text_document.version) - .await; + let item = params.text_document; + let uri = item.uri; + let kind = match item.language_id.as_str() { + "json" => BufferKind::Json, + "jsonc" => BufferKind::Jsonc, + _ => BufferKind::Markdown, + }; + let was_markdown = { + let mut documents = self.documents.write().await; + let mut passive = self.passive_buffers.write().await; + let was_markdown = documents.remove(&uri).is_some(); + passive.remove(&uri); + if kind == BufferKind::Markdown { + let mut document = OpenDocument::default(); + Self::update_document(&mut document, &item.text, item.version); + documents.insert(uri.clone(), document); + } else { + passive.insert( + uri.clone(), + ExportBuffer { + text: item.text, + version: item.version, + incarnation: Arc::new(()), + kind, + }, + ); + } + was_markdown + }; + self.changed.notify_waiters(); + if kind != BufferKind::Markdown { + if was_markdown { + // Clear diagnostics from the former language without analysing JSON. + self.send_push(uri, vec![], None).await; + } + return; + } if self.settings().lab.trigger == LabTrigger::Save { self.refresh_lab_and_publish(&uri).await; } else { @@ -2241,6 +2350,9 @@ impl LanguageServer for TerraphimLspServer { async fn did_save(&self, params: DidSaveTextDocumentParams) { let uri = params.text_document.uri; + if !self.documents.read().await.contains_key(&uri) { + return; + } if self.settings().reanchor_on_save { self.reanchor_on_save(&uri).await; } @@ -2271,8 +2383,32 @@ impl LanguageServer for TerraphimLspServer { async fn did_change(&self, params: DidChangeTextDocumentParams) { let uri = params.text_document.uri; let version = params.text_document.version; - if let Some(change) = params.content_changes.into_iter().last() { - self.store_document(&uri, &change.text, version).await; + let Some(change) = params.content_changes.into_iter().last() else { + return; + }; + let markdown_changed = { + let mut documents = self.documents.write().await; + let mut passive = self.passive_buffers.write().await; + if let Some(document) = documents.get_mut(&uri) { + if version <= document.version { + return; + } + Self::update_document(document, &change.text, version); + true + } else if let Some(buffer) = passive.get_mut(&uri) { + if version <= buffer.version { + return; + } + buffer.text = change.text; + buffer.version = version; + false + } else { + // A delayed change after close must not create Markdown state. + return; + } + }; + self.changed.notify_waiters(); + if markdown_changed { self.push_open(&uri).await; } } @@ -2282,11 +2418,17 @@ impl LanguageServer for TerraphimLspServer { /// owns the results it pulled and stops pulling (and drops them) for a /// closed document, so neither a push nor a refresh is needed. async fn did_close(&self, params: DidCloseTextDocumentParams) { - self.documents - .write() - .await - .remove(¶ms.text_document.uri); - self.send_push(params.text_document.uri, vec![], None).await; + let uri = params.text_document.uri; + let was_markdown = { + let mut documents = self.documents.write().await; + let mut passive = self.passive_buffers.write().await; + passive.remove(&uri); + documents.remove(&uri).is_some() + }; + self.changed.notify_waiters(); + if was_markdown { + self.send_push(uri, vec![], None).await; + } } async fn hover(&self, params: HoverParams) -> Result> { diff --git a/crates/terraphim_lsp/tests/passive_export_stdio_tests.rs b/crates/terraphim_lsp/tests/passive_export_stdio_tests.rs new file mode 100644 index 000000000..7ec074b8c --- /dev/null +++ b/crates/terraphim_lsp/tests/passive_export_stdio_tests.rs @@ -0,0 +1,447 @@ +//! Real server executable, protocol and files: JSON is tracked only for export safety. +use std::io::{BufRead, BufReader, Read, Write}; +use std::process::{Child, ChildStdin, Command, Stdio}; +use std::sync::mpsc::{self, Receiver}; +use std::time::Duration; + +use serde_json::{Value, json}; +use tower_lsp::lsp_types::Url; + +const COMMAND: &str = "terraphim.export_knowledge_graph"; + +struct Wire { + child: Child, + input: ChildStdin, + messages: Receiver, + observed: Vec, +} + +impl Wire { + fn start() -> Self { + let mut child = Command::new(env!("CARGO_BIN_EXE_terraphim-lsp")) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()) + .spawn() + .expect("real LSP binary"); + let input = child.stdin.take().unwrap(); + let output = child.stdout.take().unwrap(); + let (sender, messages) = mpsc::channel(); + std::thread::spawn(move || { + let mut reader = BufReader::new(output); + loop { + let mut length = 0; + loop { + let mut line = String::new(); + if reader.read_line(&mut line).unwrap_or(0) == 0 { + return; + } + if line == "\r\n" { + break; + } + if let Some(value) = line.to_ascii_lowercase().strip_prefix("content-length:") { + length = value.trim().parse().unwrap(); + } + } + let mut body = vec![0; length]; + if reader.read_exact(&mut body).is_err() { + return; + } + if sender.send(serde_json::from_slice(&body).unwrap()).is_err() { + return; + } + } + }); + let mut wire = Self { + child, + input, + messages, + observed: Vec::new(), + }; + wire.send(json!({"jsonrpc":"2.0","id":1,"method":"initialize","params":{ + "processId":null,"rootUri":null,"capabilities":{"workspace":{ + "applyEdit":true,"workspaceEdit":{"documentChanges":true,"resourceOperations":["create"]} + }},"initializationOptions":{"lab":{"trigger":"save","actions":["hedges_and_filler"]},"reanchorOnSave":true} + }})); + let result = wire.response(1); + assert!( + result["result"]["capabilities"]["executeCommandProvider"]["commands"] + .as_array() + .unwrap() + .contains(&json!(COMMAND)), + "{result}" + ); + wire.send(json!({"jsonrpc":"2.0","method":"initialized","params":{}})); + wire + } + fn send(&mut self, value: Value) { + let body = value.to_string(); + write!(self.input, "Content-Length: {}\r\n\r\n{body}", body.len()).unwrap(); + self.input.flush().unwrap(); + } + fn next(&mut self) -> Value { + let value = self + .messages + .recv_timeout(Duration::from_secs(20)) + .expect("server response"); + self.observed.push(value.clone()); + value + } + fn response(&mut self, id: i32) -> Value { + loop { + let value = self.next(); + if value["id"] == id && value.get("method").is_none() { + return value; + } + assert_ne!( + value["method"], "workspace/applyEdit", + "unexpected edit: {value}" + ); + } + } + fn apply_request(&mut self) -> Value { + loop { + let value = self.next(); + if value["method"] == "workspace/applyEdit" { + return value; + } + assert!(value.get("error").is_none(), "unexpected failure: {value}"); + } + } + fn open(&mut self, uri: &Url, language: &str, text: &str, version: i32) { + self.send( + json!({"jsonrpc":"2.0","method":"textDocument/didOpen","params":{ + "textDocument":{"uri":uri,"languageId":language,"version":version,"text":text} + }}), + ); + self.barrier(uri); + } + fn barrier(&mut self, uri: &Url) { + self.send( + json!({"jsonrpc":"2.0","id":9,"method":"textDocument/diagnostic","params":{ + "textDocument":{"uri":uri} + }}), + ); + assert!(self.response(9).get("error").is_none()); + } + fn change(&mut self, uri: &Url, text: &str, version: i32) { + self.send( + json!({"jsonrpc":"2.0","method":"textDocument/didChange","params":{ + "textDocument":{"uri":uri,"version":version},"contentChanges":[{"text":text}] + }}), + ); + } + fn export(&mut self, source: &Url, version: i32) { + self.send( + json!({"jsonrpc":"2.0","id":10,"method":"workspace/executeCommand","params":{ + "command":COMMAND,"arguments":[{"uri":source,"version":version}] + }}), + ); + } + fn close(&mut self, uri: &Url) { + self.send(json!({"jsonrpc":"2.0","method":"textDocument/didClose","params":{"textDocument":{"uri":uri}}})); + self.barrier(uri); + } + fn acknowledge(&mut self, request: &Value, applied: bool) { + self.send(json!({"jsonrpc":"2.0","id":request["id"],"result":{"applied":applied}})); + } +} + +impl Drop for Wire { + fn drop(&mut self) { + // A failed assertion must not leave the real server running. + let _ = self.child.kill(); + let _ = self.child.wait(); + } +} + +fn authored(alternative: &str) -> String { + let body = "Risk: delivery may slip.\n"; + let added = terraphim_lsp::core::add_alternative( + body, + terraphim_lsp::core::TextRange::from_bytes(body, 0, 4), + alternative, + None, + ) + .unwrap(); + terraphim_lsp::core::apply_edits(body, &[added.edit]) +} + +#[test] +fn json_and_jsonc_never_receive_markdown_analysis_or_commands() { + for language in ["json", "jsonc"] { + let directory = tempfile::tempdir().unwrap(); + let uri = Url::from_file_path(directory.path().join("graph.json")).unwrap(); + let mut wire = Wire::start(); + let text = + "Risk is actually really probably possible.\n```terraphim-alternatives\n{invalid}\n```"; + wire.open(&uri, language, text, 1); + for (method, extra) in [ + ( + "textDocument/hover", + json!({"position":{"line":0,"character":1}}), + ), + ( + "textDocument/completion", + json!({"position":{"line":0,"character":1}}), + ), + ( + "textDocument/codeAction", + json!({"range":{"start":{"line":0,"character":0},"end":{"line":0,"character":4}},"context":{"diagnostics":[]}}), + ), + ( + "textDocument/inlayHint", + json!({"range":{"start":{"line":0,"character":0},"end":{"line":1,"character":0}}}), + ), + ("textDocument/foldingRange", json!({})), + ] { + let mut params = extra; + params["textDocument"] = json!({"uri":uri}); + wire.send(json!({"jsonrpc":"2.0","id":20,"method":method,"params":params})); + let response = wire.response(20); + assert_eq!( + response["result"], + Value::Null, + "{language} {method}: {response}" + ); + } + wire.send(json!({"jsonrpc":"2.0","method":"textDocument/didSave","params":{"textDocument":{"uri":uri}}})); + wire.send(json!({"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{"lab":{"trigger":"save"}}}})); + wire.send(json!({"jsonrpc":"2.0","id":21,"method":"textDocument/diagnostic","params":{"textDocument":{"uri":uri}}})); + assert_eq!(wire.response(21)["result"]["items"], json!([])); + for (command, extra) in [ + ("terraphim.export", json!({})), + ("terraphim.export_knowledge_graph", json!({})), + ("terraphim.lab.mark", json!({"action":"hedges_and_filler"})), + ("terraphim.trim.preview", json!({"level":"slight"})), + ("terraphim.trim.make_cuts", json!({})), + ( + "terraphim.alternative.add", + json!({"text":"Concern","range":{"start":{"line":0,"character":0},"end":{"line":0,"character":4}}}), + ), + ( + "terraphim.alternative.add_and_apply", + json!({"text":"Concern","range":{"start":{"line":0,"character":0},"end":{"line":0,"character":4}}}), + ), + ] { + let mut argument = extra; + argument["uri"] = json!(uri); + argument["version"] = json!(1); + wire.send( + json!({"jsonrpc":"2.0","id":22,"method":"workspace/executeCommand","params":{ + "command":command,"arguments":[argument] + }}), + ); + let result = wire.response(22); + assert!( + result["error"]["message"] + .as_str() + .is_some_and(|message| message.ends_with("is not open")), + "{command} must reject JSON source eligibility, not malformed arguments: {result}" + ); + } + assert!( + wire.observed.iter().all(|message| message["method"] + != "textDocument/publishDiagnostics" + || message["params"]["uri"] != json!(uri) + || message["params"]["diagnostics"] == json!([])), + "JSON must not receive Markdown findings" + ); + } +} + +#[test] +fn dirty_json_and_jsonc_refuse_without_touching_real_files() { + for language in ["json", "jsonc"] { + let directory = tempfile::tempdir().unwrap(); + let source_path = directory.path().join("brief.md"); + let target_path = directory.path().join("brief.knowledge-graph.json"); + let source = Url::from_file_path(&source_path).unwrap(); + let target = Url::from_file_path(&target_path).unwrap(); + let body = authored("Concern"); + let disk = "{\"name\":\"Original user graph\",\"data\":{}}\n"; + std::fs::write(&source_path, &body).unwrap(); + std::fs::write(&target_path, disk).unwrap(); + let mut wire = Wire::start(); + wire.open(&source, "markdown", &body, 1); + wire.open(&target, language, disk, 1); + let dirty = disk.replace("Original", "UNSAVED INTERIOR") + "UNSAVED APPEND\n"; + wire.change(&target, &dirty, 2); + wire.barrier(&target); + wire.export(&source, 1); + let result = wire.response(10); + assert_eq!(result["result"]["written"], false, "{result}"); + // Notifications and command responses use separate output queues. + // Await the actual warning rather than assuming it preceded the result. + while !wire.observed.iter().any(|message| { + message["method"] == "window/showMessage" + && message["params"]["message"] + .as_str() + .is_some_and(|text| text.contains("unconfirmed changes")) + }) { + let message = wire.next(); + assert_ne!(message["method"], "workspace/applyEdit"); + } + assert_eq!(std::fs::read_to_string(&target_path).unwrap(), disk); + assert_eq!(std::fs::read_to_string(&source_path).unwrap(), body); + // Older changes cannot erase the known pending edit. + wire.change(&target, disk, 1); + wire.barrier(&target); + wire.export(&source, 1); + assert_eq!(wire.response(10)["result"]["written"], false); + } +} + +#[test] +fn closed_passive_buffer_changes_cannot_create_a_markdown_source() { + let directory = tempfile::tempdir().unwrap(); + let uri = Url::from_file_path(directory.path().join("graph.json")).unwrap(); + let mut wire = Wire::start(); + wire.open(&uri, "json", "{}", 7); + wire.close(&uri); + wire.change(&uri, &authored("Concern"), 8); + wire.barrier(&uri); + wire.export(&uri, 8); + assert!(wire.response(10).get("error").is_some()); + wire.open(&uri, "jsonc", "{}", 0); + wire.export(&uri, 0); + assert!(wire.response(10).get("error").is_some()); +} + +#[test] +fn clean_json_repeated_export_uses_versions_and_saves_actual_changed_graph() { + let directory = tempfile::tempdir().unwrap(); + let source_path = directory.path().join("brief.md"); + let target_path = directory.path().join("brief.knowledge-graph.json"); + let source = Url::from_file_path(&source_path).unwrap(); + let target = Url::from_file_path(&target_path).unwrap(); + let mut wire = Wire::start(); + let mut current = "{}\n".to_string(); + std::fs::write(&target_path, ¤t).unwrap(); + wire.open(&target, "json", ¤t, 1); + for (version, alternative) in [(1, "Concern"), (2, "Hazard")] { + let body = authored(alternative); + std::fs::write(&source_path, &body).unwrap(); + if version == 1 { + wire.open(&source, "markdown", &body, version); + } else { + wire.change(&source, &body, version); + wire.barrier(&source); + } + wire.export(&source, version); + let request = wire.apply_request(); + let operations = request["params"]["edit"]["documentChanges"] + .as_array() + .unwrap(); + assert_eq!( + operations.len(), + 1, + "existing output must not be created/truncated" + ); + let edit = &operations[0]; + assert_eq!(edit["textDocument"]["uri"], json!(target)); + assert_eq!(edit["textDocument"]["version"], version); + let edits = edit["edits"].as_array().unwrap(); + assert_eq!(edits.len(), 1); + let index = terraphim_lsp::core::LineIndex::new(¤t); + let end = &edits[0]["range"]["end"]; + assert_eq!( + index.byte_offset(terraphim_lsp::core::LinePosition { + line: end["line"].as_u64().unwrap() as u32, + character: end["character"].as_u64().unwrap() as u32, + }), + current.len() + ); + current = edits[0]["newText"].as_str().unwrap().to_string(); + std::fs::write(&target_path, ¤t).unwrap(); + wire.change(&target, ¤t, version + 1); + wire.acknowledge(&request, true); + assert_eq!(wire.response(10)["result"]["written"], true); + wire.barrier(&target); + let actual: Value = serde_json::from_slice(&std::fs::read(&target_path).unwrap()).unwrap(); + assert_eq!( + actual["data"][alternative.to_lowercase()]["id"], + actual["data"]["risk"]["id"] + ); + assert!(actual["data"]["risk"]["id"].is_number()); + assert_eq!(std::fs::read_to_string(&source_path).unwrap(), body); + } + wire.close(&target); + wire.open( + &target, + "json", + &std::fs::read_to_string(&target_path).unwrap(), + 0, + ); + wire.export(&source, 2); + let request = wire.apply_request(); + assert_eq!( + request["params"]["edit"]["documentChanges"][0]["textDocument"]["version"], + 0 + ); + wire.acknowledge(&request, false); + assert_eq!(wire.response(10)["result"]["written"], false); + assert_eq!(std::fs::read_to_string(&target_path).unwrap(), current); +} + +#[test] +fn reopening_with_a_different_language_moves_the_buffer_kind() { + let directory = tempfile::tempdir().unwrap(); + let uri = Url::from_file_path(directory.path().join("changed-language.json")).unwrap(); + let body = authored("Concern"); + let mut wire = Wire::start(); + for language in ["markdown", "json", "jsonc", "markdown"] { + wire.open(&uri, language, &body, 0); + wire.send(json!({"jsonrpc":"2.0","id":25,"method":"textDocument/codeAction","params":{ + "textDocument":{"uri":uri},"range":{"start":{"line":0,"character":0},"end":{"line":0,"character":4}},"context":{"diagnostics":[]} + }})); + let result = wire.response(25); + if language == "markdown" { + assert!( + result["result"] + .as_array() + .is_some_and(|items| !items.is_empty()), + "{result}" + ); + } else { + assert_eq!(result["result"], Value::Null); + } + } +} + +#[test] +fn rejected_graph_payload_does_not_authorize_later_unsaved_output() { + let directory = tempfile::tempdir().unwrap(); + let source_path = directory.path().join("brief.md"); + let target_path = directory.path().join("brief.knowledge-graph.json"); + let source = Url::from_file_path(&source_path).unwrap(); + let target = Url::from_file_path(&target_path).unwrap(); + let body = authored("Concern"); + let disk = "{}\n"; + std::fs::write(&source_path, &body).unwrap(); + std::fs::write(&target_path, disk).unwrap(); + let mut wire = Wire::start(); + wire.open(&source, "markdown", &body, 1); + wire.open(&target, "json", disk, 1); + wire.export(&source, 1); + let request = wire.apply_request(); + let rejected = request["params"]["edit"]["documentChanges"][0]["edits"][0]["newText"] + .as_str() + .unwrap() + .to_string(); + assert_ne!(rejected, disk); + assert_eq!( + request["params"]["edit"]["documentChanges"][0]["textDocument"]["version"], + 1 + ); + // The target changes after dispatch: the client refuses the version-1 edit. + // This is new unsaved client input, not an acknowledged generated export. + wire.change(&target, &rejected, 2); + wire.barrier(&target); + wire.acknowledge(&request, false); + assert_eq!(wire.response(10)["result"]["written"], false); + wire.export(&source, 1); + assert_eq!(wire.response(10)["result"]["written"], false); + assert_eq!(std::fs::read_to_string(target_path).unwrap(), disk); + assert_eq!(std::fs::read_to_string(source_path).unwrap(), body); +} diff --git a/crates/terraphim_lsp/tests/save_export_tests.rs b/crates/terraphim_lsp/tests/save_export_tests.rs index 9f73b926c..44c6d6ee5 100644 --- a/crates/terraphim_lsp/tests/save_export_tests.rs +++ b/crates/terraphim_lsp/tests/save_export_tests.rs @@ -948,6 +948,7 @@ async fn export_applying_to_disk( async fn assert_export_alias_refused( link: fn(&std::path::Path, &std::path::Path), other_open: bool, + language: &str, ) { for (command, suffix) in [ (commands::EXPORT_KNOWLEDGE_GRAPH, "knowledge-graph.json"), @@ -960,14 +961,14 @@ async fn assert_export_alias_refused( std::fs::write(&source_path, &original).unwrap(); let mut harness = Harness::start(ZED, json!({}), source.clone(), &original).await; let protected_path = if other_open { - let path = directory.path().join("other.md"); + let path = directory.path().join(format!("other.{language}")); std::fs::write(&path, "Saved other document.\n").unwrap(); harness .server() .did_open(DidOpenTextDocumentParams { text_document: TextDocumentItem { uri: Url::from_file_path(&path).unwrap(), - language_id: "markdown".into(), + language_id: language.into(), version: 4, text: "Unsaved changes in another document.\n".into(), }, @@ -1000,6 +1001,7 @@ async fn graph_and_clean_exports_refuse_source_hard_links() { assert_export_alias_refused( |source, target| std::fs::hard_link(source, target).unwrap(), false, + "markdown", ) .await; } @@ -1009,6 +1011,7 @@ async fn graph_and_clean_exports_refuse_dirty_open_document_hard_links() { assert_export_alias_refused( |source, target| std::fs::hard_link(source, target).unwrap(), true, + "markdown", ) .await; } @@ -1019,6 +1022,7 @@ async fn graph_and_clean_exports_refuse_source_symlinks() { assert_export_alias_refused( |source, target| std::os::unix::fs::symlink(source, target).unwrap(), false, + "markdown", ) .await; } @@ -1029,6 +1033,7 @@ async fn graph_and_clean_exports_refuse_dirty_open_document_symlinks() { assert_export_alias_refused( |source, target| std::os::unix::fs::symlink(source, target).unwrap(), true, + "markdown", ) .await; } @@ -1078,7 +1083,7 @@ fn graph_and_clean_exports_refuse_source_changes_during_file_inspection() { (commands::EXPORT_KNOWLEDGE_GRAPH, "knowledge-graph.json"), (commands::EXPORT, "clean.md"), ] { - for change in ["edit", "close", "reopen"] { + for change in ["edit", "close", "reopen", "reopen_same"] { let directory = tempfile::tempdir().unwrap(); let source_path = directory.path().join("brief.md"); let source = Url::from_file_path(&source_path).unwrap(); @@ -1131,14 +1136,18 @@ fn graph_and_clean_exports_refuse_source_changes_during_file_inspection() { }, }) .await; - if change == "reopen" { + if change == "reopen" || change == "reopen_same" { server .did_open(DidOpenTextDocumentParams { text_document: TextDocumentItem { uri: source.clone(), language_id: "markdown".into(), version: 1, - text: updated.into(), + text: if change == "reopen_same" { + original.clone() + } else { + updated.into() + }, }, }) .await; @@ -1628,3 +1637,149 @@ async fn export_replaces_existing_closed_file_without_truncating_it_first() { assert_eq!(std::fs::read_to_string(&target).unwrap(), "Keep this."); std::fs::remove_dir_all(dir).unwrap(); } + +#[tokio::test] +async fn graph_and_clean_exports_protect_passive_json_hard_links() { + for language in ["json", "jsonc"] { + assert_export_alias_refused( + |source, target| std::fs::hard_link(source, target).unwrap(), + true, + language, + ) + .await; + } +} + +#[cfg(unix)] +#[tokio::test] +async fn graph_and_clean_exports_protect_passive_json_symlinks() { + for language in ["json", "jsonc"] { + assert_export_alias_refused( + |source, target| std::os::unix::fs::symlink(source, target).unwrap(), + true, + language, + ) + .await; + } +} + +#[test] +fn exports_refuse_target_lifecycle_changes_during_file_inspection() { + // The real filesystem inspection is delayed by occupying its blocking pool. + // Notifications run after the actual target snapshot and before dispatch. + tokio::runtime::Builder::new_current_thread() + .enable_all() + .max_blocking_threads(1) + .build() + .unwrap() + .block_on(async { + for (command, suffix) in [ + (commands::EXPORT_KNOWLEDGE_GRAPH, "knowledge-graph.json"), + (commands::EXPORT, "clean.md"), + ] { + for change in ["edit", "close", "reopen", "open", "kind"] { + let directory = tempfile::tempdir().unwrap(); + let source_path = directory.path().join("brief.md"); + let source = Url::from_file_path(&source_path).unwrap(); + let original = authored_graph_document(); + std::fs::write(&source_path, &original).unwrap(); + let target_path = directory.path().join(format!("brief.{suffix}")); + let target = Url::from_file_path(&target_path).unwrap(); + let previous = "{\"user\":\"preserve this output\"}\n"; + std::fs::write(&target_path, previous).unwrap(); + let mut harness = Harness::start( + ZED, + json!({"lab":{"trigger":"command"}}), + source.clone(), + &original, + ) + .await; + let (server, client) = (harness.service.inner(), &mut harness.client); + if change != "open" { + server + .did_open(DidOpenTextDocumentParams { + text_document: TextDocumentItem { + uri: target.clone(), + language_id: "json".into(), + version: 1, + text: previous.into(), + }, + }) + .await; + } + let (entered_tx, entered_rx) = tokio::sync::oneshot::channel(); + let (release_tx, release_rx) = std::sync::mpsc::channel(); + let occupying = tokio::task::spawn_blocking(move || { + entered_tx.send(()).unwrap(); + let _ = release_rx.recv(); + }); + entered_rx.await.unwrap(); + let mut run = Box::pin(server.execute_command(ExecuteCommandParams { + command: command.into(), + arguments: vec![json!({"uri":source,"version":1})], + work_done_progress_params: Default::default(), + })); + assert!(futures::poll!(&mut run).is_pending()); + if change == "edit" { + server + .did_change(DidChangeTextDocumentParams { + text_document: VersionedTextDocumentIdentifier { + uri: target.clone(), + version: 2, + }, + content_changes: vec![TextDocumentContentChangeEvent { + range: None, + range_length: None, + text: previous.into(), + }], + }) + .await; + } else { + if change != "open" { + server + .did_close(DidCloseTextDocumentParams { + text_document: TextDocumentIdentifier { + uri: target.clone(), + }, + }) + .await; + } + if change != "close" { + server + .did_open(DidOpenTextDocumentParams { + text_document: TextDocumentItem { + uri: target.clone(), + language_id: if change == "kind" { + "jsonc" + } else { + "json" + } + .into(), + version: 1, + text: previous.into(), + }, + }) + .await; + } + } + release_tx.send(()).unwrap(); + occupying.await.unwrap(); + let result = tokio::time::timeout(Duration::from_secs(5), run) + .await + .expect("refusal requires no client applyEdit") + .unwrap() + .unwrap(); + assert_eq!(result["written"], false, "{command} {change}"); + assert_eq!(std::fs::read_to_string(&target_path).unwrap(), previous); + assert_eq!(std::fs::read_to_string(&source_path).unwrap(), original); + assert!( + client + .quiet() + .await + .iter() + .all(|request| request.method() != "workspace/applyEdit") + ); + } + } + }); +} diff --git a/release/lsp-crates.json b/release/lsp-crates.json index 2f94ba6a2..2a539e4cc 100644 --- a/release/lsp-crates.json +++ b/release/lsp-crates.json @@ -12,7 +12,7 @@ }, { "name": "terraphim_lsp", - "version": "1.22.0-rc.6", + "version": "1.22.0-rc.7", "path": "crates/terraphim_lsp/Cargo.toml" } ]