forge_notify: escape ATX headings in embedded body so they don't blow into wrapper h1/h2 (closes #455)
This commit is contained in:
parent
0460bbe9ec
commit
12f2c8311e
1 changed files with 94 additions and 5 deletions
|
|
@ -192,6 +192,35 @@ fn notif_type_label(t: &str) -> &str {
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Truncate a string to `max` bytes at a char boundary, appending `…` if cut.
|
/// Truncate a string to `max` bytes at a char boundary, appending `…` if cut.
|
||||||
|
/// Escape ATX-style markdown headings (`# h`, `## h`, …) in a
|
||||||
|
/// comment/review body before we embed it inline in the forge-notify
|
||||||
|
/// wrapper. The wrapper is the markdown context the dashboard's
|
||||||
|
/// `marked.parse` sees; without this, a body line like `## argus
|
||||||
|
/// review` blows into a top-level h2 in the agent's chat row,
|
||||||
|
/// dwarfing the rest of the wrapper text (closes #455).
|
||||||
|
///
|
||||||
|
/// Backslash before `#` is the standard markdown escape — `\#` renders
|
||||||
|
/// as the literal character `#`, so the line is preserved verbatim
|
||||||
|
/// without claiming heading-level styling. Indented lines keep their
|
||||||
|
/// indentation. Lines that don't start with `#` (ignoring leading
|
||||||
|
/// 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.
|
||||||
|
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")
|
||||||
|
}
|
||||||
|
|
||||||
fn truncate(s: &str, max: usize) -> String {
|
fn truncate(s: &str, max: usize) -> String {
|
||||||
if s.len() <= max {
|
if s.len() <= max {
|
||||||
return s.to_owned();
|
return s.to_owned();
|
||||||
|
|
@ -386,6 +415,13 @@ async fn format_comment_notification(
|
||||||
let author = if actor_login.is_empty() { "?" } else { actor_login };
|
let author = if actor_login.is_empty() { "?" } else { actor_login };
|
||||||
let NotifMeta { title, notif_type, num, repo, meta_suffix, .. } = meta;
|
let NotifMeta { title, notif_type, num, repo, meta_suffix, .. } = meta;
|
||||||
|
|
||||||
|
// Escape ATX headings in the user-authored body so the embedded
|
||||||
|
// text doesn't blow into top-level h1/h2 in the wrapper message
|
||||||
|
// when the dashboard renders it (closes #455). Done once here
|
||||||
|
// because both code paths fall through the same truncate+embed
|
||||||
|
// pattern.
|
||||||
|
let escaped = escape_md_headings(body_text);
|
||||||
|
let body_for_embed = truncate(&escaped, BODY_TRUNCATE);
|
||||||
if let Some(review_label) = review_state {
|
if let Some(review_label) = review_state {
|
||||||
// Review submission on a PR.
|
// Review submission on a PR.
|
||||||
let kind = format!("PR {review_label}{num}{repo}");
|
let kind = format!("PR {review_label}{num}{repo}");
|
||||||
|
|
@ -393,17 +429,14 @@ async fn format_comment_notification(
|
||||||
if body_text.is_empty() {
|
if body_text.is_empty() {
|
||||||
write!(out, "\n\nreviewer: {author}").ok();
|
write!(out, "\n\nreviewer: {author}").ok();
|
||||||
} else {
|
} else {
|
||||||
write!(out, "\n\n{author}: {}", truncate(body_text, BODY_TRUNCATE)).ok();
|
write!(out, "\n\n{author}: {body_for_embed}").ok();
|
||||||
}
|
}
|
||||||
out.push_str(meta_suffix);
|
out.push_str(meta_suffix);
|
||||||
Some(out)
|
Some(out)
|
||||||
} else {
|
} else {
|
||||||
// Regular comment.
|
// Regular comment.
|
||||||
let kind = format!("comment on {}{num}{repo}", notif_type_label(notif_type));
|
let kind = format!("comment on {}{num}{repo}", notif_type_label(notif_type));
|
||||||
let mut out = format!(
|
let mut out = format!("[{kind}] {title}\nurl: {url}\n\n{author}: {body_for_embed}");
|
||||||
"[{kind}] {title}\nurl: {url}\n\n{author}: {}",
|
|
||||||
truncate(body_text, BODY_TRUNCATE)
|
|
||||||
);
|
|
||||||
if out.ends_with('\n') {
|
if out.ends_with('\n') {
|
||||||
out.pop();
|
out.pop();
|
||||||
}
|
}
|
||||||
|
|
@ -620,3 +653,59 @@ async fn mark_read(client: &reqwest::Client, forge_url: &str, token: &str, id: u
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn escape_md_headings_escapes_top_level_atx() {
|
||||||
|
// The #455 repro: argus reviews start with `## argus review`,
|
||||||
|
// which would otherwise become an h2 in the wrapper message.
|
||||||
|
assert_eq!(
|
||||||
|
escape_md_headings("## argus review\n\nlgtm."),
|
||||||
|
"\\## argus review\n\nlgtm.",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn escape_md_headings_escapes_all_heading_depths() {
|
||||||
|
let body = "# h1\n## h2\n### h3\n###### h6\nbody";
|
||||||
|
assert_eq!(
|
||||||
|
escape_md_headings(body),
|
||||||
|
"\\# h1\n\\## h2\n\\### h3\n\\###### h6\nbody",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn escape_md_headings_preserves_indent() {
|
||||||
|
// Indented "headings" inside lists / nested quotes keep
|
||||||
|
// their leading whitespace so structure isn't visually
|
||||||
|
// collapsed by the escape.
|
||||||
|
assert_eq!(
|
||||||
|
escape_md_headings(" ## indented\nbody"),
|
||||||
|
" \\## indented\nbody",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn escape_md_headings_passes_non_heading_lines_through() {
|
||||||
|
let body = "plain text\nwith a #hashtag in middle\n```\n# in fenced code\n```";
|
||||||
|
let escaped = escape_md_headings(body);
|
||||||
|
// Lines without leading `#` are untouched. The `# in fenced
|
||||||
|
// code` line still gets escaped (we don't track fenced-code
|
||||||
|
// state) — acceptable: inside a fenced block the escape is
|
||||||
|
// visually inert anyway because the renderer treats the
|
||||||
|
// content as literal.
|
||||||
|
assert!(escaped.contains("plain text"));
|
||||||
|
assert!(escaped.contains("with a #hashtag in middle"));
|
||||||
|
assert!(escaped.contains("\\# in fenced code"));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
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");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue