feat: audit fixes — graceful OTEL, async-safe plugins, URL validation, +100 tests

src/core/context.rs

@@ -15,7 +15,7 @@ pub struct LLMContextChunk {
     pub line_range: Option<(usize, usize)>,
 }
 
-#[derive(Debug, Clone, Serialize, Deserialize)]
+#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
 pub enum ContextType {
     FileContent,
     Definition,

src/core/interactive.rs

@@ -246,6 +246,188 @@ Interactive commands respect these configurations."#
     }
 }
 
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    // === InteractiveCommand::parse tests ===
+
+    #[test]
+    fn test_parse_review_command() {
+        let cmd = InteractiveCommand::parse("@diffscope review").unwrap();
+        assert_eq!(cmd.command, CommandType::Review);
+        assert!(cmd.args.is_empty());
+        assert!(cmd.context.is_none());
+    }
+
+    #[test]
+    fn test_parse_review_with_focus() {
+        let cmd = InteractiveCommand::parse("@diffscope review security performance").unwrap();
+        assert_eq!(cmd.command, CommandType::Review);
+        assert_eq!(cmd.args, vec!["security", "performance"]);
+    }
+
+    #[test]
+    fn test_parse_ignore_command() {
+        let cmd = InteractiveCommand::parse("@diffscope ignore src/generated/").unwrap();
+        assert_eq!(cmd.command, CommandType::Ignore);
+        assert_eq!(cmd.args, vec!["src/generated/"]);
+    }
+
+    #[test]
+    fn test_parse_explain_command() {
+        let cmd = InteractiveCommand::parse("@diffscope explain").unwrap();
+        assert_eq!(cmd.command, CommandType::Explain);
+    }
+
+    #[test]
+    fn test_parse_generate_tests() {
+        let cmd = InteractiveCommand::parse("@diffscope generate tests").unwrap();
+        assert_eq!(cmd.command, CommandType::Generate);
+        assert_eq!(cmd.args, vec!["tests"]);
+    }
+
+    #[test]
+    fn test_parse_help() {
+        let cmd = InteractiveCommand::parse("@diffscope help").unwrap();
+        assert_eq!(cmd.command, CommandType::Help);
+    }
+
+    #[test]
+    fn test_parse_config() {
+        let cmd = InteractiveCommand::parse("@diffscope config").unwrap();
+        assert_eq!(cmd.command, CommandType::Config);
+    }
+
+    #[test]
+    fn test_parse_case_insensitive() {
+        let cmd = InteractiveCommand::parse("@diffscope REVIEW").unwrap();
+        assert_eq!(cmd.command, CommandType::Review);
+
+        let cmd = InteractiveCommand::parse("@diffscope Help").unwrap();
+        assert_eq!(cmd.command, CommandType::Help);
+    }
+
+    #[test]
+    fn test_parse_unknown_command_returns_none() {
+        assert!(InteractiveCommand::parse("@diffscope foobar").is_none());
+    }
+
+    #[test]
+    fn test_parse_no_at_mention_returns_none() {
+        assert!(InteractiveCommand::parse("just a regular comment").is_none());
+    }
+
+    #[test]
+    fn test_parse_empty_string_returns_none() {
+        assert!(InteractiveCommand::parse("").is_none());
+    }
+
+    #[test]
+    fn test_parse_at_mention_only_returns_none() {
+        assert!(InteractiveCommand::parse("@diffscope").is_none());
+    }
+
+    #[test]
+    fn test_parse_embedded_in_text() {
+        // Command embedded in longer comment
+        let cmd =
+            InteractiveCommand::parse("Hey team, can you @diffscope review this PR?").unwrap();
+        assert_eq!(cmd.command, CommandType::Review);
+    }
+
+    #[test]
+    fn test_parse_extra_whitespace() {
+        let cmd = InteractiveCommand::parse("@diffscope   review   security").unwrap();
+        assert_eq!(cmd.command, CommandType::Review);
+        assert_eq!(cmd.args, vec!["security"]);
+    }
+
+    #[test]
+    fn test_parse_different_at_mention_ignored() {
+        assert!(InteractiveCommand::parse("@someone review").is_none());
+    }
+
+    // === execute_ignore tests ===
+
+    #[test]
+    fn test_ignore_no_args() {
+        let cmd = InteractiveCommand {
+            command: CommandType::Ignore,
+            args: vec![],
+            context: None,
+        };
+        let result = cmd.execute_ignore().unwrap();
+        assert!(result.contains("specify what to ignore"));
+    }
+
+    #[test]
+    fn test_ignore_with_patterns() {
+        let cmd = InteractiveCommand {
+            command: CommandType::Ignore,
+            args: vec!["src/generated/".to_string(), "*.test.js".to_string()],
+            context: None,
+        };
+        let result = cmd.execute_ignore().unwrap();
+        assert!(result.contains("src/generated/"));
+        assert!(result.contains("*.test.js"));
+    }
+
+    // === help_text / config tests ===
+
+    #[test]
+    fn test_help_text_contains_commands() {
+        let help = InteractiveCommand::help_text();
+        assert!(help.contains("@diffscope review"));
+        assert!(help.contains("@diffscope ignore"));
+        assert!(help.contains("@diffscope explain"));
+        assert!(help.contains("@diffscope generate"));
+        assert!(help.contains("@diffscope help"));
+        assert!(help.contains("@diffscope config"));
+    }
+
+    #[test]
+    fn test_config_info_contains_yaml() {
+        let info = InteractiveCommand::get_config_info();
+        assert!(info.contains(".diffscope.yml"));
+        assert!(info.contains("exclude_patterns"));
+    }
+
+    // === InteractiveProcessor tests ===
+
+    #[test]
+    fn test_processor_new_empty() {
+        let processor = InteractiveProcessor::new();
+        assert!(!processor.should_ignore("any/path"));
+    }
+
+    #[test]
+    fn test_processor_add_and_check_pattern() {
+        let mut processor = InteractiveProcessor::new();
+        processor.add_ignore_pattern("src/generated/");
+        assert!(processor.should_ignore("src/generated/types.rs"));
+        assert!(!processor.should_ignore("src/main.rs"));
+    }
+
+    #[test]
+    fn test_processor_glob_pattern() {
+        let mut processor = InteractiveProcessor::new();
+        processor.add_ignore_pattern("*.test.js");
+        assert!(processor.should_ignore("foo.test.js"));
+        assert!(!processor.should_ignore("foo.js"));
+    }
+
+    #[test]
+    fn test_processor_multiple_patterns() {
+        let mut processor = InteractiveProcessor::new();
+        processor.add_ignore_pattern("vendor/");
+        processor.add_ignore_pattern("*.generated.*");
+        assert!(processor.should_ignore("vendor/lib.js"));
+        assert!(processor.should_ignore("types.generated.ts"));
+        assert!(!processor.should_ignore("src/main.rs"));
+    }
+}
+
 /// Manages per-session ignore patterns from @diffscope ignore commands.
 /// Will be wired into the review pipeline's triage filter.
 #[allow(dead_code)]

