From 623aef4f42e9ca65bcd3416f3f5185c9ccc65a2d Mon Sep 17 00:00:00 2001 From: Levi Neely <141506390+lneely@users.noreply.github.com> Date: Wed, 30 Sep 2026 21:28:52 +0200 Subject: [PATCH] Fix phantom namespace nodes and child section insertion Dynamic child resolvers returned Some for any name, fabricating phantom document and section directories so walks to nonexistent paths appeared to succeed. Validate existence and return None instead: - root resolver checks get_doc - document resolver checks new OrkState::section_exists - section resolver checks child_ids Also fix create_child_section, which corrupted the org file: - find_section_end used the parent's headline-end offset, inserting the child before the parent's PROPERTIES/ID drawer and detaching its ID. Use section.span.end, which covers planning, drawer, body, children. - The child headline was written verbatim; demote it to parent.level + 1 via the new restar helper. Add regression tests for restar and child section nesting. --- crates/ork-server/src/namespace.rs | 18 ++++- crates/ork-server/src/state.rs | 113 +++++++++++++++++++++++------ 2 files changed, 103 insertions(+), 28 deletions(-) diff --git a/crates/ork-server/src/namespace.rs b/crates/ork-server/src/namespace.rs index 9ffa4e7..665aa92 100644 --- a/crates/ork-server/src/namespace.rs +++ b/crates/ork-server/src/namespace.rs @@ -13,7 +13,7 @@ pub fn build_namespace(state: Arc) -> FsNode { virtfs::dir_dynamic("/", vec![ // ctl - control commands - virtfs::file("ctl", 0o222, virtfs::rdwr({ + virtfs::file("ctl", 0o666, virtfs::rdwr({ let state = state.clone(); move |data| { let cmd = String::from_utf8_lossy(data); @@ -35,7 +35,7 @@ pub fn build_namespace(state: Arc) -> FsNode { })), // new - create new org file - virtfs::file("new", 0o222, virtfs::rdwr({ + virtfs::file("new", 0o666, virtfs::rdwr({ let state = state.clone(); move |data| { let name = String::from_utf8_lossy(data); @@ -63,6 +63,9 @@ pub fn build_namespace(state: Arc) -> FsNode { // Dynamic children: document directories move || Ok(s.doc_names()), move |name| { + if s2.get_doc(name).is_none() { + return None; + } let doc_name = name.to_string(); Some(build_doc_node(s2.clone(), &doc_name)) }, @@ -133,7 +136,7 @@ fn build_doc_node(state: Arc, doc_name: &str) -> FsNode { })), // new - create top-level section - virtfs::file("new", 0o222, virtfs::rdwr({ + virtfs::file("new", 0o666, virtfs::rdwr({ let state = state.clone(); let name = name.clone(); move |data| { @@ -146,6 +149,9 @@ fn build_doc_node(state: Arc, doc_name: &str) -> FsNode { // Dynamic section directories move || Ok(s.section_ids(&n)), move |id| { + if !s2.section_exists(&n2, id) { + return None; + } Some(build_section_node(s2.clone(), &n2, id)) }, ) @@ -238,6 +244,7 @@ fn build_section_node(state: Arc, doc_name: &str, section_id: &str) -> let i_new = id.clone(); let i_list = id.clone(); + let i_child = id.clone(); virtfs::dir_dynamic(&id, vec![ @@ -373,7 +380,7 @@ fn build_section_node(state: Arc, doc_name: &str, section_id: &str) -> )), // new - create child section - virtfs::file("new", 0o222, virtfs::rdwr(move |data| { + virtfs::file("new", 0o666, virtfs::rdwr(move |data| { let headline = String::from_utf8_lossy(data); s_new.create_child_section(&d_new, &i_new, &headline) .map(|uuid| format!("{}\n", uuid).into_bytes()) @@ -382,6 +389,9 @@ fn build_section_node(state: Arc, doc_name: &str, section_id: &str) -> // children - nested sections move || Ok(s_list.child_ids(&d_list, &i_list)), move |child_id| { + if !s_child.child_ids(&d_child, &i_child).iter().any(|c| c == child_id) { + return None; + } Some(build_section_node(s_child.clone(), &d_child, child_id)) }, ) diff --git a/crates/ork-server/src/state.rs b/crates/ork-server/src/state.rs index f03544b..6b3f75d 100644 --- a/crates/ork-server/src/state.rs +++ b/crates/ork-server/src/state.rs @@ -256,6 +256,13 @@ impl OrkState { doc.all_sections().find(|s| self.section_id(s) == id) } + /// Whether a section with the given ID exists anywhere in the document. + pub fn section_exists(&self, doc_name: &str, id: &str) -> bool { + self.get_doc(doc_name) + .map(|d| self.find_section(&d.doc, id).is_some()) + .unwrap_or(false) + } + /// Get section headline. pub fn section_headline(&self, doc_name: &str, id: &str) -> Option { self.get_doc(doc_name).and_then(|d| { @@ -790,30 +797,38 @@ impl OrkState { if !headline.starts_with('*') { return Err(io::Error::new(io::ErrorKind::InvalidInput, "headline must start with *")); } - + // Generate UUID let uuid = uuid::Uuid::new_v4().to_string(); - - // Build section text with ID property - let section_text = format!( - "{}\n:PROPERTIES:\n:ID: {}\n:END:\n", - headline, uuid - ); - + // Find insertion point let mut content = doc_state.content.clone(); - + if let Some(pid) = parent_id { - // Find parent section and insert after it - if let Some(parent) = self.find_section(&doc_state.doc, pid) { - // Insert at end of parent's span (before any siblings) - let insert_pos = self.find_section_end(&content, parent); - content.insert_str(insert_pos, §ion_text); + // Find parent, demote the headline to parent.level + 1, and insert + // after the parent's full span (content, drawer, and any children). + let parent = self.find_section(&doc_state.doc, pid) + .ok_or_else(|| io::Error::new(io::ErrorKind::NotFound, "parent section not found"))?; + let child_level = parent.headline.level as usize + 1; + let child_headline = restar(headline, child_level); + let section_text = format!( + "{}\n:PROPERTIES:\n:ID: {}\n:END:\n", + child_headline, uuid + ); + let insert_pos = self.find_section_end(&content, parent); + // Ensure the insertion begins on its own line. + if insert_pos > 0 && !content[..insert_pos].ends_with('\n') { + content.insert(insert_pos, '\n'); + content.insert_str(insert_pos + 1, §ion_text); } else { - return Err(io::Error::new(io::ErrorKind::NotFound, "parent section not found")); + content.insert_str(insert_pos, §ion_text); } } else { - // Append to end of file + // Append a top-level section to the end of the file. + let section_text = format!( + "{}\n:PROPERTIES:\n:ID: {}\n:END:\n", + headline, uuid + ); if !content.ends_with('\n') { content.push('\n'); } @@ -835,18 +850,22 @@ impl OrkState { Ok(uuid) } - /// Find the end position of a section (after its content, before siblings). + /// Find the end position of a section: the offset just past all of its + /// content and descendants, where a new child/sibling can be inserted. fn find_section_end(&self, content: &str, section: &org_ast::Section) -> usize { - // If section has children, insert after last child - if let Some(last_child) = section.children.last() { - return self.find_section_end(content, last_child); - } - - // Otherwise, use section's span end - section.headline.span.end.offset.min(content.len()) + // The section span already covers its planning line, property drawer, + // body, and all nested children. + section.span.end.offset.min(content.len()) } } +/// Replace the leading run of `*` in a headline with exactly `level` stars, +/// preserving the rest of the line. +fn restar(headline: &str, level: usize) -> String { + let rest = headline.trim_start_matches('*').trim_start(); + format!("{} {}", "*".repeat(level), rest) +} + /// Slugify a string for use as an ID. fn slugify(s: &str) -> String { s.to_lowercase() @@ -869,4 +888,50 @@ mod tests { assert_eq!(slugify("TODO: Fix bug #123"), "todo-fix-bug-123"); assert_eq!(slugify(" multiple spaces "), "multiple-spaces"); } + + #[test] + fn test_restar() { + assert_eq!(restar("* TODO Task", 2), "** TODO Task"); + assert_eq!(restar("*** Deep", 1), "* Deep"); + assert_eq!(restar("* Extra spaces", 2), "** Extra spaces"); + } + + #[test] + fn test_create_child_section_nests_and_preserves_parent() { + use std::fs; + use tempfile::tempdir; + + let dir = tempdir().unwrap(); + let path = dir.path().join("w.org"); + fs::write( + &path, + "#+TITLE: W\n* TODO Parent\n:PROPERTIES:\n:ID: parent-id\n:END:\nParent body.\n", + ) + .unwrap(); + + let state = OrkState::new(dir.path()).unwrap(); + + // Create a child under the parent (parent addressed by its :ID:). + let child_uuid = state + .create_child_section("w", "parent-id", "* NEXT Child") + .unwrap(); + + let content = fs::read_to_string(&path).unwrap(); + + // Parent keeps its ID drawer and body, still resolvable. + assert!(content.contains(":ID: parent-id")); + assert!(content.contains("Parent body.")); + assert!(state.section_exists("w", "parent-id")); + + // Child is demoted to level 2 and inserted after the parent's content. + assert!(content.contains("** NEXT Child")); + let parent_pos = content.find("* TODO Parent").unwrap(); + let body_pos = content.find("Parent body.").unwrap(); + let child_pos = content.find("** NEXT Child").unwrap(); + assert!(child_pos > body_pos, "child must come after parent body"); + assert!(child_pos > parent_pos); + + // The child is walkable by its generated UUID. + assert!(state.section_exists("w", &child_uuid)); + } }