From 388010df709093c874fb91e878d26c7206ec27e2 Mon Sep 17 00:00:00 2001 From: Frank O'Hara Date: Sun, 9 Aug 2026 08:11:24 -0600 Subject: [PATCH] fix(add): keep multi-line agent notes intact through emit and parse `lash add --agent-note "$(printf 'first\nsecond')"` wrote ` @agent-note: first` followed by a bare, unindented `second`. The parser treats an unindented line as the end of the annotation block, so `lash show` afterwards returned only "first" and the rest of the note was gone, with exit code 0. `format_task_annotations` built the note with a single `format!` and pushed the result as one "line", so any newline in the value landed in the file with no indentation. It now writes the first line as the `@agent-note:` line and each continuation with the annotation indent, which is the shape `parser::annotations::parse_annotation_block` folds back into one value. Indentation alone is not enough for every value, because the folding is lossy in two ways. A blank line is skipped, so a note containing one comes back with the gap closed up. A line starting with `@` is read as the start of a new annotation, truncating the note and possibly inventing an annotation that was never intended. `MarkdownEmitter::check_agent_note` rejects both at validation time with `E_CREATE_INVALID_AGENT_NOTE`, naming the offending line, and the file is left untouched. That is the "reject rather than write something the parser will silently truncate" half of the fix. Leading whitespace on a continuation line is normalized instead of rejected. The parser trims it, so indentation is lost but no text is, and rejecting it would rule out ordinary things like an indented list inside a note. Tests: a round-trip property test over notes of one to four lines at four indent levels, emitting through `format_task_annotations` and reading back through `parse_annotation_block`; unit tests for the accept and reject sets and for continuation indentation; and three integration tests covering a two-line note surviving to `lash show`, and both rejection cases leaving the file byte-identical. --- crates/lash-cli/tests/add_command_test.rs | 97 ++++++++++++++ crates/lash-core/src/creation/emitter.rs | 133 +++++++++++++++++++- crates/lash-core/src/creation/validation.rs | 10 ++ crates/lash-types/src/creation_errors.rs | 13 ++ lash.index.md | 11 +- 5 files changed, 261 insertions(+), 3 deletions(-) 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