From edfbcfd24b1cb63060944eab24f986b0f47b61d3 Mon Sep 17 00:00:00 2001 From: Postil Maintainer Date: Sun, 12 Jul 2026 03:06:59 +0000 Subject: [PATCH] Align operational gate replay and GitHub links --- src/forge/github.rs | 58 ++++++++++++++++++++++++++++++++++----------- src/plan.rs | 47 +++++++++++++++++++++++++++++++----- src/review.rs | 13 +++++----- 3 files changed, 92 insertions(+), 26 deletions(-) diff --git a/src/forge/github.rs b/src/forge/github.rs index c1ffab2..6825b0f 100644 --- a/src/forge/github.rs +++ b/src/forge/github.rs @@ -32,8 +32,7 @@ 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()); + let web_base = github_web_base(std::env::var("GITHUB_SERVER_URL").ok(), &api_base); Ok(GitHub { http: reqwest::Client::builder() .timeout(std::time::Duration::from_secs(60)) @@ -85,6 +84,20 @@ fn valid_details_url(value: Option) -> Option { }) } +fn github_web_base(server_url: Option, api_base: &str) -> String { + if let Some(server_url) = valid_details_url(server_url) { + return server_url.trim_end_matches('/').to_string(); + } + let normalized_api_base = api_base.trim_end_matches('/'); + if normalized_api_base != "https://api.github.com" + && let Some(web_url) = normalized_api_base.strip_suffix("/api/v3") + && valid_details_url(Some(web_url.to_string())).is_some() + { + return web_url.trim_end_matches('/').to_string(); + } + "https://github.com".to_string() +} + fn gate_title(envelope: &Envelope) -> &'static str { if envelope.gate.failing { "Merge gate failed" @@ -105,12 +118,14 @@ fn gate_summary(envelope: &Envelope) -> String { .findings .iter() .filter(|f| { - crate::envelope::finding_blocks_gate( - f, - &envelope.gate.fail_on, - &envelope.gate.block_on_kinds, - false, - ) + f.path == crate::envelope::OPERATIONAL_PATH + || (f.path == crate::envelope::PROVIDER_PATH && envelope.gate.failing) + || 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(); @@ -405,7 +420,7 @@ impl Forge for GitHub { #[cfg(test)] mod tests { - use super::{gate_summary, valid_details_url}; + use super::{gate_summary, github_web_base, valid_details_url}; use crate::envelope::{Envelope, Finding, Gate, Kind, Severity, Usage}; #[test] @@ -424,6 +439,22 @@ mod tests { assert_eq!(valid_details_url(None), None); } + #[test] + fn web_base_uses_documented_github_enterprise_api_url() { + assert_eq!( + github_web_base(None, "https://github.example.com/api/v3"), + "https://github.example.com" + ); + assert_eq!( + github_web_base(None, "https://github.example.com/api/v3/"), + "https://github.example.com" + ); + assert_eq!( + github_web_base(Some("https://code.example.com/".into()), "ignored"), + "https://code.example.com" + ); + } + #[test] fn gate_summary_counts_qualified_kind_blocks_from_stored_envelope() { let finding = Finding { @@ -472,7 +503,7 @@ mod tests { } #[test] - fn gate_summary_reports_operational_failure_without_inventing_blockers() { + fn gate_summary_reports_provider_failure_from_serialized_gate_outcome() { let mut env = Envelope { version: 1, summary: String::new(), @@ -499,9 +530,8 @@ mod tests { 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" - ); + let summary = gate_summary(&env); + assert!(summary.contains("1 finding blocks under the configured policy")); + assert!(summary.contains(".postil/provider:1")); } } diff --git a/src/plan.rs b/src/plan.rs index 7bec4c3..9c5566e 100644 --- a/src/plan.rs +++ b/src/plan.rs @@ -75,12 +75,10 @@ fn replay(path: &Path, env: &Envelope, cfg: &Config) -> Result { .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 + if finding.path == crate::envelope::OPERATIONAL_PATH { + true + } else if finding.path == crate::envelope::PROVIDER_PATH { + cfg.gate_on_error == crate::config::OnError::Block } else { crate::envelope::finding_blocks_gate( finding, @@ -249,4 +247,41 @@ mod tests { assert_eq!(rows[0].findings_after, 1); assert!(!rows[0].gate_after); } + + #[test] + fn replay_applies_candidate_error_policy() { + let dir = tempfile::tempdir().unwrap(); + let mut provider = f(crate::envelope::PROVIDER_PATH, Severity::Error, 1.0); + provider.kind = Kind::Uncertainty; + let env = envelope_with(vec![provider], true); + std::fs::write( + dir.path().join("provider.json"), + serde_json::to_string(&env).unwrap(), + ) + .unwrap(); + + let mut cfg = Config::default(); + cfg.gate_fail_on = crate::config::GateLevel::Never; + cfg.gate_on_error = crate::config::OnError::Advisory; + assert!(!run(dir.path(), &cfg).unwrap()[0].gate_after); + + cfg.gate_on_error = crate::config::OnError::Block; + assert!(run(dir.path(), &cfg).unwrap()[0].gate_after); + } + + #[test] + fn replay_always_blocks_unusable_model_output() { + let dir = tempfile::tempdir().unwrap(); + let env = envelope_with(vec![crate::envelope::fail_closed_finding("invalid")], true); + std::fs::write( + dir.path().join("invalid.json"), + serde_json::to_string(&env).unwrap(), + ) + .unwrap(); + + let mut cfg = Config::default(); + cfg.gate_fail_on = crate::config::GateLevel::Never; + cfg.gate_on_error = crate::config::OnError::Advisory; + assert!(run(dir.path(), &cfg).unwrap()[0].gate_after); + } } diff --git a/src/review.rs b/src/review.rs index e930faa..46812f1 100644 --- a/src/review.rs +++ b/src/review.rs @@ -798,12 +798,13 @@ async fn review_diff(cfg: &Config, args: &ReviewArgs, input: ReviewInput<'_>) -> .map(|kind| kind.as_str().to_string()) .collect(); let gate_failing = findings.iter().any(|f| { - crate::envelope::finding_blocks_gate( - f, - gate_fail_on, - &gate_block_on_kinds, - advisory_on_error && f.path == crate::envelope::PROVIDER_PATH, - ) + if f.path == crate::envelope::OPERATIONAL_PATH { + true + } else if f.path == crate::envelope::PROVIDER_PATH { + !advisory_on_error + } else { + crate::envelope::finding_blocks_gate(f, gate_fail_on, &gate_block_on_kinds, false) + } }); let silent = findings.is_empty(); let mut counts = Envelope::counts_of(&findings, suppressed);