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.
This commit is contained in:
Levi Neely 2026-09-30 21:28:52 +02:00
parent 17f4caa37f
commit 623aef4f42
2 changed files with 103 additions and 28 deletions

View File

@ -13,7 +13,7 @@ pub fn build_namespace(state: Arc<OrkState>) -> FsNode {
virtfs::dir_dynamic("/", virtfs::dir_dynamic("/",
vec![ vec![
// ctl - control commands // ctl - control commands
virtfs::file("ctl", 0o222, virtfs::rdwr({ virtfs::file("ctl", 0o666, virtfs::rdwr({
let state = state.clone(); let state = state.clone();
move |data| { move |data| {
let cmd = String::from_utf8_lossy(data); let cmd = String::from_utf8_lossy(data);
@ -35,7 +35,7 @@ pub fn build_namespace(state: Arc<OrkState>) -> FsNode {
})), })),
// new - create new org file // new - create new org file
virtfs::file("new", 0o222, virtfs::rdwr({ virtfs::file("new", 0o666, virtfs::rdwr({
let state = state.clone(); let state = state.clone();
move |data| { move |data| {
let name = String::from_utf8_lossy(data); let name = String::from_utf8_lossy(data);
@ -63,6 +63,9 @@ pub fn build_namespace(state: Arc<OrkState>) -> FsNode {
// Dynamic children: document directories // Dynamic children: document directories
move || Ok(s.doc_names()), move || Ok(s.doc_names()),
move |name| { move |name| {
if s2.get_doc(name).is_none() {
return None;
}
let doc_name = name.to_string(); let doc_name = name.to_string();
Some(build_doc_node(s2.clone(), &doc_name)) Some(build_doc_node(s2.clone(), &doc_name))
}, },
@ -133,7 +136,7 @@ fn build_doc_node(state: Arc<OrkState>, doc_name: &str) -> FsNode {
})), })),
// new - create top-level section // new - create top-level section
virtfs::file("new", 0o222, virtfs::rdwr({ virtfs::file("new", 0o666, virtfs::rdwr({
let state = state.clone(); let state = state.clone();
let name = name.clone(); let name = name.clone();
move |data| { move |data| {
@ -146,6 +149,9 @@ fn build_doc_node(state: Arc<OrkState>, doc_name: &str) -> FsNode {
// Dynamic section directories // Dynamic section directories
move || Ok(s.section_ids(&n)), move || Ok(s.section_ids(&n)),
move |id| { move |id| {
if !s2.section_exists(&n2, id) {
return None;
}
Some(build_section_node(s2.clone(), &n2, id)) Some(build_section_node(s2.clone(), &n2, id))
}, },
) )
@ -238,6 +244,7 @@ fn build_section_node(state: Arc<OrkState>, doc_name: &str, section_id: &str) ->
let i_new = id.clone(); let i_new = id.clone();
let i_list = id.clone(); let i_list = id.clone();
let i_child = id.clone();
virtfs::dir_dynamic(&id, virtfs::dir_dynamic(&id,
vec![ vec![
@ -373,7 +380,7 @@ fn build_section_node(state: Arc<OrkState>, doc_name: &str, section_id: &str) ->
)), )),
// new - create child section // 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); let headline = String::from_utf8_lossy(data);
s_new.create_child_section(&d_new, &i_new, &headline) s_new.create_child_section(&d_new, &i_new, &headline)
.map(|uuid| format!("{}\n", uuid).into_bytes()) .map(|uuid| format!("{}\n", uuid).into_bytes())
@ -382,6 +389,9 @@ fn build_section_node(state: Arc<OrkState>, doc_name: &str, section_id: &str) ->
// children - nested sections // children - nested sections
move || Ok(s_list.child_ids(&d_list, &i_list)), move || Ok(s_list.child_ids(&d_list, &i_list)),
move |child_id| { 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)) Some(build_section_node(s_child.clone(), &d_child, child_id))
}, },
) )

View File

@ -256,6 +256,13 @@ impl OrkState {
doc.all_sections().find(|s| self.section_id(s) == id) 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. /// Get section headline.
pub fn section_headline(&self, doc_name: &str, id: &str) -> Option<String> { pub fn section_headline(&self, doc_name: &str, id: &str) -> Option<String> {
self.get_doc(doc_name).and_then(|d| { self.get_doc(doc_name).and_then(|d| {
@ -794,26 +801,34 @@ impl OrkState {
// Generate UUID // Generate UUID
let uuid = uuid::Uuid::new_v4().to_string(); 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 // Find insertion point
let mut content = doc_state.content.clone(); let mut content = doc_state.content.clone();
if let Some(pid) = parent_id { if let Some(pid) = parent_id {
// Find parent section and insert after it // Find parent, demote the headline to parent.level + 1, and insert
if let Some(parent) = self.find_section(&doc_state.doc, pid) { // after the parent's full span (content, drawer, and any children).
// Insert at end of parent's span (before any siblings) 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); let insert_pos = self.find_section_end(&content, parent);
content.insert_str(insert_pos, &section_text); // 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, &section_text);
} else { } else {
return Err(io::Error::new(io::ErrorKind::NotFound, "parent section not found")); content.insert_str(insert_pos, &section_text);
} }
} else { } 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') { if !content.ends_with('\n') {
content.push('\n'); content.push('\n');
} }
@ -835,16 +850,20 @@ impl OrkState {
Ok(uuid) 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 { fn find_section_end(&self, content: &str, section: &org_ast::Section) -> usize {
// If section has children, insert after last child // The section span already covers its planning line, property drawer,
if let Some(last_child) = section.children.last() { // body, and all nested children.
return self.find_section_end(content, last_child); section.span.end.offset.min(content.len())
}
} }
// Otherwise, use section's span end /// Replace the leading run of `*` in a headline with exactly `level` stars,
section.headline.span.end.offset.min(content.len()) /// 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. /// Slugify a string for use as an ID.
@ -869,4 +888,50 @@ mod tests {
assert_eq!(slugify("TODO: Fix bug #123"), "todo-fix-bug-123"); assert_eq!(slugify("TODO: Fix bug #123"), "todo-fix-bug-123");
assert_eq!(slugify(" multiple spaces "), "multiple-spaces"); 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));
}
} }