diff --git a/README.md b/README.md index 735f195..6595621 100644 --- a/README.md +++ b/README.md @@ -113,7 +113,13 @@ Planned lanes: | Trigger | Status | |---|---| | Push / commit-range review | `push` events are parsed and typed with fixtures today; execution needs a PR-less task kind, which ships with headless contract v3 | -| Advisory / blocking publication gates | Issue #11 | + +Review findings pass **deterministic publication gates** before any surface +sees them: out-of-scope files (never consulted by the session), findings below +the repo's `min_severity` policy, and duplicates are withheld — with the +withheld counts stated in the digest. The `[review] publish` policy routes the +gated digest to the Check Run (default), additionally to the status comment +(`advisory_comment`), or as a blocking PR review verdict (`request_changes`). ## Maintainer commands diff --git a/config/example.toml b/config/example.toml index eaf4b90..6040409 100644 --- a/config/example.toml +++ b/config/example.toml @@ -63,6 +63,11 @@ trigger_labels = ["coven:fix", "coven:review"] # Labels that trigger this famil # pull_request = true # Review on opened/synchronize/reopened/ready_for_review # include_drafts = false # Draft PRs are skipped unless explicitly labeled # audit_instruction = "Focus on correctness and security." +# min_severity = "medium" # Withhold findings below this severity +# # (info | low | medium | high | critical) +# publish = "check_run" # Where gated findings go: check_run (default), +# # advisory_comment (also on the status comment), +# # request_changes (blocking PR review verdict) # [review.repos."OpenCoven/coven-code"] # pull_request = false # Per-repo override diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index 0ac81d4..22faa40 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -143,6 +143,13 @@ pub struct ReviewConfig { pub include_drafts: bool, /// Adapter-authored instruction forwarded as the brief's audit_instruction. pub audit_instruction: Option, + /// Minimum finding severity that publishes (`info`, `low`, `medium`, + /// `high`, `critical`). Absent = every severity publishes (issue #11). + pub min_severity: Option, + /// Where gated findings publish (issue #11): `check_run` (default), + /// `advisory_comment` (also on the status comment), or `request_changes` + /// (submit a PR review that requests changes when findings exist). + pub publish: Option, /// Per-repo overrides keyed "owner/name". #[serde(default)] pub repos: std::collections::HashMap, @@ -155,6 +162,8 @@ pub struct RepoReviewOverride { pub include_drafts: Option, pub familiar: Option, pub audit_instruction: Option, + pub min_severity: Option, + pub publish: Option, } impl ReviewConfig { @@ -185,6 +194,20 @@ impl ReviewConfig { .and_then(|o| o.audit_instruction.clone()) .or_else(|| self.audit_instruction.clone()) } + + /// Minimum publishable severity for `repo` (raw string; issue #11). + pub fn min_severity_for(&self, repo: &str) -> Option { + self.overrides(repo) + .and_then(|o| o.min_severity.clone()) + .or_else(|| self.min_severity.clone()) + } + + /// Findings publication mode for `repo` (raw string; issue #11). + pub fn publish_for(&self, repo: &str) -> Option { + self.overrides(repo) + .and_then(|o| o.publish.clone()) + .or_else(|| self.publish.clone()) + } } #[derive(Debug, Clone, Deserialize, Serialize)] @@ -476,6 +499,40 @@ impl Config { ); } } + // Publication policy values are closed enums (issue #11): a typo would + // silently change what publishes, so doctor rejects unknown values. + let severities = ["info", "low", "medium", "high", "critical"]; + let publish_modes = ["check_run", "advisory_comment", "request_changes"]; + let mut check_policy = |scope: &str, min_severity: Option<&str>, publish: Option<&str>| { + if let Some(value) = min_severity { + if !severities.contains(&value.to_ascii_lowercase().as_str()) { + out.push(Diagnostic::error( + "review.min_severity", + format!("{scope} has unknown min_severity '{value}' — use one of: {}.", severities.join(", ")), + )); + } + } + if let Some(value) = publish { + if !publish_modes.contains(&value.to_ascii_lowercase().as_str()) { + out.push(Diagnostic::error( + "review.publish", + format!("{scope} has unknown publish mode '{value}' — use one of: {}.", publish_modes.join(", ")), + )); + } + } + }; + check_policy( + "the [review] section", + self.review.min_severity.as_deref(), + self.review.publish.as_deref(), + ); + for (repo, o) in &self.review.repos { + check_policy( + &format!("the review override for '{repo}'"), + o.min_severity.as_deref(), + o.publish.as_deref(), + ); + } // ── Memory governance (issue #6) ──────────────────────────────── // Memory is off by default; when an operator enables it anywhere, @@ -585,6 +642,12 @@ fn next_step_for(field: &str, _message: &str) -> &'static str { "api.tenants[].token" => { "Generate long random tokens (e.g. openssl rand -hex 32) and keep each scope's token unique." } + "review.min_severity" => { + "Set review.min_severity to one of: info, low, medium, high, critical." + } + "review.publish" => { + "Set review.publish to one of: check_run, advisory_comment, request_changes." + } _ => "Update this config field, then rerun coven-github doctor.", } } @@ -729,6 +792,8 @@ mod tests { pull_request: true, include_drafts: false, audit_instruction: Some("global".to_string()), + min_severity: Some("medium".to_string()), + publish: None, repos: std::collections::HashMap::from([( "OpenCoven/quiet".to_string(), RepoReviewOverride { @@ -736,10 +801,27 @@ mod tests { include_drafts: Some(true), familiar: Some("nova".to_string()), audit_instruction: None, + min_severity: None, + publish: Some("advisory_comment".to_string()), }, )]), }; + // Publication policy inherits globally and overrides per repo (#11). + assert_eq!( + policy.min_severity_for("OpenCoven/loud").as_deref(), + Some("medium") + ); + assert_eq!( + policy.min_severity_for("OpenCoven/quiet").as_deref(), + Some("medium") + ); + assert_eq!(policy.publish_for("OpenCoven/loud"), None); + assert_eq!( + policy.publish_for("OpenCoven/quiet").as_deref(), + Some("advisory_comment") + ); + assert!(policy.pull_request_enabled("OpenCoven/loud")); assert!(!policy.pull_request_enabled("OpenCoven/quiet")); assert!(!policy.drafts_included("OpenCoven/loud")); diff --git a/crates/github/src/pr.rs b/crates/github/src/pr.rs index 75a2464..12b8eb5 100644 --- a/crates/github/src/pr.rs +++ b/crates/github/src/pr.rs @@ -265,6 +265,60 @@ fn update_comment_request( } } +/// Verdict of an adapter-submitted PR review (issue #11). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ReviewVerdict { + /// Findings exist: block with requested changes. + RequestChanges, + /// Nothing actionable: a non-blocking comment review. + Comment, +} + +impl ReviewVerdict { + fn as_str(self) -> &'static str { + match self { + ReviewVerdict::RequestChanges => "REQUEST_CHANGES", + ReviewVerdict::Comment => "COMMENT", + } + } +} + +/// Submits a pull request review with the given verdict and body +/// (`request_changes` publication mode, issue #11). +pub async fn submit_review_with_base_url( + api_base_url: &str, + installation_token: &str, + repo_owner: &str, + repo_name: &str, + pr_number: u64, + verdict: ReviewVerdict, + body: &str, +) -> Result<()> { + let client = client()?; + send_json( + &client, + api_base_url, + installation_token, + submit_review_request(repo_owner, repo_name, pr_number, verdict, body), + ) + .await?; + Ok(()) +} + +fn submit_review_request( + repo_owner: &str, + repo_name: &str, + pr_number: u64, + verdict: ReviewVerdict, + body: &str, +) -> GitHubRequest { + GitHubRequest { + method: "POST", + path: format!("/repos/{repo_owner}/{repo_name}/pulls/{pr_number}/reviews"), + body: serde_json::json!({ "event": verdict.as_str(), "body": body }), + } +} + #[cfg(test)] mod comment_tests { use super::*; @@ -285,6 +339,16 @@ mod comment_tests { assert_eq!(request.body["body"], json!("new body")); } + #[test] + fn submit_review_request_posts_the_verdict() { + let request = + submit_review_request("octo", "repo", 88, ReviewVerdict::RequestChanges, "digest"); + assert_eq!(request.method, "POST"); + assert_eq!(request.path, "/repos/octo/repo/pulls/88/reviews"); + assert_eq!(request.body["event"], json!("REQUEST_CHANGES")); + assert_eq!(request.body["body"], json!("digest")); + } + #[test] fn issue_comment_deserializes_id_body_and_author() { let comments: Vec = serde_json::from_value(json!([ diff --git a/crates/webhook/src/routes.rs b/crates/webhook/src/routes.rs index 7653716..a6434bc 100644 --- a/crates/webhook/src/routes.rs +++ b/crates/webhook/src/routes.rs @@ -1003,6 +1003,8 @@ mod review_lane_tests { pull_request: true, include_drafts: false, audit_instruction: None, + min_severity: None, + publish: None, repos: std::collections::HashMap::new(), } } @@ -1100,6 +1102,8 @@ mod review_lane_tests { include_drafts: None, familiar: None, audit_instruction: None, + min_severity: None, + publish: None, }, ); let state = app_state_with_review(review); diff --git a/crates/worker/src/findings.rs b/crates/worker/src/findings.rs new file mode 100644 index 0000000..da2f74a --- /dev/null +++ b/crates/worker/src/findings.rs @@ -0,0 +1,315 @@ +//! Deterministic publication gates for review findings (issue #11). +//! +//! Agent judgment produces findings; the adapter decides what publishes. +//! Every finding passes a gate chain before reaching any GitHub surface: +//! +//! 1. **Scope** — the file must appear in the reviewed / supporting / +//! changed-file sets the session actually consulted. Findings about files +//! the review never touched are speculative and are withheld. +//! 2. **Severity threshold** — repo policy can set a minimum severity; +//! anything below is filtered (still counted, so the surface stays honest). +//! 3. **Duplicates** — identical `(file, line, title)` findings collapse. +//! +//! The rendered digest always states how many findings were withheld and +//! why, so a quiet report is distinguishable from a filtered one. +//! (A confidence axis is specified for headless contract v3; v2 findings +//! carry severity only.) + +use std::collections::HashSet; + +use coven_github_api::{ReviewFinding, ReviewResult, ReviewSeverity}; + +/// Result of running the gate chain over a review's findings. +pub struct GateOutcome { + /// Findings that may publish, highest severity first. + pub published: Vec, + pub dropped_out_of_scope: usize, + pub dropped_below_threshold: usize, + pub dropped_duplicates: usize, +} + +impl GateOutcome { + pub fn dropped_total(&self) -> usize { + self.dropped_out_of_scope + self.dropped_below_threshold + self.dropped_duplicates + } +} + +/// Applies the gate chain. `changed_files` is the PR's changed set from live +/// GitHub state; `min_severity` comes from repo policy (`None` = publish all +/// severities). +pub fn gate( + review: &ReviewResult, + changed_files: &[String], + min_severity: Option, +) -> GateOutcome { + let in_scope: HashSet<&str> = review + .reviewed_files + .iter() + .chain(review.supporting_files.iter()) + .chain(changed_files.iter()) + .map(String::as_str) + .collect(); + let threshold = min_severity.map(rank); + + let mut outcome = GateOutcome { + published: Vec::new(), + dropped_out_of_scope: 0, + dropped_below_threshold: 0, + dropped_duplicates: 0, + }; + let mut seen: HashSet<(String, Option, String)> = HashSet::new(); + + for finding in &review.findings { + if !in_scope.contains(finding.file.as_str()) { + outcome.dropped_out_of_scope += 1; + continue; + } + if let Some(threshold) = threshold { + if rank(finding.severity.clone()) < threshold { + outcome.dropped_below_threshold += 1; + continue; + } + } + if !seen.insert(( + finding.file.clone(), + finding.line, + finding.title.clone(), + )) { + outcome.dropped_duplicates += 1; + continue; + } + outcome.published.push(finding.clone()); + } + + outcome + .published + .sort_by_key(|f| std::cmp::Reverse(rank(f.severity.clone()))); + outcome +} + +/// Renders the gated findings as a markdown digest for Check Run output and +/// (in advisory mode) the status comment. Bounded well under GitHub's 64 KiB +/// Check Run summary limit. +pub fn render(outcome: &GateOutcome) -> String { + const MAX_LEN: usize = 48_000; + + let mut out = String::new(); + if outcome.published.is_empty() { + out.push_str("**Findings:** none published."); + } else { + out.push_str(&format!( + "**Findings ({} published):**\n", + outcome.published.len() + )); + for finding in &outcome.published { + let location = match finding.line { + Some(line) => format!("`{}:{line}`", finding.file), + None => format!("`{}`", finding.file), + }; + let entry = match &finding.recommendation { + Some(rec) => format!( + "- **{}** {location} — {}\n {}\n _Recommendation: {}_\n", + severity_label(&finding.severity), + finding.title, + finding.body, + rec + ), + None => format!( + "- **{}** {location} — {}\n {}\n", + severity_label(&finding.severity), + finding.title, + finding.body + ), + }; + if out.len() + entry.len() > MAX_LEN { + out.push_str("- _…digest truncated._\n"); + break; + } + out.push_str(&entry); + } + } + if outcome.dropped_total() > 0 { + out.push_str(&format!( + "\n_{} finding(s) withheld by publication gates: {} out of scope, {} below the severity threshold, {} duplicate(s)._", + outcome.dropped_total(), + outcome.dropped_out_of_scope, + outcome.dropped_below_threshold, + outcome.dropped_duplicates, + )); + } + out +} + +/// Parses a policy severity string (`info` … `critical`). +pub fn parse_severity(value: &str) -> Option { + match value.to_ascii_lowercase().as_str() { + "info" => Some(ReviewSeverity::Info), + "low" => Some(ReviewSeverity::Low), + "medium" => Some(ReviewSeverity::Medium), + "high" => Some(ReviewSeverity::High), + "critical" => Some(ReviewSeverity::Critical), + _ => None, + } +} + +fn rank(severity: ReviewSeverity) -> u8 { + match severity { + ReviewSeverity::Info => 0, + ReviewSeverity::Low => 1, + ReviewSeverity::Medium => 2, + ReviewSeverity::High => 3, + ReviewSeverity::Critical => 4, + } +} + +fn severity_label(severity: &ReviewSeverity) -> &'static str { + match severity { + ReviewSeverity::Info => "INFO", + ReviewSeverity::Low => "LOW", + ReviewSeverity::Medium => "MEDIUM", + ReviewSeverity::High => "HIGH", + ReviewSeverity::Critical => "CRITICAL", + } +} + +#[cfg(test)] +mod tests { + use super::*; + use coven_github_api::{ReviewEvidenceStatus, ReviewMode}; + + fn finding(file: &str, line: Option, title: &str, severity: ReviewSeverity) -> ReviewFinding { + ReviewFinding { + severity, + file: file.to_string(), + line, + title: title.to_string(), + body: "body".to_string(), + recommendation: None, + } + } + + fn review(findings: Vec) -> ReviewResult { + ReviewResult { + mode: ReviewMode::PullRequest, + evidence_status: ReviewEvidenceStatus::Complete, + reviewed_files: vec!["src/lib.rs".to_string()], + supporting_files: vec!["src/util.rs".to_string()], + findings, + tests_run: vec![], + no_findings_reason: None, + limitations: vec![], + } + } + + #[test] + fn out_of_scope_findings_are_withheld() { + let outcome = gate( + &review(vec![ + finding("src/lib.rs", Some(1), "in scope", ReviewSeverity::Low), + finding("secrets/vault.rs", Some(1), "speculative", ReviewSeverity::Critical), + ]), + &["src/changed.rs".to_string()], + None, + ); + assert_eq!(outcome.published.len(), 1); + assert_eq!(outcome.published[0].file, "src/lib.rs"); + assert_eq!(outcome.dropped_out_of_scope, 1); + } + + #[test] + fn changed_and_supporting_files_count_as_scope() { + let outcome = gate( + &review(vec![ + finding("src/changed.rs", None, "on the diff", ReviewSeverity::Low), + finding("src/util.rs", None, "supporting", ReviewSeverity::Low), + ]), + &["src/changed.rs".to_string()], + None, + ); + assert_eq!(outcome.published.len(), 2); + assert_eq!(outcome.dropped_total(), 0); + } + + #[test] + fn severity_threshold_filters_and_counts() { + let outcome = gate( + &review(vec![ + finding("src/lib.rs", Some(1), "nit", ReviewSeverity::Info), + finding("src/lib.rs", Some(2), "bug", ReviewSeverity::High), + ]), + &[], + Some(ReviewSeverity::Medium), + ); + assert_eq!(outcome.published.len(), 1); + assert_eq!(outcome.published[0].title, "bug"); + assert_eq!(outcome.dropped_below_threshold, 1); + } + + #[test] + fn duplicates_collapse_and_output_orders_by_severity() { + let outcome = gate( + &review(vec![ + finding("src/lib.rs", Some(1), "same", ReviewSeverity::Low), + finding("src/lib.rs", Some(1), "same", ReviewSeverity::Low), + finding("src/lib.rs", Some(9), "worse", ReviewSeverity::Critical), + ]), + &[], + None, + ); + assert_eq!(outcome.dropped_duplicates, 1); + assert_eq!( + outcome + .published + .iter() + .map(|f| f.title.as_str()) + .collect::>(), + vec!["worse", "same"] + ); + } + + #[test] + fn render_reports_published_and_withheld_honestly() { + let outcome = gate( + &review(vec![ + finding("src/lib.rs", Some(10), "Off-by-one", ReviewSeverity::High), + finding("elsewhere.rs", None, "speculative", ReviewSeverity::Low), + ]), + &[], + None, + ); + let text = render(&outcome); + assert!(text.contains("**HIGH** `src/lib.rs:10` — Off-by-one"), "{text}"); + assert!(text.contains("1 finding(s) withheld"), "{text}"); + assert!(text.contains("1 out of scope"), "{text}"); + } + + #[test] + fn render_names_the_empty_case() { + let outcome = gate(&review(vec![]), &[], None); + assert_eq!(render(&outcome), "**Findings:** none published."); + } + + #[test] + fn digest_is_bounded() { + let many: Vec = (0..5000) + .map(|i| { + finding( + "src/lib.rs", + Some(i), + &format!("finding number {i} with a reasonably long title"), + ReviewSeverity::Medium, + ) + }) + .collect(); + let text = render(&gate(&review(many), &[], None)); + assert!(text.len() < 50_000, "digest must stay bounded: {}", text.len()); + assert!(text.contains("digest truncated")); + } + + #[test] + fn severity_strings_parse_and_reject_garbage() { + assert_eq!(parse_severity("HIGH"), Some(ReviewSeverity::High)); + assert_eq!(parse_severity("info"), Some(ReviewSeverity::Info)); + assert_eq!(parse_severity("everything"), None); + } +} diff --git a/crates/worker/src/lib.rs b/crates/worker/src/lib.rs index 3cc3679..6b53401 100644 --- a/crates/worker/src/lib.rs +++ b/crates/worker/src/lib.rs @@ -14,6 +14,7 @@ use coven_github_config::{Config, FamiliarConfig}; use coven_github_store::{Store, Terminal, TerminalState}; pub mod brief; +pub mod findings; pub mod memory; pub mod redact; pub mod status_comment; @@ -416,6 +417,60 @@ async fn execute_task_with_minter( .finish(&task.id, terminal_of(&published)) .await .ok(); + // Findings pass the deterministic publication gates before any + // surface sees them (issue #11): scope, severity policy, dedupe. + // The digest always lands on the Check Run; policy can add the + // status comment (advisory) or a blocking PR review verdict. + let mut check_summary = published.result.summary.clone(); + let mut advisory: Option = None; + if let TaskKind::ReviewPullRequest { pr_number, .. } = &task.kind { + let repo_key = format!("{}/{}", task.repo_owner, task.repo_name); + let min_severity = config + .review + .min_severity_for(&repo_key) + .as_deref() + .and_then(findings::parse_severity); + let outcome = findings::gate( + &published.result.review, + &published.changed_files, + min_severity, + ); + let report = findings::render(&outcome); + check_summary = format!("{check_summary}\n\n{report}"); + match config.review.publish_for(&repo_key).as_deref() { + Some("advisory_comment") => advisory = Some(report), + Some("request_changes") => { + // Blocking verdicts need write authority: mint the + // publication token only now, post-gates (issue #4). + let verdict = if outcome.published.is_empty() { + pr::ReviewVerdict::Comment + } else { + pr::ReviewVerdict::RequestChanges + }; + match minter.mint(TokenRole::Publication).await { + Ok(publication) => { + if let Err(e) = pr::submit_review_with_base_url( + api_base_url, + &publication, + &task.repo_owner, + &task.repo_name, + *pr_number, + verdict, + &check_summary, + ) + .await + { + warn!(task_id = %task.id, "failed to submit PR review: {e:#}"); + } + } + Err(e) => { + warn!(task_id = %task.id, "failed to mint publication token for review verdict: {e:#}"); + } + } + } + _ => {} + } + } // Terminal state on the marker-backed status surface (issue #13). if let Some(number) = surface_number(&task.kind) { let marker = status_comment::marker( @@ -424,8 +479,11 @@ async fn execute_task_with_minter( &task.repo_name, number, ); - let body = + let mut body = final_status_body(config, &task.id, &published.result, published.opened_pr); + if let Some(report) = &advisory { + body = format!("{body}\n\n{report}"); + } if let Err(e) = status_comment::upsert( api_base_url, &orchestration, @@ -448,7 +506,7 @@ async fn execute_task_with_minter( check_id, disp.conclusion, disp.title, - &published.result.summary, + &check_summary, ) .await { @@ -519,6 +577,9 @@ async fn execute_task_with_minter( struct Published { result: SessionResult, opened_pr: Option, + /// PR changed-file list fetched from live GitHub state (review tasks) — + /// one input to the findings scope gate (issue #11). + changed_files: Vec, /// Set when a PR review's head moved while the session ran (issue #8): /// the findings were computed against `reviewed_sha`, but the PR is now /// at `current_sha`. Publication must mark the task superseded instead of @@ -761,6 +822,7 @@ async fn run_and_publish( return Ok(Published { result, opened_pr: None, + changed_files: Vec::new(), stale: Some(StaleRefs { reviewed_sha: targets.head_sha.clone(), current_sha: refs.head_sha, @@ -826,6 +888,7 @@ async fn run_and_publish( Ok(Published { result, opened_pr, + changed_files: review.map(|r| r.files).unwrap_or_default(), stale: None, }) } @@ -2818,3 +2881,243 @@ impl WithId for Task { self } } + +#[cfg(test)] +mod publication_gate_tests { + //! End-to-end proof of the findings publication gates (issue #11): + //! out-of-scope, duplicate, and below-threshold findings are withheld, + //! the digest is honest about it, and the `request_changes` / + //! `advisory_comment` policy modes route the verdict correctly. + use super::*; + use coven_github_api::installation::TokenRole; + use coven_github_config::{FamiliarConfig, GitHubAppConfig, ServerConfig, WorkerConfig}; + use std::collections::HashMap; + use std::fs; + use std::os::unix::fs::PermissionsExt; + use std::path::PathBuf; + use wiremock::matchers::{method, path}; + use wiremock::{Mock, MockServer, ResponseTemplate}; + + const ORCHESTRATION: &str = "ghs_orchestration0000000000000000000000"; + const AGENT_GIT: &str = "ghs_agentgit000000000000000000000000000"; + const PUBLICATION: &str = "ghs_publication0000000000000000000000000"; + + fn fixed_minter() -> Minter { + Minter::Fixed(HashMap::from([ + (TokenRole::Orchestration, ORCHESTRATION.to_string()), + (TokenRole::AgentGit, AGENT_GIT.to_string()), + (TokenRole::Publication, PUBLICATION.to_string()), + ])) + } + + /// Review result with one publishable HIGH finding plus one duplicate, + /// one out-of-scope file, and one INFO nit for the threshold gate. + const RESULT_JSON: &str = r#"{"contract_version":"2","status":"success","branch":null,"commits":[],"files_changed":[],"summary":"Reviewed the change.","pr_body":"","review":{"mode":"pull_request","evidence_status":"complete","reviewed_files":["src/lib.rs"],"supporting_files":[],"findings":[{"severity":"high","file":"src/lib.rs","line":10,"title":"Off-by-one","body":"Loop bound skips the last element.","recommendation":null},{"severity":"high","file":"src/lib.rs","line":10,"title":"Off-by-one","body":"Loop bound skips the last element.","recommendation":null},{"severity":"critical","file":"secrets/vault.rs","line":null,"title":"Speculative","body":"Never consulted this file.","recommendation":null},{"severity":"info","file":"src/lib.rs","line":20,"title":"Nit","body":"Prefer a doc comment.","recommendation":null}],"tests_run":[],"no_findings_reason":null,"limitations":[]},"exit_reason":null}"#; + + async fn run_review(policy: coven_github_config::ReviewConfig) -> Vec { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/repos/OpenCoven/demo")) + .respond_with( + ResponseTemplate::new(200) + .set_body_json(serde_json::json!({"default_branch": "main"})), + ) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/repos/OpenCoven/demo/pulls/88")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "head": { "ref": "feat/x", "sha": "stable-sha" }, + "base": { "ref": "main", "sha": "base-sha" } + }))) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/repos/OpenCoven/demo/pulls/88/files")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!([ + { "filename": "src/lib.rs" } + ]))) + .mount(&server) + .await; + Mock::given(method("POST")) + .and(path("/repos/OpenCoven/demo/check-runs")) + .respond_with(ResponseTemplate::new(201).set_body_json(serde_json::json!({"id": 7}))) + .mount(&server) + .await; + Mock::given(method("PATCH")) + .and(path("/repos/OpenCoven/demo/check-runs/7")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({}))) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/repos/OpenCoven/demo/issues/88/comments")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!([]))) + .mount(&server) + .await; + Mock::given(method("POST")) + .and(path("/repos/OpenCoven/demo/issues/88/comments")) + .respond_with(ResponseTemplate::new(201).set_body_json(serde_json::json!({"id": 1}))) + .mount(&server) + .await; + Mock::given(method("POST")) + .and(path("/repos/OpenCoven/demo/pulls/88/reviews")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({}))) + .mount(&server) + .await; + + let script = format!("#!/usr/bin/env bash\ncat > \"$5\" <<'RESULT'\n{RESULT_JSON}\nRESULT\nexit 0\n"); + let root = + std::env::temp_dir().join(format!("coven-github-gates-{}", uuid::Uuid::new_v4())); + fs::create_dir_all(&root).expect("test dir"); + let script_path = root.join("fake-coven-code.sh"); + fs::write(&script_path, script).expect("script written"); + fs::set_permissions(&script_path, fs::Permissions::from_mode(0o755)).expect("chmod"); + + let config = Config { + server: ServerConfig { + bind: "127.0.0.1:0".to_string(), + cave_base_url: None, + }, + github: GitHubAppConfig { + app_id: 1, + private_key_path: PathBuf::from("/nonexistent/never-read.pem"), + webhook_secret: "secret".to_string(), + api_base_url: Some(server.uri()), + }, + worker: WorkerConfig { + concurrency: 1, + coven_code_bin: script_path, + workspace_root: root.clone(), + timeout_secs: 30, + max_retries: 0, + }, + familiars: vec![FamiliarConfig { + id: "cody".to_string(), + display_name: "Cody".to_string(), + bot_username: "coven-cody[bot]".to_string(), + model: None, + skills: vec![], + trigger_labels: vec![], + }], + review: policy, + storage: coven_github_config::StorageConfig::default(), + memory: coven_github_config::MemoryConfig::default(), + api: coven_github_config::ApiConfig::default(), + }; + let task = Task { + id: "task-gates".to_string(), + installation_id: 1, + repo_owner: "OpenCoven".to_string(), + repo_name: "demo".to_string(), + familiar_id: "cody".to_string(), + commander: None, + kind: TaskKind::ReviewPullRequest { + pr_number: 88, + pr_title: "t".to_string(), + reason: "synchronize".to_string(), + }, + }; + + execute_task_with_minter( + &config, + Store::open_in_memory().expect("store"), + task, + &fixed_minter(), + ) + .await + .expect("review must publish cleanly"); + + let requests = server.received_requests().await.expect("requests"); + let _ = fs::remove_dir_all(root); + requests + } + + fn policy(min_severity: Option<&str>, publish: Option<&str>) -> coven_github_config::ReviewConfig { + coven_github_config::ReviewConfig { + familiar: Some("cody".to_string()), + pull_request: true, + include_drafts: false, + audit_instruction: None, + min_severity: min_severity.map(str::to_string), + publish: publish.map(str::to_string), + repos: std::collections::HashMap::new(), + } + } + + #[tokio::test] + async fn gated_digest_lands_on_the_check_run_with_honest_counts() { + let requests = run_review(policy(Some("medium"), None)).await; + let terminal = requests + .iter() + .filter(|r| { + r.method.as_str() == "PATCH" + && r.url.path() == "/repos/OpenCoven/demo/check-runs/7" + }) + .map(|r| String::from_utf8_lossy(&r.body).to_string()) + .next_back() + .expect("terminal check patch"); + assert!(terminal.contains("Off-by-one"), "digest published: {terminal}"); + assert!( + !terminal.contains("Speculative"), + "out-of-scope finding must be withheld: {terminal}" + ); + assert!( + !terminal.contains("Prefer a doc comment"), + "below-threshold finding must be withheld: {terminal}" + ); + assert!( + terminal.contains("3 finding(s) withheld"), + "withheld counts must be stated: {terminal}" + ); + // Default mode: no PR review submitted, no advisory digest on comment. + assert!( + !requests + .iter() + .any(|r| r.url.path() == "/repos/OpenCoven/demo/pulls/88/reviews"), + "check_run mode must not submit a PR review" + ); + } + + #[tokio::test] + async fn request_changes_mode_submits_a_blocking_review_with_write_authority() { + let requests = run_review(policy(None, Some("request_changes"))).await; + let review_post = requests + .iter() + .find(|r| { + r.method.as_str() == "POST" + && r.url.path() == "/repos/OpenCoven/demo/pulls/88/reviews" + }) + .expect("PR review must be submitted"); + let body = String::from_utf8_lossy(&review_post.body); + assert!(body.contains("REQUEST_CHANGES"), "verdict: {body}"); + assert!(body.contains("Off-by-one"), "digest in verdict body: {body}"); + // The verdict is write-authority work: publication token, never + // orchestration (issue #4 boundary). + let auth = review_post + .headers + .get("authorization") + .expect("auth header") + .to_str() + .expect("ascii"); + assert_eq!(auth, format!("Bearer {PUBLICATION}")); + } + + #[tokio::test] + async fn advisory_mode_appends_the_digest_to_the_status_comment() { + let requests = run_review(policy(None, Some("advisory_comment"))).await; + let comment_bodies: Vec = requests + .iter() + .filter(|r| { + r.method.as_str() == "POST" + && r.url.path() == "/repos/OpenCoven/demo/issues/88/comments" + }) + .map(|r| String::from_utf8_lossy(&r.body).to_string()) + .collect(); + assert!( + comment_bodies + .iter() + .any(|b| b.contains("Status: done") && b.contains("Off-by-one")), + "advisory digest must ride the status comment: {comment_bodies:?}" + ); + } +}