forge_notify: strict ATX detection + preserve trailing newline (argus nits on #518)

hive-ag3nt/src/forge_notify.rs

@@ -206,19 +206,49 @@ fn notif_type_label(t: &str) -> &str {
 /// whitespace) are passed through unchanged. Setext-style headings
 /// (`heading\n===`) are not handled here — rarer in practice and
 /// would need multi-line lookahead; revisit if it actually shows up.
+///
+/// **ATX shape strictly:** CommonMark requires a space (or end-of-line)
+/// after the 1-6 leading `#`s to count as an ATX heading. Lines like
+/// `#tag`, `#123`, `#!/bin/bash` are NOT headings — passing them through
+/// untouched avoids the cosmetic noise argus flagged on PR #518 (`\#tag`
+/// renders the same as `#tag`, but the escape is unnecessary).
+///
+/// **Trailing newline preserved:** `split_inclusive('\n')` keeps each
+/// line's terminator so the join round-trips a body that ended in `\n`.
 fn escape_md_headings(body: &str) -> String {
-    body.lines()
-        .map(|line| {
-            let trimmed = line.trim_start();
-            if trimmed.starts_with('#') {
-                let lead = &line[..line.len() - trimmed.len()];
-                format!("{lead}\\{trimmed}")
-            } else {
-                line.to_owned()
-            }
-        })
-        .collect::<Vec<_>>()
-        .join("\n")
+    let mut out = String::with_capacity(body.len());
+    for line in body.split_inclusive('\n') {
+        let (content, terminator) = match line.strip_suffix('\n') {
+            Some(rest) => (rest, "\n"),
+            None => (line, ""),
+        };
+        let trimmed = content.trim_start();
+        if is_atx_heading(trimmed) {
+            let lead = &content[..content.len() - trimmed.len()];
+            out.push_str(lead);
+            out.push('\\');
+            out.push_str(trimmed);
+        } else {
+            out.push_str(content);
+        }
+        out.push_str(terminator);
+    }
+    out
+}
+
+/// Strict CommonMark ATX-heading detector: 1-6 leading `#`s followed
+/// by either a space, tab, or end-of-line. Anything tighter (`#tag`,
+/// `#123`) is a non-heading line that the renderer will not promote.
+fn is_atx_heading(line: &str) -> bool {
+    let hashes = line.bytes().take_while(|&b| b == b'#').count();
+    if !(1..=6).contains(&hashes) {
+        return false;
+    }
+    match line.as_bytes().get(hashes) {
+        None => true,                            // bare `#` / `##` / ... on its own line
+        Some(b' ') | Some(b'\t') => true,        // proper ATX with space/tab after #s
+        _ => false,                              // `#tag` / `#123` — not a heading
+    }
 }
 
 fn truncate(s: &str, max: usize) -> String {
@@ -706,6 +736,40 @@ mod tests {
     fn escape_md_headings_handles_empty_and_whitespace_only() {
         assert_eq!(escape_md_headings(""), "");
         assert_eq!(escape_md_headings("   "), "   ");
-        assert_eq!(escape_md_headings("\n\n"), "\n");
+        assert_eq!(escape_md_headings("\n\n"), "\n\n");
+    }
+
+    #[test]
+    fn escape_md_headings_skips_non_atx_hash_lines() {
+        // ATX requires a space after the `#`s. Lines like `#tag`,
+        // `#123`, `#!/bin/bash` are NOT headings — argus's PR #518
+        // yellow nit: don't add cosmetic noise where the renderer
+        // wouldn't promote the line in the first place.
+        let body = "#tag\n#123\n#!/bin/bash\n####### too many hashes\nbody";
+        let escaped = escape_md_headings(body);
+        // All four leading `#` lines pass through untouched: too few
+        // (still need space), seven `#`s (over the cap), shebang
+        // (no space).
+        assert_eq!(escaped, body);
+    }
+
+    #[test]
+    fn escape_md_headings_handles_bare_hash_lines() {
+        // `#` alone on a line IS a valid ATX (h1 with empty text) per
+        // CommonMark; escape it to match the renderer's behaviour.
+        assert_eq!(escape_md_headings("#"), "\\#");
+        assert_eq!(escape_md_headings("##"), "\\##");
+        assert_eq!(escape_md_headings("###"), "\\###");
+    }
+
+    #[test]
+    fn escape_md_headings_preserves_trailing_newline() {
+        // `split_inclusive('\n')` round-trips a body ending in a
+        // newline. Important for embedded forge-notify bodies whose
+        // source already terminates with `\n` — the wrapper's spacing
+        // otherwise gets eaten.
+        assert_eq!(escape_md_headings("## h\n"), "\\## h\n");
+        assert_eq!(escape_md_headings("body\n"), "body\n");
+        assert_eq!(escape_md_headings("no trailing"), "no trailing");
     }
 }