dk_runner/steps/agent_review/
mod.rs

1pub mod provider;
2pub mod claude;
3pub mod openrouter;
4pub mod prompt;
5pub mod parse;
6
7use std::time::Instant;
8use crate::executor::{StepOutput, StepStatus};
9use crate::findings::{Finding, Suggestion};
10use provider::{ReviewProvider, ReviewRequest, FileContext, ReviewVerdict};
11
12pub async fn run_agent_review_step_with_provider(
13    provider: &dyn ReviewProvider,
14    diff: &str,
15    files: Vec<FileContext>,
16    intent: &str,
17) -> (StepOutput, Vec<Finding>, Vec<Suggestion>) {
18    let start = Instant::now();
19    let request = ReviewRequest {
20        diff: diff.to_string(),
21        context: files,
22        language: "rust".to_string(),
23        intent: intent.to_string(),
24    };
25
26    match provider.review(request).await {
27        Ok(response) => {
28            let status = match response.verdict {
29                ReviewVerdict::Approve => StepStatus::Pass,
30                ReviewVerdict::RequestChanges => StepStatus::Fail,
31                ReviewVerdict::Comment => StepStatus::Pass,
32            };
33            (
34                StepOutput {
35                    status,
36                    stdout: format!(
37                        "Agent Review ({}): {}",
38                        provider.name(),
39                        response.summary
40                    ),
41                    stderr: String::new(),
42                    duration: start.elapsed(),
43                },
44                response.findings,
45                response.suggestions,
46            )
47        }
48        Err(e) => {
49            let finding = Finding {
50                severity: crate::findings::Severity::Warning,
51                check_name: "agent-review-error".to_string(),
52                message: format!("Agent review failed: {e}"),
53                file_path: None,
54                line: None,
55                symbol: None,
56            };
57            (
58                StepOutput {
59                    status: StepStatus::Pass,
60                    stdout: format!(
61                        "Agent Review ({}): error -- {e}",
62                        provider.name()
63                    ),
64                    stderr: String::new(),
65                    duration: start.elapsed(),
66                },
67                vec![finding],
68                Vec::new(),
69            )
70        }
71    }
72}
73
74/// Legacy stub for when no provider is configured.
75pub async fn run_agent_review_step(prompt: &str) -> StepOutput {
76    let start = Instant::now();
77    StepOutput {
78        status: StepStatus::Pass,
79        stdout: format!(
80            "agent review: skipped (no provider configured)\nprompt: {}",
81            prompt
82        ),
83        stderr: String::new(),
84        duration: start.elapsed(),
85    }
86}
87
88/// Select a `ReviewProvider` based on environment variables.
89///
90/// Precedence:
91/// 1. `DKOD_OPENROUTER_API_KEY` → `OpenRouterReviewProvider`
92/// 2. `DKOD_ANTHROPIC_API_KEY`  → `ClaudeReviewProvider`
93/// 3. Neither                    → `None`
94///
95/// When both keys are set, OpenRouter wins (single routing point for
96/// cost tracking + model flexibility).
97pub fn select_provider_from_env() -> Option<Box<dyn provider::ReviewProvider>> {
98    if std::env::var("DKOD_OPENROUTER_API_KEY").is_ok() {
99        let provider = openrouter::OpenRouterReviewProvider::from_env()
100            .map(|p| Box::new(p) as Box<dyn provider::ReviewProvider>);
101        if provider.is_none() {
102            tracing::warn!(
103                "DKOD_OPENROUTER_API_KEY is set but OpenRouterReviewProvider failed to initialise; no review provider active"
104            );
105        }
106        return provider;
107    }
108    if let Ok(key) = std::env::var("DKOD_ANTHROPIC_API_KEY") {
109        let model = std::env::var("DKOD_REVIEW_MODEL").ok();
110        let provider = claude::ClaudeReviewProvider::new(key, model, None)
111            .ok()
112            .map(|p| Box::new(p) as Box<dyn provider::ReviewProvider>);
113        if provider.is_none() {
114            tracing::warn!(
115                "DKOD_ANTHROPIC_API_KEY is set but ClaudeReviewProvider failed to initialise; no review provider active"
116            );
117        }
118        return provider;
119    }
120    None
121}
122
123#[cfg(test)]
124mod provider_factory_tests {
125    use super::select_provider_from_env;
126
127    fn clear() {
128        for k in ["DKOD_ANTHROPIC_API_KEY", "DKOD_OPENROUTER_API_KEY", "DKOD_REVIEW_MODEL", "DKOD_OPENROUTER_BASE_URL"] {
129            std::env::remove_var(k);
130        }
131    }
132
133    #[test]
134    #[serial_test::serial]
135    fn openrouter_wins_when_both_keys_set() {
136        clear();
137        std::env::set_var("DKOD_ANTHROPIC_API_KEY", "sk-ant");
138        std::env::set_var("DKOD_OPENROUTER_API_KEY", "sk-or");
139        let p = select_provider_from_env().expect("expected a provider");
140        assert_eq!(p.name(), "openrouter");
141    }
142
143    #[test]
144    #[serial_test::serial]
145    fn anthropic_selected_when_only_anthropic_set() {
146        clear();
147        std::env::set_var("DKOD_ANTHROPIC_API_KEY", "sk-ant");
148        let p = select_provider_from_env().expect("expected a provider");
149        assert_eq!(p.name(), "anthropic");
150    }
151
152    #[test]
153    #[serial_test::serial]
154    fn none_when_no_keys() {
155        clear();
156        assert!(select_provider_from_env().is_none());
157    }
158}
159
160#[cfg(test)]
161mod tests {
162    use super::*;
163    use crate::findings::Severity;
164
165    struct MockProvider {
166        response: Result<provider::ReviewResponse, String>,
167    }
168
169    #[async_trait::async_trait]
170    impl provider::ReviewProvider for MockProvider {
171        fn name(&self) -> &str {
172            "mock"
173        }
174        async fn review(
175            &self,
176            _req: provider::ReviewRequest,
177        ) -> anyhow::Result<provider::ReviewResponse> {
178            match &self.response {
179                Ok(r) => Ok(r.clone()),
180                Err(msg) => anyhow::bail!("{}", msg),
181            }
182        }
183    }
184
185    fn make_request_args() -> (String, Vec<provider::FileContext>, String) {
186        ("diff".to_string(), vec![], "test intent".to_string())
187    }
188
189    #[tokio::test]
190    async fn test_approve_verdict_returns_pass() {
191        let provider = MockProvider {
192            response: Ok(provider::ReviewResponse {
193                summary: "LGTM".to_string(),
194                findings: vec![],
195                suggestions: vec![],
196                verdict: provider::ReviewVerdict::Approve,
197            }),
198        };
199        let (diff, files, intent) = make_request_args();
200        let (output, findings, suggestions) =
201            run_agent_review_step_with_provider(&provider, &diff, files, &intent).await;
202        assert_eq!(output.status, StepStatus::Pass);
203        assert!(findings.is_empty());
204        assert!(suggestions.is_empty());
205    }
206
207    #[tokio::test]
208    async fn test_request_changes_verdict_returns_fail() {
209        let provider = MockProvider {
210            response: Ok(provider::ReviewResponse {
211                summary: "Issues found".to_string(),
212                findings: vec![Finding {
213                    severity: Severity::Error,
214                    check_name: "test".to_string(),
215                    message: "bad".to_string(),
216                    file_path: None,
217                    line: None,
218                    symbol: None,
219                }],
220                suggestions: vec![],
221                verdict: provider::ReviewVerdict::RequestChanges,
222            }),
223        };
224        let (diff, files, intent) = make_request_args();
225        let (output, findings, _) =
226            run_agent_review_step_with_provider(&provider, &diff, files, &intent).await;
227        assert_eq!(output.status, StepStatus::Fail);
228        assert_eq!(findings.len(), 1);
229    }
230
231    #[tokio::test]
232    async fn test_provider_error_returns_pass_with_warning() {
233        let provider = MockProvider {
234            response: Err("API error".to_string()),
235        };
236        let (diff, files, intent) = make_request_args();
237        let (output, findings, _) =
238            run_agent_review_step_with_provider(&provider, &diff, files, &intent).await;
239        assert_eq!(output.status, StepStatus::Pass); // soft fail
240        assert_eq!(findings.len(), 1);
241        assert_eq!(findings[0].severity, Severity::Warning);
242    }
243
244    #[tokio::test]
245    async fn test_legacy_stub_passes() {
246        let output = run_agent_review_step("test prompt").await;
247        assert_eq!(output.status, StepStatus::Pass);
248        assert!(output.stdout.contains("no provider configured"));
249    }
250}
dk_runner/steps/agent_review/mod.rs

dk_runner/steps/agent_review/
mod.rs
