diff --git a/src/forge/azure.rs b/src/forge/azure.rs index b979f9d..6792bc2 100644 --- a/src/forge/azure.rs +++ b/src/forge/azure.rs @@ -252,10 +252,7 @@ impl Azure { "comments": [{ "parentCommentId": 0, "commentType": 1, - "content": format!( - "**{}** ({} / {} confidence)\n\n{}", - f.title, f.severity.as_str(), super::format_confidence(f.confidence), f.body - ), + "content": super::finding_comment_body(f, false), }], "status": 1, "threadContext": { @@ -422,7 +419,7 @@ impl Forge for Azure { &head, "postil/review", map(advisory), - &check_summary(envelope), + &check_summary(envelope, false), ) .await?; let gate_desc = if envelope.gate.failing { diff --git a/src/forge/bitbucket.rs b/src/forge/bitbucket.rs index 1642a3c..ab02b9f 100644 --- a/src/forge/bitbucket.rs +++ b/src/forge/bitbucket.rs @@ -113,13 +113,7 @@ impl Bitbucket { /// API refuses the inline position. async fn post_finding(&self, f: &Finding) -> Result<()> { let body = json!({ - "content": { "raw": format!( - "**{}** ({} / {} confidence)\n\n{}", - f.title, - f.severity.as_str(), - super::format_confidence(f.confidence), - f.body - )}, + "content": { "raw": super::finding_comment_body(f, false) }, "inline": { "path": f.path, "to": f.line }, }); let resp = self @@ -298,7 +292,7 @@ impl Forge for Bitbucket { &head, "postil/review", map(advisory), - &check_summary(envelope), + &check_summary(envelope, false), ) .await?; let gate_desc = if envelope.gate.failing { diff --git a/src/forge/github.rs b/src/forge/github.rs index 0947f95..7c7c1f3 100644 --- a/src/forge/github.rs +++ b/src/forge/github.rs @@ -107,6 +107,10 @@ struct CheckRun { } impl Forge for GitHub { + fn rich_markdown(&self) -> bool { + true + } + async fn fetch_pr_meta(&self) -> Result { let resp = self .request( @@ -161,15 +165,7 @@ impl Forge for GitHub { "path": f.path, "line": f.line, "side": "RIGHT", - "body": format!( - "{} **{}**\n`{}` · confidence {} · kind: {}\n\n{}", - super::severity_icon(f.severity), - f.title, - f.severity.as_str(), - super::format_confidence(f.confidence), - f.kind.as_str(), - f.body - ), + "body": super::finding_comment_body(f, true), }); if let Some(end) = f.end_line && end > f.line @@ -282,7 +278,7 @@ impl Forge for GitHub { format!("Gate (`failOn: {}`) passing.\n", envelope.gate.fail_on) } } else { - check_summary(envelope) + check_summary(envelope, true) }; let mut output = json!({ "title": check_title(envelope), diff --git a/src/forge/gitlab.rs b/src/forge/gitlab.rs index adf5868..dacdc42 100644 --- a/src/forge/gitlab.rs +++ b/src/forge/gitlab.rs @@ -147,6 +147,10 @@ impl GitLab { } impl Forge for GitLab { + fn rich_markdown(&self) -> bool { + true + } + async fn fetch_pr_meta(&self) -> Result { let mr = self.mr().await?; Ok(PrMeta { @@ -208,13 +212,7 @@ impl Forge for GitLab { continue; } let body = json!({ - "body": format!( - "**{}** ({} / {} confidence)\n\n{}", - f.title, - f.severity.as_str(), - super::format_confidence(f.confidence), - f.body - ), + "body": super::finding_comment_body(f, true), "position": { "position_type": "text", "base_sha": mr.diff_refs.base_sha, @@ -299,7 +297,7 @@ impl Forge for GitLab { &head, "postil/review", map(advisory), - &check_summary(envelope), + &check_summary(envelope, true), ) .await?; let gate_desc = if envelope.gate.failing { diff --git a/src/forge/mod.rs b/src/forge/mod.rs index cf84771..baf1b8b 100644 --- a/src/forge/mod.rs +++ b/src/forge/mod.rs @@ -56,6 +56,11 @@ pub enum CheckState { #[allow(async_fn_in_trait)] pub trait Forge { + /// True when the forge renders inline HTML `` in markdown comments + /// (GitHub, GitLab). Forges that show raw HTML get text-only statuslines. + fn rich_markdown(&self) -> bool { + false + } async fn fetch_pr_meta(&self) -> Result; /// Unified diff of the full PR. async fn fetch_diff(&self) -> Result; @@ -87,23 +92,33 @@ pub fn check_title(envelope: &Envelope) -> String { } } -pub fn check_summary(envelope: &Envelope) -> String { +pub fn check_summary(envelope: &Envelope, rich: bool) -> String { let mut s = String::new(); + let pass = |s: &mut String| { + if rich { + s.push_str(&icon_md("pass")); + s.push(' '); + } + }; if envelope.silent { - s.push_str(&format!( - "{} Postil reviewed this change and found nothing that affects the merge \ - decision.\n", - icon_md("pass") - )); + pass(&mut s); + s.push_str( + "Postil reviewed this change and found nothing that affects the merge decision.\n", + ); } else { if !envelope.summary.is_empty() { s.push_str(&envelope.summary); s.push_str("\n\n"); } for f in &envelope.findings { + let icon = if rich { + format!("{} ", severity_icon(f.severity)) + } else { + String::new() + }; s.push_str(&format!( - "- {} **{}** `{}:{}` — {} · confidence {} · kind: {}\n", - severity_icon(f.severity), + "- {}**{}** `{}:{}` — {} · confidence {} · kind: {}\n", + icon, f.severity.as_str(), f.path, f.line, @@ -114,9 +129,10 @@ pub fn check_summary(envelope: &Envelope) -> String { } } if !envelope.resolved.is_empty() { + s.push('\n'); + pass(&mut s); s.push_str(&format!( - "\n{} {} finding(s) from the previous review resolved.\n", - icon_md("pass"), + "{} finding(s) from the previous review resolved.\n", envelope.resolved.len() )); } @@ -130,8 +146,85 @@ pub fn check_summary(envelope: &Envelope) -> String { s } +/// The body of one inline finding comment: icon (rich forges), bold title, +/// severity / confidence / kind statusline, then the finding body. +pub fn finding_comment_body(f: &Finding, rich: bool) -> String { + let icon = if rich { + format!("{} ", severity_icon(f.severity)) + } else { + String::new() + }; + format!( + "{}**{}**\n`{}` · confidence {} · kind: {}\n\n{}", + icon, + f.title, + f.severity.as_str(), + format_confidence(f.confidence), + f.kind.as_str(), + f.body + ) +} + /// Confidence rendered as the product statusline shows it: a bare decimal /// probability ("0.91"), not a percentage. pub fn format_confidence(c: f64) -> String { format!("{:.2}", c) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::envelope::{Kind, Severity}; + + fn finding() -> Finding { + Finding { + path: "src/auth.rs".into(), + line: 41, + end_line: None, + severity: Severity::Error, + kind: Kind::Risk, + confidence: 0.91, + title: "Unsanitized input reaches query".into(), + body: "user_input flows into exec_query.".into(), + } + } + + #[test] + fn rich_comment_carries_brand_icon_and_statusline() { + let body = finding_comment_body(&finding(), true); + assert!(body.contains("https://postil.dev/status/error.svg")); + assert!(body.contains("confidence 0.91 · kind: risk")); + } + + #[test] + fn plain_comment_has_statusline_without_html() { + let body = finding_comment_body(&finding(), false); + assert!(!body.contains("( let should_comment = !envelope.silent || matches!(cfg.on_clean, crate::config::OnClean::Comment); if should_comment { + let rich = forge.rich_markdown(); let summary = if envelope.silent { - "Postil reviewed this change and found nothing that affects the merge decision." - .to_string() + let icon = if rich { + format!("{} ", crate::forge::icon_md("pass")) + } else { + String::new() + }; + format!( + "{icon}Postil reviewed this change and found nothing that affects the \ + merge decision." + ) } else { - check_summary(&envelope) + check_summary(&envelope, rich) }; let head = envelope.head_sha.clone().unwrap_or_default(); forge