diff --git a/crates/lash-cli/tests/add_command_test.rs b/crates/lash-cli/tests/add_command_test.rs index dd41608..aea0d75 100644 --- a/crates/lash-cli/tests/add_command_test.rs +++ b/crates/lash-cli/tests/add_command_test.rs @@ -557,3 +557,100 @@ fn test_add_leaves_the_file_ending_in_a_newline() { "file ends mid-line, every later diff shows '\\ No newline at end of file':\n{content}" ); } + +// --------------------------------------------------------------------- +// `--agent-note` with an embedded newline +// +// The emitter built the note with a single `format!`, so a value holding +// a newline was written as one `@agent-note:` line followed by a bare +// unindented line. The parser treats an unindented line as the end of +// the annotation block, so everything after the first line was silently +// dropped, with exit code 0. +// --------------------------------------------------------------------- + +#[test] +fn test_add_multiline_agent_note_survives_a_round_trip() { + let project = project_with_tasks_file(); + index(&project); + + run_lash_command() + .arg("--root") + .arg(project.path()) + .arg("add") + .arg("Noted task") + .arg("--file") + .arg("tasks.md") + .arg("--id") + .arg("noted") + .arg("--agent-note") + .arg("first line\nsecond line") + .assert() + .success(); + + let content = fs::read_to_string(project.file_path("tasks.md")).unwrap(); + assert!( + content.contains(" @agent-note: first line\n second line"), + "continuation line was written without indentation:\n{content}" + ); + + // The parser has to read both lines back, not just the first. + index(&project); + run_lash_command() + .arg("--root") + .arg(project.path()) + .arg("show") + .arg("tasks#noted") + .assert() + .success() + .stdout(predicate::str::contains("first line")) + .stdout(predicate::str::contains("second line")); +} + +#[test] +fn test_add_rejects_an_agent_note_with_a_blank_line() { + let project = project_with_tasks_file(); + index(&project); + + let before = fs::read_to_string(project.file_path("tasks.md")).unwrap(); + + run_lash_command() + .arg("--root") + .arg(project.path()) + .arg("add") + .arg("Blank note task") + .arg("--file") + .arg("tasks.md") + .arg("--agent-note") + .arg("first line\n\nthird line") + .assert() + .failure() + .stderr(predicate::str::contains("E_CREATE_INVALID_AGENT_NOTE")); + + assert_eq!( + before, + fs::read_to_string(project.file_path("tasks.md")).unwrap(), + "file must be untouched when the note is rejected" + ); +} + +#[test] +fn test_add_rejects_an_agent_note_line_starting_with_an_annotation() { + let project = project_with_tasks_file(); + index(&project); + + run_lash_command() + .arg("--root") + .arg(project.path()) + .arg("add") + .arg("At note task") + .arg("--file") + .arg("tasks.md") + .arg("--agent-note") + .arg("first line\n@owner: someone") + .assert() + .failure() + .stderr(predicate::str::contains("E_CREATE_INVALID_AGENT_NOTE")); + + let content = fs::read_to_string(project.file_path("tasks.md")).unwrap(); + assert!(!content.contains("At note task")); +} diff --git a/crates/lash-core/src/creation/emitter.rs b/crates/lash-core/src/creation/emitter.rs index affe13a..7785d76 100644 --- a/crates/lash-core/src/creation/emitter.rs +++ b/crates/lash-core/src/creation/emitter.rs @@ -423,6 +423,23 @@ impl MarkdownEmitter { /// assert!(lines.iter().any(|l| l.contains("@depends-on:"))); /// assert!(lines.iter().any(|l| l.contains("@agent-note:"))); /// ``` + /// + /// A note spanning several lines becomes one `@agent-note:` line plus one + /// indented continuation line each, which is the shape the parser folds + /// back into a single value: + /// + /// ``` + /// use lash_core::creation::emitter::MarkdownEmitter; + /// + /// let lines = MarkdownEmitter::format_task_annotations( + /// 0, + /// None, + /// &[], + /// Some("first line\nsecond line"), + /// ); + /// + /// assert_eq!(lines, [" @agent-note: first line", " second line"]); + /// ``` #[must_use] pub fn format_task_annotations( indent: usize, @@ -444,14 +461,66 @@ impl MarkdownEmitter { lines.push(format!("{annotation_indent}@depends-on: {dep}")); } - // Add @agent-note if present + // Add @agent-note if present. A note may span several lines; each + // continuation has to carry the annotation indent or the parser will + // stop at it and silently drop the rest of the note (it treats an + // unindented line as the end of the annotation block). if let Some(note) = agent_note { - lines.push(format!("{annotation_indent}@agent-note: {note}")); + let mut note_lines = note.lines(); + let first = note_lines.next().unwrap_or(""); + lines.push(format!("{annotation_indent}@agent-note: {first}")); + for continuation in note_lines { + lines.push(format!("{annotation_indent}{continuation}")); + } } lines } + /// Whether an agent note survives being written and parsed back + /// + /// The parser folds indented continuation lines into a single value, but + /// the folding is lossy in two ways that matter here: it skips blank + /// lines, and it treats a line starting with `@` as the beginning of a new + /// annotation rather than as note text. A note containing either is + /// rejected at validation time instead of being written into a file the + /// parser will silently truncate. + /// + /// Leading whitespace on a continuation line is normalized away rather + /// than rejected: the parser trims it, so the indentation is lost but no + /// text is. + /// + /// # Errors + /// + /// Returns the reason the note cannot round-trip. + /// + /// # Examples + /// + /// ``` + /// use lash_core::creation::emitter::MarkdownEmitter; + /// + /// assert!(MarkdownEmitter::check_agent_note("one line").is_ok()); + /// assert!(MarkdownEmitter::check_agent_note("first\nsecond").is_ok()); + /// assert!(MarkdownEmitter::check_agent_note("first\n\nthird").is_err()); + /// assert!(MarkdownEmitter::check_agent_note("first\n@owner: me").is_err()); + /// ``` + pub fn check_agent_note(note: &str) -> Result<(), String> { + for (offset, line) in note.lines().enumerate().skip(1) { + let line_number = offset + 1; + if line.trim().is_empty() { + return Err(format!( + "line {line_number} is blank, and blank lines are dropped when the note is read back" + )); + } + if line.trim_start().starts_with('@') { + return Err(format!( + "line {line_number} starts with '@', which would be read back as a separate annotation" + )); + } + } + Ok(()) + } + /// Synthesize a task ID from the task title /// /// Converts the title to a slug-like format (lowercase, hyphens for spaces). @@ -923,4 +992,64 @@ mod tests { assert_eq!(insert_index(content, InsertAnchor::Line(2)), 1); assert_eq!(insert_index(content, InsertAnchor::Line(99)), 3); } + + /// Emit `note` as annotation lines, then read it back the way the parser + /// would when it encounters those lines under a task. + fn round_trip_agent_note(indent: usize, note: &str) -> Option { + let lines = MarkdownEmitter::format_task_annotations(indent, None, &[], Some(note)); + let block = crate::parser::annotations::parse_annotation_block( + lines.iter().map(String::as_str), + None, + ) + .expect("emitted annotation lines must parse"); + block.get_single("agent-note").map(str::to_string) + } + + #[test] + fn test_agent_note_round_trips_across_line_counts_and_indents() { + let notes = [ + "single line", + "first line\nsecond line", + "first\nsecond\nthird\nfourth", + "a note with: a colon\nand a second line", + "trailing words end here", + ]; + + for indent in 0..4 { + for note in notes { + assert_eq!( + round_trip_agent_note(indent, note).as_deref(), + Some(note), + "note did not survive a round trip at indent {indent}: {note:?}" + ); + } + } + } + + #[test] + fn test_agent_note_continuation_lines_carry_the_annotation_indent() { + let lines = MarkdownEmitter::format_task_annotations(1, None, &[], Some("first\nsecond")); + assert_eq!(lines, [" @agent-note: first", " second"]); + } + + #[test] + fn test_check_agent_note_accepts_what_round_trips() { + assert!(MarkdownEmitter::check_agent_note("one line").is_ok()); + assert!(MarkdownEmitter::check_agent_note("first\nsecond\nthird").is_ok()); + // An `@` on the *first* line is part of the value, not a new annotation. + assert!(MarkdownEmitter::check_agent_note("ask @someone").is_ok()); + } + + #[test] + fn test_check_agent_note_rejects_what_would_be_dropped() { + // A blank continuation line is skipped by the parser, so the note + // would come back with the gap closed up. + let err = MarkdownEmitter::check_agent_note("first\n\nthird").unwrap_err(); + assert!(err.contains("line 2"), "unexpected reason: {err}"); + + // A continuation starting with `@` is read back as its own annotation, + // truncating the note. + let err = MarkdownEmitter::check_agent_note("first\n@owner: me").unwrap_err(); + assert!(err.contains("line 2"), "unexpected reason: {err}"); + } } diff --git a/crates/lash-core/src/creation/validation.rs b/crates/lash-core/src/creation/validation.rs index 7175ae1..b45fa15 100644 --- a/crates/lash-core/src/creation/validation.rs +++ b/crates/lash-core/src/creation/validation.rs @@ -14,6 +14,8 @@ use std::collections::HashSet; use std::path::PathBuf; use std::sync::OnceLock; +use super::emitter::MarkdownEmitter; + // Constants const MAX_TITLE_LENGTH: usize = 200; @@ -143,6 +145,14 @@ impl TaskValidator { Self::validate_estimate(estimate, &mut errors); } + // Reject an agent note that would not survive being read back, rather + // than writing a file the parser silently truncates. + if let Some(ref note) = request.agent_note { + if let Err(reason) = MarkdownEmitter::check_agent_note(note) { + errors.push(TaskCreationError::InvalidAgentNote { reason }); + } + } + // Note: Owner validation is currently a no-op // Resolve parent and compute depth diff --git a/crates/lash-types/src/creation_errors.rs b/crates/lash-types/src/creation_errors.rs index 5a4d19f..af1b5eb 100644 --- a/crates/lash-types/src/creation_errors.rs +++ b/crates/lash-types/src/creation_errors.rs @@ -93,6 +93,12 @@ pub enum TaskCreationError { dependency: String, }, + /// The agent note cannot be written and read back unchanged + InvalidAgentNote { + /// Explanation of which part of the note does not survive a round trip + reason: String, + }, + /// The specified insert position is invalid InvalidPosition { /// Explanation of why the position is invalid @@ -155,6 +161,9 @@ impl TaskCreationError { "creating task '{task_id}' with dependency '{dependency}' would create a cycle" ) } + Self::InvalidAgentNote { reason } => { + format!("agent note cannot be stored: {reason}") + } Self::InvalidPosition { reason } => { format!("invalid insert position: {reason}") } @@ -213,6 +222,9 @@ impl TaskCreationError { Self::WouldCreateCycle { dependency, .. } => { format!("remove the dependency on '{dependency}' or restructure the task hierarchy") } + Self::InvalidAgentNote { .. } => { + "an agent note may span several lines, but each line must have non-whitespace content and must not begin with '@'".to_string() + } Self::InvalidPosition { .. } => { "use a valid position: Append, AtIndex, Before, or After with an existing task ID".to_string() } @@ -239,6 +251,7 @@ impl TaskCreationError { Self::InvalidEstimate { .. } => "E_CREATE_INVALID_ESTIMATE", Self::DependencyNotFound { .. } => "E_CREATE_DEPENDENCY_NOT_FOUND", Self::WouldCreateCycle { .. } => "E_CREATE_WOULD_CREATE_CYCLE", + Self::InvalidAgentNote { .. } => "E_CREATE_INVALID_AGENT_NOTE", Self::InvalidPosition { .. } => "E_CREATE_INVALID_POSITION", Self::IoError { .. } => "E_CREATE_IO_ERROR", } diff --git a/lash.index.md b/lash.index.md index b2c0908..4b98416 100644 --- a/lash.index.md +++ b/lash.index.md @@ -147,7 +147,7 @@ filed in the flawd repo under `tasks/tasks.fail-fast-degradation.md`. content, so it needs plumbing - Regression test: append after a task with comma-separated `@depends-on` followed by a section heading; assert the new task lands inside `## Tasks` -- [ ] `lash add --agent-note` with an embedded newline writes a malformed file and loses content #cli #bug #data-loss +- [x] `lash add --agent-note` with an embedded newline writes a malformed file and loses content #cli #bug #data-loss - Latent counterpart to the multi-line note fix, on the write side. `MarkdownEmitter::format_task_annotations` builds the note with `format!("{annotation_indent}@agent-note: {note}")` and pushes it as one @@ -166,6 +166,15 @@ filed in the flawd repo under `tasks/tasks.fail-fast-degradation.md`. - Regression test: emit a task with a two-line note, reparse the written file, assert both lines survive; plus a round-trip property test over notes with varying line counts and indentation + - Fixed on both halves. The emitter now writes each continuation line with + the annotation indent, which is the shape the parser folds back into one + value. Values that cannot round-trip regardless of indentation are + rejected up front by `MarkdownEmitter::check_agent_note`: a blank + continuation line (the parser skips blanks) or one starting with `@` (read + back as a separate annotation). Both produce + `E_CREATE_INVALID_AGENT_NOTE` and leave the file untouched. Leading + whitespace on a continuation is normalized rather than rejected, since the + parser trims it and no text is lost - [x] `lash add` prepends above the H1 when the target file has no tasks yet, and the task is never indexed #cli #bug #data-loss - Severe: the task is written to disk, invisible to Lash, and `lash lint` passes clean. Found 2026-08-08 while verifying the multi-line note fix