diff --git a/README.md b/README.md index 949a683..9dc73db 100644 --- a/README.md +++ b/README.md @@ -317,6 +317,8 @@ diffscope doctor --base-url http://localhost:11434 |----------|-------------| | `DIFFSCOPE_BASE_URL` | LLM API base URL (also accepts `OPENAI_BASE_URL`) | | `DIFFSCOPE_API_KEY` | API key for the LLM endpoint | +| `DIFFSCOPE_GITHUB_AUTO_REVIEW_EVENTS` | Comma-separated `pull_request` actions that start webhook reviews, such as `opened,synchronize` or `review_requested` | +| `DIFFSCOPE_GITHUB_REVIEW_REQUEST_REVIEWERS` | Comma-separated GitHub logins whose requested reviews trigger `review_requested` automation, for example `EvalOpsBot` | #### CLI Flags | Flag | Description | @@ -955,6 +957,17 @@ Planned support for responding to pull request comments with interactive command @diffscope help # Show all commands ``` +### Requested Reviewer Automation + +Server webhook deployments can run reviews only when a specific reviewer is requested. For an org-level EvalOpsBot setup, subscribe the GitHub App or webhook to `pull_request` events and run the server with: + +```bash +DIFFSCOPE_GITHUB_AUTO_REVIEW_EVENTS=review_requested +DIFFSCOPE_GITHUB_REVIEW_REQUEST_REVIEWERS=EvalOpsBot +``` + +With that configuration, `pull_request.review_requested` starts a full DiffScope review only when GitHub reports `requested_reviewer.login` as `EvalOpsBot`. + ### ✅ Feedback Loop (Reduce Repeated False Positives) Use the feedback store to suppress comments you’ve already reviewed: diff --git a/charts/diffscope/templates/configmap.yaml b/charts/diffscope/templates/configmap.yaml index 00fef9d..8b54f4f 100644 --- a/charts/diffscope/templates/configmap.yaml +++ b/charts/diffscope/templates/configmap.yaml @@ -6,6 +6,12 @@ metadata: {{- include "diffscope.labels" . | nindent 4 }} data: DIFFSCOPE_MODEL: {{ .Values.diffscope.model | quote }} + {{- if .Values.diffscope.github.autoReviewEvents }} + DIFFSCOPE_GITHUB_AUTO_REVIEW_EVENTS: {{ join "," .Values.diffscope.github.autoReviewEvents | quote }} + {{- end }} + {{- if .Values.diffscope.github.reviewRequestReviewers }} + DIFFSCOPE_GITHUB_REVIEW_REQUEST_REVIEWERS: {{ join "," .Values.diffscope.github.reviewRequestReviewers | quote }} + {{- end }} {{- if .Values.diffscope.baseUrl }} DIFFSCOPE_BASE_URL: {{ .Values.diffscope.baseUrl | quote }} {{- else if .Values.ollama.enabled }} diff --git a/charts/diffscope/values.yaml b/charts/diffscope/values.yaml index d570e74..4802b7a 100644 --- a/charts/diffscope/values.yaml +++ b/charts/diffscope/values.yaml @@ -23,6 +23,14 @@ diffscope: adapter: "" # Base URL for LLM API (auto-set to Ollama service URL when ollama.enabled) baseUrl: "" + github: + # pull_request actions that start server-side reviews. + # Set to ["review_requested"] with reviewRequestReviewers for on-demand org review bots. + autoReviewEvents: + - opened + - synchronize + # GitHub user logins whose requested reviews trigger review_requested automation. + reviewRequestReviewers: [] extraArgs: [] # -- API keys (stored in a Secret) diff --git a/docs/self-hosting.md b/docs/self-hosting.md index 020b01d..81ed6cc 100644 --- a/docs/self-hosting.md +++ b/docs/self-hosting.md @@ -69,6 +69,8 @@ config: extraEnv: DIFFSCOPE_SERVER_API_KEY: ${DIFFSCOPE_SERVER_API_KEY} + DIFFSCOPE_GITHUB_AUTO_REVIEW_EVENTS: review_requested + DIFFSCOPE_GITHUB_REVIEW_REQUEST_REVIEWERS: EvalOpsBot ANTHROPIC_API_KEY: ${ANTHROPIC_API_KEY} OPENROUTER_API_KEY: ${OPENROUTER_API_KEY} ``` @@ -89,14 +91,22 @@ For enterprise installs, prefer external secret injection over storing credentia - `OPENAI_API_KEY` - `OPENROUTER_API_KEY` - `DIFFSCOPE_SERVER_API_KEY` +- `DIFFSCOPE_WEBHOOK_SECRET` - `GITHUB_TOKEN` - `DIFFSCOPE_GITHUB_APP_ID` - `DIFFSCOPE_GITHUB_PRIVATE_KEY` +- `DIFFSCOPE_GITHUB_AUTO_REVIEW_EVENTS` +- `DIFFSCOPE_GITHUB_REVIEW_REQUEST_REVIEWERS` - `DIFFSCOPE_JIRA_BASE_URL` - `DIFFSCOPE_JIRA_EMAIL` - `DIFFSCOPE_JIRA_API_TOKEN` - `DIFFSCOPE_LINEAR_API_KEY` +For an on-demand review bot, set `DIFFSCOPE_GITHUB_AUTO_REVIEW_EVENTS=review_requested` +and `DIFFSCOPE_GITHUB_REVIEW_REQUEST_REVIEWERS=EvalOpsBot`. The GitHub webhook must +subscribe to `pull_request` events; DiffScope will ignore review requests for other +reviewers. + ### Validation behavior `diffscope doctor`, the server doctor endpoint, and startup warnings now surface configuration issues for: diff --git a/src/config.rs b/src/config.rs index 77b1d2c..a6f3bb9 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,6 +1,6 @@ use anyhow::Result; use serde::{Deserialize, Serialize}; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; use tracing::warn; @@ -150,7 +150,7 @@ pub struct VaultConfig { pub namespace: Option, } -#[derive(Debug, Clone, Serialize, Deserialize, Default)] +#[derive(Debug, Clone, Serialize, Deserialize)] pub struct GitHubConfig { #[serde(default, rename = "github_token")] pub token: Option, @@ -174,6 +174,37 @@ pub struct GitHubConfig { /// Webhook secret for verifying GitHub webhook signatures. #[serde(default, rename = "github_webhook_secret")] pub webhook_secret: Option, + + /// pull_request actions that should start an automated review. + /// + /// Supported values: opened, synchronize, reopened, review_requested. + #[serde( + default = "default_github_auto_review_events", + rename = "github_auto_review_events" + )] + pub auto_review_events: Vec, + + /// GitHub user logins that can trigger review_requested automation. + /// + /// This is intentionally separate from auto_review_events so an org-level + /// webhook can listen to pull_request events without reviewing every PR. + #[serde(default, rename = "github_review_request_reviewers")] + pub review_request_reviewers: Vec, +} + +impl Default for GitHubConfig { + fn default() -> Self { + Self { + token: None, + app_id: None, + client_id: None, + client_secret: None, + private_key: None, + webhook_secret: None, + auto_review_events: default_github_auto_review_events(), + review_request_reviewers: Vec::new(), + } + } } #[derive(Debug, Clone, Serialize, Deserialize, Default)] @@ -1034,6 +1065,16 @@ impl Config { .ok() .filter(|s| !s.trim().is_empty()); } + if let Ok(events) = std::env::var("DIFFSCOPE_GITHUB_AUTO_REVIEW_EVENTS") { + self.github.auto_review_events = split_csv_list(&events); + } + if self.github.review_request_reviewers.is_empty() { + self.github.review_request_reviewers = + std::env::var("DIFFSCOPE_GITHUB_REVIEW_REQUEST_REVIEWERS") + .ok() + .map(|reviewers| split_csv_list(&reviewers)) + .unwrap_or_default(); + } if self.jira.base_url.is_none() { self.jira.base_url = std::env::var("DIFFSCOPE_JIRA_BASE_URL") .ok() @@ -1090,6 +1131,8 @@ impl Config { normalize_optional_trimmed(&mut self.github.client_secret); normalize_optional_trimmed(&mut self.github.private_key); normalize_optional_trimmed(&mut self.github.webhook_secret); + normalize_lowercase_list(&mut self.github.auto_review_events); + normalize_string_list(&mut self.github.review_request_reviewers); normalize_optional_trimmed(&mut self.jira.base_url); normalize_optional_trimmed(&mut self.jira.email); normalize_optional_trimmed(&mut self.jira.api_token); @@ -1701,6 +1744,32 @@ impl Config { "github_client_secret is set without github_client_id. The device flow only uses github_client_id today.", )); } + for event in &self.github.auto_review_events { + if !matches!( + event.as_str(), + "opened" | "synchronize" | "reopened" | "review_requested" + ) { + issues.push(ConfigValidationIssue::warning( + "github_auto_review_events", + format!( + "Unsupported GitHub pull_request action '{}'; supported values are opened, synchronize, reopened, and review_requested.", + event + ), + )); + } + } + if self + .github + .auto_review_events + .iter() + .any(|event| event == "review_requested") + && self.github.review_request_reviewers.is_empty() + { + issues.push(ConfigValidationIssue::warning( + "github_review_request_reviewers", + "review_requested automation is enabled but no requested reviewer logins are configured.", + )); + } let jira_fields = [ self.jira.base_url.as_ref(), @@ -1827,6 +1896,37 @@ fn normalize_optional_trimmed(value: &mut Option) { .map(ToOwned::to_owned); } +fn split_csv_list(value: &str) -> Vec { + value.split(',').map(ToOwned::to_owned).collect() +} + +fn normalize_string_list(values: &mut Vec) { + let mut seen = HashSet::new(); + values.retain_mut(|value| { + let trimmed = value.trim(); + if trimmed.is_empty() { + return false; + } + + let normalized_key = trimmed.to_ascii_lowercase(); + if !seen.insert(normalized_key) { + return false; + } + + if trimmed.len() != value.len() { + *value = trimmed.to_string(); + } + true + }); +} + +fn normalize_lowercase_list(values: &mut Vec) { + normalize_string_list(values); + for value in values { + *value = value.to_ascii_lowercase(); + } +} + fn apply_provider_env_api_key_fallback( providers: &mut HashMap, provider_name: &str, @@ -1903,6 +2003,10 @@ fn default_temperature() -> f32 { 0.2 } +fn default_github_auto_review_events() -> Vec { + vec!["opened".to_string(), "synchronize".to_string()] +} + fn default_max_tokens() -> usize { 4000 } @@ -2927,6 +3031,88 @@ auditing_model_role: primary assert!(issues.iter().any(|issue| issue.field == "vault")); } + #[test] + fn test_github_auto_review_events_default_to_opened_and_synchronize() { + let config = Config::default(); + + assert_eq!( + config.github.auto_review_events, + vec!["opened".to_string(), "synchronize".to_string()] + ); + } + + #[test] + fn test_config_deserialize_github_review_request_controls() { + let config: Config = serde_yaml::from_str( + r#" +github_auto_review_events: + - review_requested +github_review_request_reviewers: + - EvalOpsBot +"#, + ) + .unwrap(); + + assert_eq!( + config.github.auto_review_events, + vec!["review_requested".to_string()] + ); + assert_eq!( + config.github.review_request_reviewers, + vec!["EvalOpsBot".to_string()] + ); + } + + #[test] + fn test_normalize_github_review_request_controls() { + let mut config = Config { + github: GitHubConfig { + auto_review_events: vec![ + " Review_Requested ".to_string(), + "review_requested".to_string(), + "opened".to_string(), + "".to_string(), + ], + review_request_reviewers: vec![ + " EvalOpsBot ".to_string(), + "evalopsbot".to_string(), + "AnotherBot".to_string(), + "".to_string(), + ], + ..GitHubConfig::default() + }, + ..Config::default() + }; + + config.normalize(); + + assert_eq!( + config.github.auto_review_events, + vec!["review_requested".to_string(), "opened".to_string()] + ); + assert_eq!( + config.github.review_request_reviewers, + vec!["EvalOpsBot".to_string(), "AnotherBot".to_string()] + ); + } + + #[test] + fn test_validation_warns_when_review_request_enabled_without_reviewers() { + let config = Config { + github: GitHubConfig { + auto_review_events: vec!["review_requested".to_string()], + review_request_reviewers: Vec::new(), + ..GitHubConfig::default() + }, + ..Config::default() + }; + + let issues = config.validation_issues(); + assert!(issues + .iter() + .any(|issue| issue.field == "github_review_request_reviewers")); + } + #[test] fn test_normalize_rejects_embedding_for_generation_verification_and_auditing() { let mut config = Config { diff --git a/src/server/github.rs b/src/server/github.rs index 99580d8..72dc97d 100644 --- a/src/server/github.rs +++ b/src/server/github.rs @@ -278,6 +278,8 @@ pub async fn handle_webhook( let token = config.github.token.clone(); let github_app_id = config.github.app_id; let private_key = config.github.private_key.clone(); + let auto_review_events = config.github.auto_review_events.clone(); + let review_request_reviewers = config.github.review_request_reviewers.clone(); drop(config); info!(event = %event_type, "Received GitHub webhook"); @@ -289,8 +291,15 @@ pub async fn handle_webhook( let action = payload["action"].as_str().unwrap_or(""); - // Auto-review on opened or synchronized (new push) - if matches!(action, "opened" | "synchronize") { + // Auto-review on configured pull_request actions. review_requested + // is gated by requested reviewer login so an org webhook can stay + // centralized without reviewing every PR. + if should_start_review_for_pull_request_action( + action, + &payload, + &auto_review_events, + &review_request_reviewers, + ) { let repo = payload["repository"]["full_name"] .as_str() .unwrap_or("") @@ -547,6 +556,43 @@ pub async fn handle_webhook( Ok(Json(serde_json::json!({ "ok": true, "action": "ignored" }))) } +fn should_start_review_for_pull_request_action( + action: &str, + payload: &serde_json::Value, + auto_review_events: &[String], + review_request_reviewers: &[String], +) -> bool { + if !matches!( + action, + "opened" | "synchronize" | "reopened" | "review_requested" + ) { + return false; + } + if !auto_review_events + .iter() + .any(|event| event.eq_ignore_ascii_case(action)) + { + return false; + } + if action != "review_requested" { + return true; + } + + let Some(login) = requested_reviewer_login(payload) else { + return false; + }; + review_request_reviewers + .iter() + .any(|reviewer| reviewer.trim().eq_ignore_ascii_case(login.trim())) +} + +fn requested_reviewer_login(payload: &serde_json::Value) -> Option<&str> { + payload + .get("requested_reviewer") + .and_then(|reviewer| reviewer.get("login")) + .and_then(|login| login.as_str()) +} + fn verify_webhook_signature(secret: &str, body: &str, signature: &str) -> Result<(), String> { let sig_hex = signature .strip_prefix("sha256=") @@ -1157,6 +1203,7 @@ async fn run_webhook_review(state: Arc, params: WebhookReviewParams) { #[cfg(test)] mod tests { use super::*; + use serde_json::json; #[test] fn test_hex_encode() { @@ -1187,6 +1234,68 @@ mod tests { assert!(verify_webhook_signature(secret, body, &signature).is_ok()); } + #[test] + fn review_request_action_matches_configured_reviewer() { + let payload = json!({ + "action": "review_requested", + "requested_reviewer": { "login": "evalopsbot" } + }); + let events = vec!["review_requested".to_string()]; + let reviewers = vec!["EvalOpsBot".to_string()]; + + assert!(should_start_review_for_pull_request_action( + "review_requested", + &payload, + &events, + &reviewers + )); + } + + #[test] + fn review_request_action_ignores_unconfigured_reviewer() { + let payload = json!({ + "action": "review_requested", + "requested_reviewer": { "login": "other-bot" } + }); + let events = vec!["review_requested".to_string()]; + let reviewers = vec!["EvalOpsBot".to_string()]; + + assert!(!should_start_review_for_pull_request_action( + "review_requested", + &payload, + &events, + &reviewers + )); + } + + #[test] + fn review_request_action_requires_explicit_event_enablement() { + let payload = json!({ + "action": "review_requested", + "requested_reviewer": { "login": "EvalOpsBot" } + }); + let events = vec!["opened".to_string(), "synchronize".to_string()]; + let reviewers = vec!["EvalOpsBot".to_string()]; + + assert!(!should_start_review_for_pull_request_action( + "review_requested", + &payload, + &events, + &reviewers + )); + } + + #[test] + fn opened_action_matches_when_enabled_without_reviewer_gate() { + let payload = json!({}); + let events = vec!["opened".to_string()]; + let reviewers = Vec::new(); + + assert!(should_start_review_for_pull_request_action( + "opened", &payload, &events, &reviewers + )); + } + #[test] fn test_verify_webhook_signature_invalid() { let secret = "test-secret";