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
8 changes: 7 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 5 additions & 0 deletions config/example.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
82 changes: 82 additions & 0 deletions crates/config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,
/// Minimum finding severity that publishes (`info`, `low`, `medium`,
/// `high`, `critical`). Absent = every severity publishes (issue #11).
pub min_severity: Option<String>,
/// 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<String>,
/// Per-repo overrides keyed "owner/name".
#[serde(default)]
pub repos: std::collections::HashMap<String, RepoReviewOverride>,
Expand All @@ -155,6 +162,8 @@ pub struct RepoReviewOverride {
pub include_drafts: Option<bool>,
pub familiar: Option<String>,
pub audit_instruction: Option<String>,
pub min_severity: Option<String>,
pub publish: Option<String>,
}

impl ReviewConfig {
Expand Down Expand Up @@ -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<String> {
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<String> {
self.overrides(repo)
.and_then(|o| o.publish.clone())
.or_else(|| self.publish.clone())
}
}
Comment on lines +199 to 211

#[derive(Debug, Clone, Deserialize, Serialize)]
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.",
}
}
Expand Down Expand Up @@ -729,17 +792,36 @@ 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 {
pull_request: Some(false),
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"));
Expand Down
64 changes: 64 additions & 0 deletions crates/github/src/pr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand All @@ -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<IssueComment> = serde_json::from_value(json!([
Expand Down
4 changes: 4 additions & 0 deletions crates/webhook/src/routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}
}
Expand Down Expand Up @@ -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);
Expand Down
Loading
Loading