src/main.rs

@@ -304,32 +304,44 @@ async fn main() -> Result<()> {
     let _otel_guard: Option<opentelemetry_sdk::trace::TracerProvider> = {
         let otel_enabled = std::env::var("OTEL_EXPORTER_OTLP_ENDPOINT").is_ok();
         if otel_enabled {
-            let exporter = opentelemetry_otlp::SpanExporter::builder()
+            match opentelemetry_otlp::SpanExporter::builder()
                 .with_tonic()
                 .build()
-                .expect("Failed to build OTLP span exporter");
-
-            let tracer_provider = opentelemetry_sdk::trace::TracerProvider::builder()
-                .with_batch_exporter(exporter, opentelemetry_sdk::runtime::Tokio)
-                .with_resource(opentelemetry_sdk::Resource::new(vec![
-                    opentelemetry::KeyValue::new("service.name", "diffscope"),
-                ]))
-                .build();
-
-            opentelemetry::global::set_tracer_provider(tracer_provider.clone());
-
-            let otel_layer =
-                tracing_opentelemetry::layer().with_tracer(tracer_provider.tracer("diffscope"));
-
-            let subscriber = tracing_subscriber::fmt::Subscriber::builder()
-                .with_env_filter(filter)
-                .finish()
-                .with(otel_layer);
-
-            tracing::subscriber::set_global_default(subscriber)
-                .expect("Failed to set tracing subscriber");
-
-            Some(tracer_provider)
+            {
+                Ok(exporter) => {
+                    let tracer_provider = opentelemetry_sdk::trace::TracerProvider::builder()
+                        .with_batch_exporter(exporter, opentelemetry_sdk::runtime::Tokio)
+                        .with_resource(opentelemetry_sdk::Resource::new(vec![
+                            opentelemetry::KeyValue::new("service.name", "diffscope"),
+                        ]))
+                        .build();
+
+                    opentelemetry::global::set_tracer_provider(tracer_provider.clone());
+
+                    let otel_layer = tracing_opentelemetry::layer()
+                        .with_tracer(tracer_provider.tracer("diffscope"));
+
+                    let subscriber = tracing_subscriber::fmt::Subscriber::builder()
+                        .with_env_filter(filter)
+                        .finish()
+                        .with(otel_layer);
+
+                    if let Err(e) = tracing::subscriber::set_global_default(subscriber) {
+                        eprintln!("Warning: failed to set OTEL tracing subscriber: {}", e);
+                        // Already initialized by another thread or test — not fatal
+                    }
+
+                    Some(tracer_provider)
+                }
+                Err(e) => {
+                    eprintln!(
+                        "Warning: OTEL_EXPORTER_OTLP_ENDPOINT set but exporter failed to initialize: {}. Continuing without OpenTelemetry.",
+                        e
+                    );
+                    tracing_subscriber::fmt().with_env_filter(filter).init();
+                    None
+                }
+            }
         } else {
             tracing_subscriber::fmt().with_env_filter(filter).init();
             None

src/plugins/builtin/duplicate_filter.rs

@@ -33,3 +33,102 @@ impl PostProcessor for DuplicateFilter {
         Ok(comments)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::core::comment::{Category, FixEffort, Severity};
+    use std::path::PathBuf;
+
+    fn make_comment(file: &str, line: usize, content: &str) -> Comment {
+        Comment {
+            id: format!("c-{}-{}", file, line),
+            file_path: PathBuf::from(file),
+            line_number: line,
+            content: content.to_string(),
+            rule_id: None,
+            severity: Severity::Warning,
+            category: Category::Style,
+            suggestion: None,
+            confidence: 0.8,
+            code_suggestion: None,
+            tags: vec![],
+            fix_effort: FixEffort::Low,
+            feedback: None,
+        }
+    }
+
+    #[test]
+    fn test_duplicate_filter_id() {
+        let filter = DuplicateFilter::new();
+        assert_eq!(filter.id(), "duplicate_filter");
+    }
+
+    #[tokio::test]
+    async fn test_removes_exact_duplicates() {
+        let filter = DuplicateFilter::new();
+        let comments = vec![
+            make_comment("a.rs", 10, "fix this"),
+            make_comment("a.rs", 10, "fix this"),
+            make_comment("a.rs", 10, "fix this"),
+        ];
+        let result = filter.run(comments, "/repo").await.unwrap();
+        assert_eq!(result.len(), 1);
+    }
+
+    #[tokio::test]
+    async fn test_keeps_different_lines() {
+        let filter = DuplicateFilter::new();
+        let comments = vec![
+            make_comment("a.rs", 10, "fix this"),
+            make_comment("a.rs", 20, "fix this"),
+        ];
+        let result = filter.run(comments, "/repo").await.unwrap();
+        assert_eq!(result.len(), 2);
+    }
+
+    #[tokio::test]
+    async fn test_keeps_different_content() {
+        let filter = DuplicateFilter::new();
+        let comments = vec![
+            make_comment("a.rs", 10, "fix this"),
+            make_comment("a.rs", 10, "fix that"),
+        ];
+        let result = filter.run(comments, "/repo").await.unwrap();
+        assert_eq!(result.len(), 2);
+    }
+
+    #[tokio::test]
+    async fn test_keeps_different_files() {
+        let filter = DuplicateFilter::new();
+        let comments = vec![
+            make_comment("a.rs", 10, "fix this"),
+            make_comment("b.rs", 10, "fix this"),
+        ];
+        let result = filter.run(comments, "/repo").await.unwrap();
+        assert_eq!(result.len(), 2);
+    }
+
+    #[tokio::test]
+    async fn test_empty_input() {
+        let filter = DuplicateFilter::new();
+        let result = filter.run(vec![], "/repo").await.unwrap();
+        assert!(result.is_empty());
+    }
+
+    #[tokio::test]
+    async fn test_preserves_order() {
+        let filter = DuplicateFilter::new();
+        let comments = vec![
+            make_comment("c.rs", 1, "third"),
+            make_comment("a.rs", 1, "first"),
+            make_comment("b.rs", 1, "second"),
+            make_comment("a.rs", 1, "first"), // duplicate
+        ];
+        let result = filter.run(comments, "/repo").await.unwrap();
+        assert_eq!(result.len(), 3);
+        assert_eq!(result[0].content, "third");
+        assert_eq!(result[1].content, "first");
+        assert_eq!(result[2].content, "second");
+    }
+}