From 6e992a7a2f28df20a267a379300d4633c2734e87 Mon Sep 17 00:00:00 2001 From: Postil Maintainer Date: Fri, 10 Jul 2026 14:41:10 +0000 Subject: [PATCH] Plain-language gate check output and optional details_url The postil/gate check-run now reads "Merge gate passed/failed: N finding(s) at or above the blocking threshold ()" instead of the failOn jargon, with its own title distinct from the review check. POSTIL_DETAILS_URL, when set to a valid http(s) URL, is passed as details_url on both check-runs so GitHub's "View more details" link can target a specific hosted run page. --- README.md | 3 +- src/forge/github.rs | 128 +++++++++++++++++++++++++++++++++----------- tests/e2e.rs | 45 ++++++++++++++++ 3 files changed, 145 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index 298c70a..ca23954 100644 --- a/README.md +++ b/README.md @@ -150,7 +150,8 @@ single-user local setup where the checked-out repo is trusted, set `POSTIL_ALLOW_CONFIG_API_BASE=1` to honor the config value. Environment: `POSTIL_API_KEY`, `OPENROUTER_API_KEY`, `MODEL_API_KEY`, or -`LLM_API_KEY`, `POSTIL_API_BASE`, +`LLM_API_KEY`, `POSTIL_API_BASE`, `POSTIL_DETAILS_URL` (optional HTTP(S) target +for GitHub check-run details links), `REVIEW_MODEL`, `REVIEW_MODEL_CASCADE`, `GITHUB_TOKEN`/`GITHUB_API_URL`, `GITLAB_TOKEN`/`GITLAB_API_URL`, `BITBUCKET_TOKEN`/`BITBUCKET_USER`/`BITBUCKET_API_URL`, `AZURE_DEVOPS_TOKEN`/`AZURE_DEVOPS_API_URL`. diff --git a/src/forge/github.rs b/src/forge/github.rs index aa0cd2e..b409620 100644 --- a/src/forge/github.rs +++ b/src/forge/github.rs @@ -10,6 +10,7 @@ use crate::envelope::{Envelope, Finding, Severity}; pub struct GitHub { http: reqwest::Client, api_base: String, + details_url: Option, token: String, owner: String, repo: String, @@ -25,11 +26,13 @@ impl GitHub { .map_err(|_| anyhow!("GITHUB_TOKEN is required for --forge 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()); Ok(GitHub { http: reqwest::Client::builder() .timeout(std::time::Duration::from_secs(60)) .build()?, api_base: api_base.trim_end_matches('/').to_string(), + details_url, token, owner: owner.to_string(), repo: repo.to_string(), @@ -49,6 +52,12 @@ impl GitHub { .header("X-GitHub-Api-Version", "2022-11-28") } + fn add_details_url(&self, body: &mut serde_json::Value) { + if let Some(details_url) = &self.details_url { + body["details_url"] = json!(details_url); + } + } + async fn check_ok(resp: reqwest::Response, what: &str) -> Result { let status = resp.status(); if status.is_success() { @@ -60,6 +69,53 @@ impl GitHub { } } +fn valid_details_url(value: Option) -> Option { + value.filter(|value| { + reqwest::Url::parse(value) + .map(|url| matches!(url.scheme(), "http" | "https") && url.has_host()) + .unwrap_or(false) + }) +} + +fn gate_title(envelope: &Envelope) -> &'static str { + if envelope.gate.failing { + "Merge gate failed" + } else { + "Merge gate passed" + } +} + +fn gate_summary(envelope: &Envelope) -> String { + if !envelope.gate.failing { + return format!( + "Merge gate passed: no findings at or above the blocking threshold ({}).\n", + envelope.gate.fail_on + ); + } + + let failing: Vec<_> = envelope + .findings + .iter() + .filter(|f| { + crate::config::GateLevel::parse(&envelope.gate.fail_on) + .map(|g| g.fails(f.severity)) + .unwrap_or(false) + }) + .map(|f| format!("- `{}:{}` {}", f.path, f.line, f.title)) + .collect(); + let noun = if failing.len() == 1 { + "finding" + } else { + "findings" + }; + format!( + "Merge gate failed: {} {noun} at or above the blocking threshold ({}).\n\n{}\n", + failing.len(), + envelope.gate.fail_on, + failing.join("\n") + ) +} + #[derive(Deserialize)] struct PrResponse { title: String, @@ -178,13 +234,15 @@ impl Forge for GitHub { async fn start_checks(&self, head_sha: &str) -> Result<(String, String)> { let mut ids = Vec::with_capacity(2); for name in ["postil/review", "postil/gate"] { + let mut body = json!({ + "name": name, + "head_sha": head_sha, + "status": "in_progress", + }); + self.add_details_url(&mut body); let resp = self .request(reqwest::Method::POST, self.url("/check-runs")) - .json(&json!({ - "name": name, - "head_sha": head_sha, - "status": "in_progress", - })) + .json(&body) .send() .await .with_context(|| format!("creating check-run {name}"))?; @@ -238,47 +296,36 @@ impl Forge for GitHub { (gate_id, gate, "postil/gate", false), ] { let gate_note = if name == "postil/gate" { - let failing: Vec<_> = envelope - .findings - .iter() - .filter(|f| { - crate::config::GateLevel::parse(&envelope.gate.fail_on) - .map(|g| g.fails(f.severity)) - .unwrap_or(false) - }) - .map(|f| format!("- `{}:{}` {}", f.path, f.line, f.title)) - .collect(); - if envelope.gate.failing { - format!( - "Gate failing at `{}` on:\n{}\n", - envelope.gate.fail_on, - failing.join("\n") - ) - } else { - format!("Gate (`failOn: {}`) passing.\n", envelope.gate.fail_on) - } + gate_summary(envelope) } else { check_summary(envelope, true) }; + let title = if name == "postil/gate" { + gate_title(envelope).to_string() + } else { + check_title(envelope) + }; let mut output = json!({ // GitHub rejects title >255 and summary >65535 with HTTP 422, // which would abort posting both checks. Cap both defensively. - "title": super::cap_check_title(&check_title(envelope)), + "title": super::cap_check_title(&title), "summary": super::cap_check_summary(&gate_note), }); if with_annotations && !annotations.is_empty() { output["annotations"] = json!(annotations); } + let mut body = json!({ + "status": "completed", + "conclusion": conclusion(state), + "output": output, + }); + self.add_details_url(&mut body); let resp = self .request( reqwest::Method::PATCH, self.url(&format!("/check-runs/{id}")), ) - .json(&json!({ - "status": "completed", - "conclusion": conclusion(state), - "output": output, - })) + .json(&body) .send() .await .with_context(|| format!("completing check-run {name}"))?; @@ -316,3 +363,24 @@ impl Forge for GitHub { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::valid_details_url; + + #[test] + fn details_url_accepts_only_http_and_https_urls() { + assert_eq!( + valid_details_url(Some("https://postil.dev/orgs/acme/runs/review-1".into())), + Some("https://postil.dev/orgs/acme/runs/review-1".into()) + ); + assert_eq!( + valid_details_url(Some("http://localhost:3000/runs/review-1".into())), + Some("http://localhost:3000/runs/review-1".into()) + ); + assert_eq!(valid_details_url(Some("ftp://postil.dev/run".into())), None); + assert_eq!(valid_details_url(Some("https://".into())), None); + assert_eq!(valid_details_url(Some("not a URL".into())), None); + assert_eq!(valid_details_url(None), None); + } +} diff --git a/tests/e2e.rs b/tests/e2e.rs index da427a5..3d5503c 100644 --- a/tests/e2e.rs +++ b/tests/e2e.rs @@ -72,6 +72,7 @@ fn postil() -> Command { .env_remove("OPENROUTER_API_KEY") .env_remove("POSTIL_API_KEY") .env_remove("POSTIL_API_BASE") + .env_remove("POSTIL_DETAILS_URL") .env("MODEL_API_KEY", "test-key"); cmd } @@ -830,6 +831,10 @@ async fn github_flow_posts_review_and_completes_both_checks() { .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/review-7", + ) .args(["review", "--repo", "acme/api", "--pr", "7", "--output-json"]) .assert() .code(1); @@ -847,6 +852,10 @@ async fn github_flow_posts_review_and_completes_both_checks() { }) .collect(); assert_eq!(creates.len(), 2); + assert!(creates.iter().all(|request| { + request.body_json::().unwrap()["details_url"] + == "https://postil.dev/orgs/acme/runs/review-7" + })); let names: Vec = creates .iter() .map(|r| { @@ -865,12 +874,26 @@ async fn github_flow_posts_review_and_completes_both_checks() { .map(|r| r.body_json().unwrap()) .collect(); assert_eq!(patches.len(), 2); + assert!( + patches + .iter() + .all(|patch| { patch["details_url"] == "https://postil.dev/orgs/acme/runs/review-7" }) + ); let conclusions: Vec<&str> = patches .iter() .map(|p| p["conclusion"].as_str().unwrap()) .collect(); assert!(conclusions.contains(&"success")); assert!(conclusions.contains(&"failure")); + let gate_patch = patches + .iter() + .find(|patch| patch["conclusion"] == "failure") + .unwrap(); + 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" + ); // Inline review posted with the finding at the cited line. let review = reqs .iter() @@ -1094,6 +1117,28 @@ async fn github_clean_pr_stays_silent_but_completes_checks() { }) .collect(); assert_eq!(conclusions, vec!["success", "success"]); + let check_requests: Vec = reqs + .iter() + .filter(|request| { + request.url.path().starts_with("/repos/acme/api/check-runs") + && matches!( + request.method, + wiremock::http::Method::POST | wiremock::http::Method::PATCH + ) + }) + .map(|request| request.body_json().unwrap()) + .collect(); + assert!( + check_requests + .iter() + .all(|request| request.get("details_url").is_none()) + ); + let gate_patch = &check_requests[3]; + 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" + ); } #[tokio::test]