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
58 changes: 44 additions & 14 deletions src/forge/github.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -85,6 +84,20 @@ fn valid_details_url(value: Option<String>) -> Option<String> {
})
}

fn github_web_base(server_url: Option<String>, 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"
Expand All @@ -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();
Expand Down Expand Up @@ -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]
Expand All @@ -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 {
Expand Down Expand Up @@ -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(),
Expand All @@ -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"));
}
}
47 changes: 41 additions & 6 deletions src/plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,10 @@ fn replay(path: &Path, env: &Envelope, cfg: &Config) -> Result<PlanRow> {
.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,
Expand Down Expand Up @@ -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);
}
}
13 changes: 7 additions & 6 deletions src/review.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading