Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions src/forge/azure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down Expand Up @@ -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 {
Expand Down
10 changes: 2 additions & 8 deletions src/forge/bitbucket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
16 changes: 6 additions & 10 deletions src/forge/github.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,10 @@ struct CheckRun {
}

impl Forge for GitHub {
fn rich_markdown(&self) -> bool {
true
}

async fn fetch_pr_meta(&self) -> Result<PrMeta> {
let resp = self
.request(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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),
Expand Down
14 changes: 6 additions & 8 deletions src/forge/gitlab.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,10 @@ impl GitLab {
}

impl Forge for GitLab {
fn rich_markdown(&self) -> bool {
true
}

async fn fetch_pr_meta(&self) -> Result<PrMeta> {
let mr = self.mr().await?;
Ok(PrMeta {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
113 changes: 103 additions & 10 deletions src/forge/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ pub enum CheckState {

#[allow(async_fn_in_trait)]
pub trait Forge {
/// True when the forge renders inline HTML `<img>` 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<PrMeta>;
/// Unified diff of the full PR.
async fn fetch_diff(&self) -> Result<String>;
Expand Down Expand Up @@ -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,
Expand All @@ -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()
));
}
Expand All @@ -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("<img"));
assert!(body.contains("`error` · confidence 0.91 · kind: risk"));
}

#[test]
fn silent_summary_icon_only_when_rich() {
let env = Envelope {
version: 1,
summary: String::new(),
silent: true,
findings: vec![],
resolved: vec![],
counts: Default::default(),
confidence_buckets: [0; 5],
gate: crate::envelope::Gate {
fail_on: "error".into(),
failing: false,
},
model_used: "m".into(),
usage: Default::default(),
duration_ms: 0,
base_sha: None,
head_sha: None,
since_sha: None,
};
assert!(check_summary(&env, true).contains("status/pass.svg"));
assert!(!check_summary(&env, false).contains("<img"));
}
}
14 changes: 11 additions & 3 deletions src/review.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,11 +392,19 @@ async fn finish<F: Forge>(
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
Expand Down
Loading