diff --git a/Cargo.lock b/Cargo.lock index d5bff32..fb19284 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1041,7 +1041,7 @@ checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" [[package]] name = "postil-cli" -version = "0.4.3" +version = "0.4.4" dependencies = [ "anyhow", "assert_cmd", diff --git a/Cargo.toml b/Cargo.toml index 4354eab..c9e7fa8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "postil-cli" -version = "0.4.3" +version = "0.4.4" edition = "2024" description = "Postil: a low-noise AI review gate. Silent on clean PRs, hard gate on real risk." license = "Apache-2.0" diff --git a/src/config.rs b/src/config.rs index 152aa20..31cc65c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -91,7 +91,8 @@ pub struct Config { pub gate_fail_on: GateLevel, /// Gate behavior on operational error. Default: fail closed. pub gate_on_error: OnError, - /// Finding kinds that block the gate regardless of severity. Default: [HumanEscalation]. + /// Finding kinds that block regardless of severity. Human escalations must + /// also meet the calibrated 0.30 gate confidence floor. Default: [HumanEscalation]. pub block_on_kinds: Vec, pub model: String, pub cascade: Vec, @@ -499,7 +500,7 @@ review: gate: failOn: error # the postil/gate check fails at/above: info | warn | error | never - # blockOnKinds: # finding kinds that block regardless of severity (default: [humanEscalation]) + # blockOnKinds: # kinds that block regardless of severity; humanEscalation also requires confidence >= 0.30 # - humanEscalation # onError: block # block (default, fail closed) | advisory — gate outcome when # # the review itself errors (model outage). advisory keeps an diff --git a/src/envelope.rs b/src/envelope.rs index 4414a12..30ef527 100644 --- a/src/envelope.rs +++ b/src/envelope.rs @@ -47,6 +47,15 @@ pub enum Kind { ContentPolicy, } +/// Minimum confidence at which a human-escalation finding can block a merge. +/// +/// Escalations are intentionally kept visible below this floor so reviewers can +/// inspect weak signals, but a very weak escalation must not turn into a hard +/// gate merely because of its kind or severity. A 0.30 floor is conservative: +/// it filters the near-zero boilerplate observed in production while retaining +/// uncertain but concrete concerns for human attention. +pub const HUMAN_ESCALATION_GATE_MIN_CONFIDENCE: f64 = 0.30; + impl Kind { pub fn parse(s: &str) -> Option { match s.trim().to_ascii_lowercase().as_str() { @@ -192,6 +201,33 @@ impl Envelope { } } +/// Apply the serialized gate contract to one finding. +/// +/// Keeping this beside the envelope makes live reviews, stored-envelope replay, +/// and forge summaries share exactly the same blocking semantics. +pub fn finding_blocks_gate( + finding: &Finding, + fail_on: &str, + block_on_kinds: &[String], + provider_error_is_advisory: bool, +) -> bool { + if finding.kind == Kind::HumanEscalation + && finding.confidence < HUMAN_ESCALATION_GATE_MIN_CONFIDENCE + { + return false; + } + + let severity_blocks = if provider_error_is_advisory || fail_on.eq_ignore_ascii_case("never") { + false + } else { + Severity::parse(fail_on).is_some_and(|threshold| finding.severity >= threshold) + }; + let kind_blocks = block_on_kinds + .iter() + .any(|kind| Kind::parse(kind).is_some_and(|kind| kind == finding.kind)); + severity_blocks || kind_blocks +} + /// Path marker for synthetic unusable-output findings (the model answered but /// the output could not be validated). A malicious diff can induce this class /// via prompt injection, so it always fails the gate — even under @@ -347,6 +383,29 @@ mod tests { assert_eq!(Kind::parse("unknown"), None); } + #[test] + fn weak_human_escalation_never_blocks_but_floor_does() { + let mut escalation = finding(Severity::Error, 0.05); + escalation.kind = Kind::HumanEscalation; + let block_on_kinds = vec!["humanEscalation".to_string()]; + + assert!(!finding_blocks_gate( + &escalation, + "error", + &block_on_kinds, + false + )); + + escalation.severity = Severity::Warn; + escalation.confidence = HUMAN_ESCALATION_GATE_MIN_CONFIDENCE; + assert!(finding_blocks_gate( + &escalation, + "error", + &block_on_kinds, + false + )); + } + #[test] fn envelope_serializes_camel_case() { let env = Envelope { diff --git a/src/forge/azure.rs b/src/forge/azure.rs index 83a8d13..4dc9465 100644 --- a/src/forge/azure.rs +++ b/src/forge/azure.rs @@ -424,7 +424,7 @@ impl Forge for Azure { &head, "postil/review", map(advisory), - &check_summary(envelope, false), + &check_summary(envelope, false, Default::default()), ) .await?; let gate_desc = if envelope.gate.failing { diff --git a/src/forge/bitbucket.rs b/src/forge/bitbucket.rs index f02f81e..31f826b 100644 --- a/src/forge/bitbucket.rs +++ b/src/forge/bitbucket.rs @@ -308,7 +308,7 @@ impl Forge for Bitbucket { &head, "postil/review", map(advisory), - &check_summary(envelope, false), + &check_summary(envelope, false, Default::default()), ) .await?; let gate_desc = if envelope.gate.failing { diff --git a/src/forge/github.rs b/src/forge/github.rs index a7138a1..c1ffab2 100644 --- a/src/forge/github.rs +++ b/src/forge/github.rs @@ -4,7 +4,10 @@ use anyhow::{Context, Result, anyhow}; use serde::Deserialize; use serde_json::json; -use super::{CheckState, Forge, PrMeta, ThreadKind, check_summary, check_title, wrap_plain_text}; +use super::{ + CheckState, Forge, PrMeta, SummaryContext, ThreadKind, check_summary, check_title, + wrap_plain_text, +}; use crate::envelope::{Envelope, Finding, Severity}; use crate::filter; @@ -16,6 +19,7 @@ pub struct GitHub { owner: String, repo: String, pr: u64, + web_base: String, } impl GitHub { @@ -28,6 +32,8 @@ impl GitHub { let api_base = std::env::var("GITHUB_API_URL") .unwrap_or_else(|_| "https://api.github.com".to_string()); let details_url = valid_details_url(std::env::var("POSTIL_DETAILS_URL").ok()); + let web_base = valid_details_url(std::env::var("GITHUB_SERVER_URL").ok()) + .unwrap_or_else(|| "https://github.com".to_string()); Ok(GitHub { http: reqwest::Client::builder() .timeout(std::time::Duration::from_secs(60)) @@ -38,6 +44,7 @@ impl GitHub { owner: owner.to_string(), repo: repo.to_string(), pr, + web_base: web_base.trim_end_matches('/').to_string(), }) } @@ -89,7 +96,7 @@ fn gate_title(envelope: &Envelope) -> &'static str { fn gate_summary(envelope: &Envelope) -> String { if !envelope.gate.failing { return format!( - "Merge gate passed: no findings at or above the blocking threshold ({}).\n", + "Merge gate passed: no findings block under the configured policy (failOn: {}).\n", envelope.gate.fail_on ); } @@ -98,20 +105,28 @@ fn gate_summary(envelope: &Envelope) -> String { .findings .iter() .filter(|f| { - crate::config::GateLevel::parse(&envelope.gate.fail_on) - .map(|g| g.fails(f.severity)) - .unwrap_or(false) + crate::envelope::finding_blocks_gate( + f, + &envelope.gate.fail_on, + &envelope.gate.block_on_kinds, + false, + ) }) .map(|f| format!("- `{}:{}` {}", f.path, f.line, f.title)) .collect(); - let noun = if failing.len() == 1 { - "finding" + if failing.is_empty() { + return format!( + "Merge gate failed under the configured operational error policy (failOn: {}).\n", + envelope.gate.fail_on + ); + } + let subject = if failing.len() == 1 { + "1 finding blocks".to_string() } else { - "findings" + format!("{} findings block", failing.len()) }; format!( - "Merge gate failed: {} {noun} at or above the blocking threshold ({}).\n\n{}\n", - failing.len(), + "Merge gate failed: {subject} under the configured policy (failOn: {}).\n\n{}\n", envelope.gate.fail_on, failing.join("\n") ) @@ -140,6 +155,23 @@ impl Forge for GitHub { true } + fn review_summary(&self, envelope: &Envelope) -> String { + let commit_url = envelope.head_sha.as_deref().map(|sha| { + format!( + "{}/{}/{}/commit/{sha}", + self.web_base, self.owner, self.repo + ) + }); + check_summary( + envelope, + true, + SummaryContext { + commit_url: commit_url.as_deref(), + details_url: self.details_url.as_deref(), + }, + ) + } + async fn fetch_pr_meta(&self) -> Result { let resp = self .request( @@ -305,7 +337,7 @@ impl Forge for GitHub { let gate_note = if name == "postil/gate" { gate_summary(envelope) } else { - check_summary(envelope, true) + self.review_summary(envelope) }; let title = if name == "postil/gate" { gate_title(envelope).to_string() @@ -373,7 +405,8 @@ impl Forge for GitHub { #[cfg(test)] mod tests { - use super::valid_details_url; + use super::{gate_summary, valid_details_url}; + use crate::envelope::{Envelope, Finding, Gate, Kind, Severity, Usage}; #[test] fn details_url_accepts_only_http_and_https_urls() { @@ -390,4 +423,85 @@ mod tests { assert_eq!(valid_details_url(Some("not a URL".into())), None); assert_eq!(valid_details_url(None), None); } + + #[test] + fn gate_summary_counts_qualified_kind_blocks_from_stored_envelope() { + let finding = Finding { + path: "src/auth.rs".into(), + line: 41, + end_line: None, + severity: Severity::Warn, + kind: Kind::HumanEscalation, + confidence: 0.30, + generator_confidence: None, + scorer_confidence: None, + generator_kind: None, + scorer_kind: None, + scorer_reason: None, + title: "Human judgment required".into(), + body: "Concrete compatibility concern.".into(), + id: None, + }; + let env = Envelope { + version: 1, + summary: String::new(), + silent: false, + findings: vec![finding], + resolved: vec![], + counts: Default::default(), + confidence_buckets: [0; 5], + gate: Gate { + fail_on: "error".into(), + failing: true, + block_on_kinds: vec!["humanEscalation".into()], + }, + model_used: "m".into(), + scorer_model: None, + scorer_error: None, + scorer_disagreements: None, + usage: Usage::default(), + duration_ms: 0, + base_sha: None, + head_sha: None, + since_sha: None, + }; + + let summary = gate_summary(&env); + assert!(summary.contains("1 finding blocks under the configured policy")); + assert!(summary.contains("src/auth.rs:41")); + } + + #[test] + fn gate_summary_reports_operational_failure_without_inventing_blockers() { + let mut env = Envelope { + version: 1, + summary: String::new(), + silent: false, + findings: vec![], + resolved: vec![], + counts: Default::default(), + confidence_buckets: [0; 5], + gate: Gate { + fail_on: "never".into(), + failing: true, + block_on_kinds: vec![], + }, + model_used: "m".into(), + scorer_model: None, + scorer_error: None, + scorer_disagreements: None, + usage: Usage::default(), + duration_ms: 0, + base_sha: None, + head_sha: None, + since_sha: None, + }; + env.findings + .push(crate::envelope::provider_error_finding("timeout")); + + assert_eq!( + gate_summary(&env), + "Merge gate failed under the configured operational error policy (failOn: never).\n" + ); + } } diff --git a/src/forge/gitlab.rs b/src/forge/gitlab.rs index d668b5d..cd1f3c5 100644 --- a/src/forge/gitlab.rs +++ b/src/forge/gitlab.rs @@ -322,7 +322,7 @@ impl Forge for GitLab { &head, "postil/review", map(advisory), - &check_summary(envelope, true), + &check_summary(envelope, true, Default::default()), ) .await?; let gate_desc = if envelope.gate.failing { diff --git a/src/forge/mod.rs b/src/forge/mod.rs index 5079c3a..e57c410 100644 --- a/src/forge/mod.rs +++ b/src/forge/mod.rs @@ -72,6 +72,11 @@ pub trait Forge { fn rich_markdown(&self) -> bool { false } + /// Compose the top-level review body. Forges can add validated links to the + /// otherwise forge-neutral envelope metadata. + fn review_summary(&self, envelope: &Envelope) -> String { + check_summary(envelope, self.rich_markdown(), SummaryContext::default()) + } async fn fetch_pr_meta(&self) -> Result; /// Unified diff of the full PR. async fn fetch_diff(&self) -> Result; @@ -233,8 +238,19 @@ pub fn clean_review_message(envelope: &Envelope) -> &'static str { } } -pub fn check_summary(envelope: &Envelope, rich: bool) -> String { +#[derive(Default)] +pub struct SummaryContext<'a> { + pub commit_url: Option<&'a str>, + pub details_url: Option<&'a str>, +} + +pub fn check_summary(envelope: &Envelope, rich: bool, context: SummaryContext<'_>) -> String { let mut s = String::new(); + s.push_str(if envelope.gate.failing { + "Gate: failed\n\n" + } else { + "Gate: passed\n\n" + }); let pass = |s: &mut String| { if rich { s.push_str(&icon_md("pass")); @@ -256,15 +272,24 @@ pub fn check_summary(envelope: &Envelope, rich: bool) -> String { } else { String::new() }; + let escalation_note = if f.kind == crate::envelope::Kind::HumanEscalation + && f.confidence < crate::envelope::HUMAN_ESCALATION_GATE_MIN_CONFIDENCE + { + format!( + " · non-blocking below {:.2} escalation floor", + crate::envelope::HUMAN_ESCALATION_GATE_MIN_CONFIDENCE + ) + } else { + String::new() + }; s.push_str(&format!( - "- {}**{}** `{}:{}` — {} · confidence {} · kind: {}\n", + "- {}**{}** · severity: `{}` · confidence {} · kind: {}{}\n", icon, - f.severity.as_str(), - f.path, - f.line, f.title, + f.severity.as_str(), format_confidence(f.confidence), f.kind.as_str(), + escalation_note, )); } } @@ -282,10 +307,59 @@ pub fn check_summary(envelope: &Envelope, rich: bool) -> String { envelope.counts.suppressed )); } - s.push_str(&format!("\nModel: {}\n", envelope.model_used)); + if rich { + s.push_str("\n
Review metadata\n\n"); + } else { + s.push_str("\nReview metadata:\n"); + } + s.push_str(&format!("- Model: `{}`\n", envelope.model_used)); + s.push_str(&format!( + "- Review duration: {}\n", + format_duration(envelope.duration_ms) + )); + if let Some(head_sha) = envelope.head_sha.as_deref() { + let short_sha: String = head_sha.chars().take(7).collect(); + if let Some(commit_url) = context.commit_url { + s.push_str(&format!("- Commit: [`{short_sha}`]({commit_url})\n")); + } else { + s.push_str(&format!("- Commit: `{short_sha}`\n")); + } + } + if let Some(details_url) = context.details_url { + s.push_str(&format!( + "- Dashboard run: [View in Postil]({details_url})\n" + )); + } + s.push_str(&format!( + "- Tokens: {} prompt, {} completion\n", + envelope.usage.prompt_tokens, envelope.usage.completion_tokens + )); + if let Some(scorer_model) = envelope.scorer_model.as_deref() { + s.push_str(&format!("- Scorer: `{scorer_model}`")); + if let Some(disagreements) = envelope.scorer_disagreements { + s.push_str(&format!(" ({disagreements} disagreement(s))")); + } + s.push('\n'); + } + if envelope.scorer_error.is_some() { + // Provider response bodies can contain arbitrary text. Keep the public + // metadata useful without copying raw upstream errors into Markdown. + s.push_str("- Scorer status: unavailable\n"); + } + if rich { + s.push_str("\n
\n"); + } s } +fn format_duration(duration_ms: u64) -> String { + if duration_ms < 1_000 { + format!("{duration_ms} ms") + } else { + format!("{:.2} s", duration_ms as f64 / 1_000.0) + } +} + /// 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 { @@ -342,6 +416,92 @@ mod tests { assert!(body.contains("confidence 0.91 · kind: risk")); } + #[test] + fn summary_is_explicit_path_free_and_marks_weak_escalations_non_blocking() { + let mut escalation = finding(); + escalation.kind = Kind::HumanEscalation; + escalation.confidence = 0.05; + let env = Envelope { + version: 1, + summary: "A weak signal needs review.".into(), + silent: false, + findings: vec![escalation], + resolved: vec![], + counts: Default::default(), + confidence_buckets: [1, 0, 0, 0, 0], + gate: crate::envelope::Gate { + fail_on: "error".into(), + failing: false, + block_on_kinds: vec!["humanEscalation".into()], + }, + model_used: "review-model".into(), + scorer_model: Some("scorer-model".into()), + scorer_error: None, + scorer_disagreements: Some(1), + usage: crate::envelope::Usage { + prompt_tokens: 10, + completion_tokens: 5, + }, + duration_ms: 1_250, + base_sha: None, + head_sha: Some("abcdef123456".into()), + since_sha: None, + }; + + let summary = check_summary( + &env, + true, + SummaryContext { + commit_url: Some("https://github.com/acme/api/commit/abcdef123456"), + details_url: Some("https://postil.dev/orgs/acme/runs/run-1"), + }, + ); + + assert!(summary.starts_with("Gate: passed\n\n")); + assert!(summary.contains("**Unsanitized input reaches query** · severity: `error`")); + assert!(!summary.contains("src/auth.rs:41")); + assert!(summary.contains("non-blocking below 0.30 escalation floor")); + assert!(summary.contains("
Review metadata")); + assert!(summary.contains("[`abcdef1`](https://github.com/acme/api/commit/abcdef123456)")); + assert!(summary.contains("[View in Postil](https://postil.dev/orgs/acme/runs/run-1)")); + assert!(summary.contains("- Review duration: 1.25 s")); + assert!(summary.contains("- Tokens: 10 prompt, 5 completion")); + assert!(summary.contains("- Scorer: `scorer-model` (1 disagreement(s))")); + } + + #[test] + fn plain_summary_uses_plain_metadata_and_hides_raw_scorer_errors() { + 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, + block_on_kinds: vec![], + }, + model_used: "review-model".into(), + scorer_model: None, + scorer_error: Some("[click me](https://attacker.invalid)".into()), + scorer_disagreements: None, + usage: Default::default(), + duration_ms: 0, + base_sha: None, + head_sha: None, + since_sha: None, + }; + let summary = check_summary(&env, false, Default::default()); + + assert!(summary.contains("\nReview metadata:\n")); + assert!(!summary.contains("
")); + assert!(summary.contains("- Scorer status: unavailable")); + assert!(!summary.contains("attacker.invalid")); + } + #[test] fn plain_comment_has_statusline_without_html() { let body = finding_comment_body(&finding(), false); @@ -394,8 +554,8 @@ mod tests { head_sha: None, since_sha: None, }; - assert!(check_summary(&env, true).contains("status/pass.svg")); - assert!(!check_summary(&env, false).contains(" Result { if kept.len() > cfg.max_findings { suppressed.extend(kept.split_off(cfg.max_findings)); } - let gate_after = kept.iter().any(|f| cfg.gate_fail_on.fails(f.severity)); + let block_on_kinds: Vec = cfg + .block_on_kinds + .iter() + .map(|kind| kind.as_str().to_string()) + .collect(); + let gate_after = kept.iter().any(|finding| { + if finding.path == crate::envelope::PROVIDER_PATH { + // Provider-only pre-review failures encode their onError outcome + // only in gate.failing. Envelope v1 does not carry enough detail + // to reconstruct that decision from failOn, so preserve it rather + // than reporting a fabricated candidate-policy result. + env.gate.failing + } else { + crate::envelope::finding_blocks_gate( + finding, + cfg.gate_fail_on.as_str(), + &block_on_kinds, + false, + ) + } + }); Ok(PlanRow { name: path .file_name() @@ -209,4 +229,24 @@ mod tests { assert_eq!(rows[0].newly_suppressed.len(), 2); let _ = Counts::default(); } + + #[test] + fn replay_uses_human_escalation_confidence_floor() { + let dir = tempfile::tempdir().unwrap(); + let mut weak = f("a.rs", Severity::Error, 0.05); + weak.kind = Kind::HumanEscalation; + let env = envelope_with(vec![weak], true); + std::fs::write( + dir.path().join("r1.json"), + serde_json::to_string(&env).unwrap(), + ) + .unwrap(); + + let mut cfg = Config::default(); + cfg.min_confidence = 0.0; + let rows = run(dir.path(), &cfg).unwrap(); + + assert_eq!(rows[0].findings_after, 1); + assert!(!rows[0].gate_after); + } } diff --git a/src/review.rs b/src/review.rs index e5b05f8..5df66d8 100644 --- a/src/review.rs +++ b/src/review.rs @@ -10,8 +10,7 @@ use crate::diff::{self, DiffIndex}; use crate::envelope::{Envelope, Finding, Gate, Kind, Usage, fail_closed_finding}; use crate::filter; use crate::forge::{ - CheckState, Forge, PrMeta, azure::Azure, bitbucket::Bitbucket, check_summary, - clean_review_message, github::GitHub, gitlab::GitLab, + CheckState, Forge, PrMeta, azure::Azure, bitbucket::Bitbucket, github::GitHub, gitlab::GitLab, }; use crate::llm::{FindingScore, LlmClient}; use crate::local::{self, LocalSource}; @@ -790,10 +789,19 @@ async fn review_diff(cfg: &Config, args: &ReviewArgs, input: ReviewInput<'_>) -> // output (OPERATIONAL_PATH) never bypasses the gate: a malicious diff can // induce it via prompt injection. let advisory_on_error = cfg.gate_on_error == OnError::Advisory; + let gate_fail_on = cfg.gate_fail_on.as_str(); + let gate_block_on_kinds: Vec = cfg + .block_on_kinds + .iter() + .map(|kind| kind.as_str().to_string()) + .collect(); let gate_failing = findings.iter().any(|f| { - (cfg.gate_fail_on.fails(f.severity) - && !(advisory_on_error && f.path == crate::envelope::PROVIDER_PATH)) - || cfg.block_on_kinds.contains(&f.kind) + crate::envelope::finding_blocks_gate( + f, + gate_fail_on, + &gate_block_on_kinds, + advisory_on_error && f.path == crate::envelope::PROVIDER_PATH, + ) }); let silent = findings.is_empty(); let mut counts = Envelope::counts_of(&findings, suppressed); @@ -814,11 +822,7 @@ async fn review_diff(cfg: &Config, args: &ReviewArgs, input: ReviewInput<'_>) -> gate: Gate { fail_on: cfg.gate_fail_on.as_str().to_string(), failing: gate_failing, - block_on_kinds: cfg - .block_on_kinds - .iter() - .map(|k| k.as_str().to_string()) - .collect(), + block_on_kinds: gate_block_on_kinds, }, model_used, scorer_model, @@ -864,17 +868,7 @@ async fn finish( || matches!(cfg.on_clean, crate::config::OnClean::Comment)) && !duplicate_of_baseline; if should_comment { - let rich = forge.rich_markdown(); - let summary = if envelope.silent { - let icon = if rich { - format!("{} ", crate::forge::icon_md("pass")) - } else { - String::new() - }; - format!("{icon}{}", clean_review_message(&envelope)) - } else { - check_summary(&envelope, rich) - }; + let summary = forge.review_summary(&envelope); let head = envelope.head_sha.clone().unwrap_or_default(); // A posting failure here (rate limit, transient 5xx, network blip) // must not discard a review that already computed and persisted its diff --git a/tests/e2e.rs b/tests/e2e.rs index 5fc54e2..3639f69 100644 --- a/tests/e2e.rs +++ b/tests/e2e.rs @@ -109,6 +109,7 @@ fn postil() -> Command { .env_remove("POSTIL_API_KEY") .env_remove("POSTIL_API_BASE") .env_remove("POSTIL_DETAILS_URL") + .env_remove("GITHUB_SERVER_URL") .env_remove("POSTIL_ENABLE_BITBUCKET_INCREMENTAL") .env("MODEL_API_KEY", "test-key"); cmd @@ -195,6 +196,59 @@ async fn local_review_reports_grounded_finding_and_gates() { assert_eq!(request["messages"].as_array().unwrap().len(), 2); } +#[tokio::test] +async fn weak_human_escalation_remains_visible_without_blocking_gate() { + let server = MockServer::start().await; + mock_review( + &server, + json!([finding_at_with_kind(41, "error", "humanEscalation", 0.05)]), + ) + .await; + + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join(".postil.yaml"), "minConfidence: 0\n").unwrap(); + let diff = write_diff(dir.path()); + let out = postil() + .current_dir(dir.path()) + .env("POSTIL_API_BASE", server.uri()) + .args(["review", "--diff-file"]) + .arg(&diff) + .args(["--output", "json"]) + .assert() + .code(0); + let env: Value = serde_json::from_slice(&out.get_output().stdout).unwrap(); + + assert_eq!(env["findings"][0]["kind"], "humanEscalation"); + assert_eq!(env["findings"][0]["confidence"], 0.05); + assert_eq!(env["gate"]["failing"], false); +} + +#[tokio::test] +async fn human_escalation_at_floor_blocks_even_at_warn_severity() { + let server = MockServer::start().await; + mock_review( + &server, + json!([finding_at_with_kind(41, "warn", "humanEscalation", 0.30)]), + ) + .await; + + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join(".postil.yaml"), "minConfidence: 0\n").unwrap(); + let diff = write_diff(dir.path()); + let out = postil() + .current_dir(dir.path()) + .env("POSTIL_API_BASE", server.uri()) + .args(["review", "--diff-file"]) + .arg(&diff) + .args(["--output", "json"]) + .assert() + .code(1); + let env: Value = serde_json::from_slice(&out.get_output().stdout).unwrap(); + + assert_eq!(env["findings"][0]["severity"], "warn"); + assert_eq!(env["gate"]["failing"], true); +} + #[tokio::test] async fn scorer_lowers_confidence_and_stores_both_values() { let server = MockServer::start().await; @@ -1675,7 +1729,7 @@ async fn github_flow_posts_review_and_completes_both_checks() { assert_eq!(gate_patch["output"]["title"], "Merge gate failed"); assert_eq!( gate_patch["output"]["summary"], - "Merge gate failed: 1 finding at or above the blocking threshold (error).\n\n- `src/auth.rs:41` Unsanitized input reaches query\n" + "Merge gate failed: 1 finding blocks under the configured policy (failOn: error).\n\n- `src/auth.rs:41` Unsanitized input reaches query\n" ); // Inline review posted with the finding at the cited line. let review = reqs @@ -1685,6 +1739,20 @@ async fn github_flow_posts_review_and_completes_both_checks() { let body: Value = review.body_json().unwrap(); assert_eq!(body["comments"][0]["path"], "src/auth.rs"); assert_eq!(body["comments"][0]["line"], 41); + let summary = body["body"].as_str().unwrap(); + assert!(summary.starts_with("Gate: failed\n\n")); + assert!(summary.contains("**Unsanitized input reaches query** · severity: `error`")); + assert!(!summary.contains("`src/auth.rs:41`")); + assert!(summary.contains("
Review metadata")); + assert!( + summary.contains("- Commit: [`headsha`](https://github.com/acme/api/commit/headsha111)") + ); + assert!( + summary.contains( + "- Dashboard run: [View in Postil](https://postil.dev/orgs/acme/runs/review-7)" + ) + ); + assert!(summary.contains("- Tokens: 300 prompt, 150 completion")); } // An LLM response with a caller-provided summary and findings (used for @@ -1802,7 +1870,7 @@ async fn content_policy_pr_body_finding_survives_grounding() { body["body"] .as_str() .unwrap() - .contains(".postil/pr-description") + .contains("AI-authorship residue in PR description") ); } @@ -1910,8 +1978,45 @@ async fn github_clean_pr_stays_silent_but_completes_checks() { assert_eq!(gate_patch["output"]["title"], "Merge gate passed"); assert_eq!( gate_patch["output"]["summary"], - "Merge gate passed: no findings at or above the blocking threshold (error).\n" + "Merge gate passed: no findings block under the configured policy (failOn: error).\n" ); + + // The explicit onClean mode uses the same unified summary as finding-bearing + // reviews instead of falling back to the former one-line clean message. + Mock::given(method("POST")) + .and(path("/repos/acme/api/pulls/7/reviews")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({}))) + .mount(&server) + .await; + std::fs::write( + dir.path().join(".postil.yaml"), + "review:\n onClean: comment\n", + ) + .unwrap(); + postil() + .current_dir(dir.path()) + .env("POSTIL_API_BASE", server.uri()) + .env("GITHUB_API_URL", server.uri()) + .env("GITHUB_TOKEN", "gh-test-token") + .env( + "POSTIL_DETAILS_URL", + "https://postil.dev/orgs/acme/runs/clean-7", + ) + .args(["review", "--repo", "acme/api", "--pr", "7"]) + .assert() + .code(0); + let reqs = server.received_requests().await.unwrap(); + let clean_review = reqs + .iter() + .rev() + .find(|request| request.url.path().ends_with("/reviews")) + .expect("onClean review posted"); + let clean_body: Value = clean_review.body_json().unwrap(); + let clean_summary = clean_body["body"].as_str().unwrap(); + assert!(clean_summary.starts_with("Gate: passed\n\n")); + assert!(clean_summary.contains("Postil reviewed this change and found nothing")); + assert!(clean_summary.contains("
Review metadata")); + assert!(clean_summary.contains("[View in Postil](https://postil.dev/orgs/acme/runs/clean-7)")); } #[tokio::test]