oxify_model/
linter.rs

1// Workflow Linter - Code quality and best practices checker
2//
3// This module provides a comprehensive linting system for workflows.
4// Unlike validation (which checks correctness), linting checks for:
5// - Code quality and best practices
6// - Performance optimization opportunities
7// - Security concerns
8// - Maintainability issues
9// - Resource usage patterns
10
11use crate::{Edge, LoopType, Node, NodeKind, Workflow};
12use serde::{Deserialize, Serialize};
13use std::collections::{HashMap, HashSet};
14
15/// Severity level for lint findings
16#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
17pub enum LintSeverity {
18    /// Informational suggestion
19    Info,
20    /// Warning about potential issues
21    Warning,
22    /// Error that should be fixed
23    Error,
24}
25
26impl std::fmt::Display for LintSeverity {
27    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
28        match self {
29            LintSeverity::Info => write!(f, "INFO"),
30            LintSeverity::Warning => write!(f, "WARNING"),
31            LintSeverity::Error => write!(f, "ERROR"),
32        }
33    }
34}
35
36/// Category of lint rule
37#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
38pub enum LintCategory {
39    /// Performance-related issues
40    Performance,
41    /// Security concerns
42    Security,
43    /// Maintainability and code quality
44    Maintainability,
45    /// Resource usage optimization
46    ResourceUsage,
47    /// Best practices
48    BestPractice,
49    /// Reliability concerns
50    Reliability,
51}
52
53/// A single lint finding
54#[derive(Debug, Clone, Serialize, Deserialize)]
55pub struct LintFinding {
56    /// Rule that triggered this finding
57    pub rule_id: String,
58    /// Severity level
59    pub severity: LintSeverity,
60    /// Category of the rule
61    pub category: LintCategory,
62    /// Human-readable message
63    pub message: String,
64    /// Optional node ID where the issue was found
65    pub node_id: Option<String>,
66    /// Optional suggestion for fixing the issue
67    pub suggestion: Option<String>,
68    /// Line number (if applicable)
69    pub line: Option<usize>,
70}
71
72impl LintFinding {
73    /// Create a new lint finding
74    pub fn new(
75        rule_id: impl Into<String>,
76        severity: LintSeverity,
77        category: LintCategory,
78        message: impl Into<String>,
79    ) -> Self {
80        Self {
81            rule_id: rule_id.into(),
82            severity,
83            category,
84            message: message.into(),
85            node_id: None,
86            suggestion: None,
87            line: None,
88        }
89    }
90
91    /// Set the node ID where the issue was found
92    pub fn with_node_id(mut self, node_id: impl Into<String>) -> Self {
93        self.node_id = Some(node_id.into());
94        self
95    }
96
97    /// Add a suggestion for fixing the issue
98    pub fn with_suggestion(mut self, suggestion: impl Into<String>) -> Self {
99        self.suggestion = Some(suggestion.into());
100        self
101    }
102
103    /// Set the line number
104    #[allow(dead_code)]
105    pub fn with_line(mut self, line: usize) -> Self {
106        self.line = Some(line);
107        self
108    }
109}
110
111/// Result of linting a workflow
112#[derive(Debug, Clone, Serialize, Deserialize)]
113pub struct LintResult {
114    /// All findings from linting
115    pub findings: Vec<LintFinding>,
116    /// Summary statistics
117    pub stats: LintStats,
118}
119
120/// Statistics about lint results
121#[derive(Debug, Clone, Serialize, Deserialize)]
122pub struct LintStats {
123    /// Total number of findings
124    pub total: usize,
125    /// Number of errors
126    pub errors: usize,
127    /// Number of warnings
128    pub warnings: usize,
129    /// Number of info messages
130    pub info: usize,
131}
132
133impl LintResult {
134    /// Create a new lint result
135    pub fn new(findings: Vec<LintFinding>) -> Self {
136        let errors = findings
137            .iter()
138            .filter(|f| f.severity == LintSeverity::Error)
139            .count();
140        let warnings = findings
141            .iter()
142            .filter(|f| f.severity == LintSeverity::Warning)
143            .count();
144        let info = findings
145            .iter()
146            .filter(|f| f.severity == LintSeverity::Info)
147            .count();
148
149        Self {
150            stats: LintStats {
151                total: findings.len(),
152                errors,
153                warnings,
154                info,
155            },
156            findings,
157        }
158    }
159
160    /// Check if there are any errors
161    pub fn has_errors(&self) -> bool {
162        self.stats.errors > 0
163    }
164
165    /// Check if there are any warnings
166    pub fn has_warnings(&self) -> bool {
167        self.stats.warnings > 0
168    }
169
170    /// Get findings by severity
171    pub fn findings_by_severity(&self, severity: LintSeverity) -> Vec<&LintFinding> {
172        self.findings
173            .iter()
174            .filter(|f| f.severity == severity)
175            .collect()
176    }
177
178    /// Get findings by category
179    pub fn findings_by_category(&self, category: LintCategory) -> Vec<&LintFinding> {
180        self.findings
181            .iter()
182            .filter(|f| f.category == category)
183            .collect()
184    }
185}
186
187/// Configuration for the workflow linter
188#[derive(Debug, Clone)]
189pub struct LinterConfig {
190    /// Maximum recommended retry count
191    pub max_retry_count: u32,
192    /// Maximum recommended timeout in milliseconds
193    pub max_timeout_ms: u64,
194    /// Maximum recommended nodes in a sequence (before suggesting parallelization)
195    pub max_sequential_nodes: usize,
196    /// Maximum nesting depth for conditional nodes
197    pub max_nesting_depth: usize,
198    /// Whether to check for naming conventions
199    pub check_naming: bool,
200    /// Whether to check for security issues
201    pub check_security: bool,
202    /// Whether to check for performance issues
203    pub check_performance: bool,
204}
205
206impl Default for LinterConfig {
207    fn default() -> Self {
208        Self {
209            max_retry_count: 5,
210            max_timeout_ms: 300_000, // 5 minutes
211            max_sequential_nodes: 10,
212            max_nesting_depth: 4,
213            check_naming: true,
214            check_security: true,
215            check_performance: true,
216        }
217    }
218}
219
220/// Workflow linter for checking code quality and best practices
221pub struct WorkflowLinter {
222    config: LinterConfig,
223}
224
225impl WorkflowLinter {
226    /// Create a new linter with default configuration
227    pub fn new() -> Self {
228        Self {
229            config: LinterConfig::default(),
230        }
231    }
232
233    /// Create a linter with custom configuration
234    pub fn with_config(config: LinterConfig) -> Self {
235        Self { config }
236    }
237
238    /// Lint a workflow and return findings
239    pub fn lint(&self, workflow: &Workflow) -> LintResult {
240        let mut findings = Vec::new();
241
242        // Run all lint rules
243        findings.extend(self.check_unused_nodes(workflow));
244        findings.extend(self.check_missing_error_handling(workflow));
245        findings.extend(self.check_excessive_retries(workflow));
246        findings.extend(self.check_missing_timeouts(workflow));
247        findings.extend(self.check_sequential_opportunities(workflow));
248        findings.extend(self.check_deep_nesting(workflow));
249        findings.extend(self.check_naming_conventions(workflow));
250        findings.extend(self.check_hardcoded_values(workflow));
251        findings.extend(self.check_loop_safety(workflow));
252        findings.extend(self.check_dead_end_paths(workflow));
253
254        LintResult::new(findings)
255    }
256
257    /// Check for nodes that are defined but never used (unreachable)
258    fn check_unused_nodes(&self, workflow: &Workflow) -> Vec<LintFinding> {
259        let mut findings = Vec::new();
260        let mut reachable = HashSet::new();
261
262        // Find start nodes
263        for node in &workflow.nodes {
264            if matches!(node.kind, NodeKind::Start) {
265                Self::mark_reachable(&node.id, workflow, &mut reachable);
266            }
267        }
268
269        // Check for unreachable nodes
270        for node in &workflow.nodes {
271            if !reachable.contains(&node.id) && !matches!(node.kind, NodeKind::Start) {
272                findings.push(
273                    LintFinding::new(
274                        "unreachable-node",
275                        LintSeverity::Warning,
276                        LintCategory::Maintainability,
277                        format!("Node '{}' is unreachable from start nodes", node.name),
278                    )
279                    .with_node_id(node.id.to_string())
280                    .with_suggestion("Remove this node or add edges to connect it to the workflow"),
281                );
282            }
283        }
284
285        findings
286    }
287
288    /// Mark all nodes reachable from a given node
289    fn mark_reachable(
290        node_id: &uuid::Uuid,
291        workflow: &Workflow,
292        reachable: &mut HashSet<uuid::Uuid>,
293    ) {
294        if !reachable.insert(*node_id) {
295            return; // Already visited
296        }
297
298        // Find outgoing edges
299        for edge in &workflow.edges {
300            if edge.from == *node_id {
301                Self::mark_reachable(&edge.to, workflow, reachable);
302            }
303        }
304    }
305
306    /// Check for missing error handling (no try-catch around risky operations)
307    fn check_missing_error_handling(&self, workflow: &Workflow) -> Vec<LintFinding> {
308        let mut findings = Vec::new();
309
310        // Build a map of nodes that have error handling
311        let mut protected_nodes = HashSet::new();
312        for node in &workflow.nodes {
313            if let NodeKind::TryCatch(config) = &node.kind {
314                protected_nodes.insert(config.try_expression.as_str());
315            }
316        }
317
318        // Check risky nodes (LLM, Code, Tool, Retriever)
319        for node in &workflow.nodes {
320            let is_risky = matches!(
321                node.kind,
322                NodeKind::LLM(_) | NodeKind::Code(_) | NodeKind::Tool(_) | NodeKind::Retriever(_)
323            );
324
325            if is_risky
326                && !protected_nodes.contains(node.id.to_string().as_str())
327                && node.retry_config.is_none()
328            {
329                findings.push(
330                    LintFinding::new(
331                        "missing-error-handling",
332                        LintSeverity::Info,
333                        LintCategory::Reliability,
334                        format!(
335                            "Node '{}' has no error handling (no try-catch or retry)",
336                            node.name
337                        ),
338                    )
339                    .with_node_id(node.id.to_string())
340                    .with_suggestion(
341                        "Consider adding retry configuration or wrapping in a TryCatch node",
342                    ),
343                );
344            }
345        }
346
347        findings
348    }
349
350    /// Check for excessive retry counts
351    fn check_excessive_retries(&self, workflow: &Workflow) -> Vec<LintFinding> {
352        let mut findings = Vec::new();
353
354        for node in &workflow.nodes {
355            if let Some(retry) = &node.retry_config {
356                if retry.max_retries > self.config.max_retry_count {
357                    findings.push(
358                        LintFinding::new(
359                            "excessive-retries",
360                            LintSeverity::Warning,
361                            LintCategory::Performance,
362                            format!(
363                                "Node '{}' has {} retries (recommended max: {})",
364                                node.name, retry.max_retries, self.config.max_retry_count
365                            ),
366                        )
367                        .with_node_id(node.id.to_string())
368                        .with_suggestion(format!(
369                            "Consider reducing max_retries to {}",
370                            self.config.max_retry_count
371                        )),
372                    );
373                }
374            }
375        }
376
377        findings
378    }
379
380    /// Check for missing timeouts on long-running operations
381    fn check_missing_timeouts(&self, workflow: &Workflow) -> Vec<LintFinding> {
382        let mut findings = Vec::new();
383
384        for node in &workflow.nodes {
385            let needs_timeout = matches!(
386                node.kind,
387                NodeKind::LLM(_)
388                    | NodeKind::Code(_)
389                    | NodeKind::Tool(_)
390                    | NodeKind::Retriever(_)
391                    | NodeKind::Approval(_)
392                    | NodeKind::Form(_)
393            );
394
395            if needs_timeout && node.timeout_config.is_none() {
396                findings.push(
397                    LintFinding::new(
398                        "missing-timeout",
399                        LintSeverity::Info,
400                        LintCategory::Reliability,
401                        format!("Node '{}' has no timeout configuration", node.name),
402                    )
403                    .with_node_id(node.id.to_string())
404                    .with_suggestion("Consider adding a timeout to prevent hanging executions"),
405                );
406            }
407        }
408
409        findings
410    }
411
412    /// Check for opportunities to parallelize sequential operations
413    fn check_sequential_opportunities(&self, workflow: &Workflow) -> Vec<LintFinding> {
414        let mut findings = Vec::new();
415
416        if !self.config.check_performance {
417            return findings;
418        }
419
420        // Build adjacency list
421        let edges_by_source: HashMap<uuid::Uuid, Vec<&Edge>> =
422            workflow.edges.iter().fold(HashMap::new(), |mut acc, edge| {
423                acc.entry(edge.from).or_default().push(edge);
424                acc
425            });
426
427        // Find long sequential chains
428        for node in &workflow.nodes {
429            if matches!(node.kind, NodeKind::Start) {
430                let chain_length = Self::find_longest_chain(&node.id, workflow, &edges_by_source);
431                if chain_length > self.config.max_sequential_nodes {
432                    findings.push(
433                        LintFinding::new(
434                            "long-sequential-chain",
435                            LintSeverity::Info,
436                            LintCategory::Performance,
437                            format!(
438                                "Found sequential chain of {} nodes starting from '{}'",
439                                chain_length, node.name
440                            ),
441                        )
442                        .with_node_id(node.id.to_string())
443                        .with_suggestion(
444                            "Consider using a Parallel node to execute independent operations concurrently",
445                        ),
446                    );
447                }
448            }
449        }
450
451        findings
452    }
453
454    /// Find the longest sequential chain from a node
455    fn find_longest_chain(
456        node_id: &uuid::Uuid,
457        workflow: &Workflow,
458        edges_by_source: &HashMap<uuid::Uuid, Vec<&Edge>>,
459    ) -> usize {
460        let Some(edges) = edges_by_source.get(node_id) else {
461            return 0;
462        };
463
464        if edges.is_empty() {
465            return 0;
466        }
467        if edges.len() > 1 {
468            // Branching - don't count as sequential
469            return 0;
470        }
471
472        // Get the node to check if it's a branching node
473        let target_id = &edges[0].to;
474        let target_node = workflow.nodes.iter().find(|n| n.id == *target_id);
475        if let Some(node) = target_node {
476            if matches!(
477                node.kind,
478                NodeKind::IfElse(_) | NodeKind::Switch(_) | NodeKind::Parallel(_)
479            ) {
480                // Branching node - stop counting
481                return 1;
482            }
483        }
484
485        1 + Self::find_longest_chain(target_id, workflow, edges_by_source)
486    }
487
488    /// Check for deeply nested conditional structures
489    fn check_deep_nesting(&self, workflow: &Workflow) -> Vec<LintFinding> {
490        let mut findings = Vec::new();
491
492        // Build adjacency list
493        let edges_by_source: HashMap<uuid::Uuid, Vec<&Edge>> =
494            workflow.edges.iter().fold(HashMap::new(), |mut acc, edge| {
495                acc.entry(edge.from).or_default().push(edge);
496                acc
497            });
498
499        // Check nesting depth for each conditional node
500        for node in &workflow.nodes {
501            if matches!(node.kind, NodeKind::IfElse(_) | NodeKind::Switch(_)) {
502                let depth = Self::calculate_nesting_depth(&node.id, workflow, &edges_by_source, 0);
503                if depth > self.config.max_nesting_depth {
504                    findings.push(
505                        LintFinding::new(
506                            "deep-nesting",
507                            LintSeverity::Warning,
508                            LintCategory::Maintainability,
509                            format!(
510                                "Node '{}' has nesting depth of {} (max recommended: {})",
511                                node.name, depth, self.config.max_nesting_depth
512                            ),
513                        )
514                        .with_node_id(node.id.to_string())
515                        .with_suggestion(
516                            "Consider refactoring into sub-workflows or flattening the structure",
517                        ),
518                    );
519                }
520            }
521        }
522
523        findings
524    }
525
526    /// Calculate nesting depth from a node
527    fn calculate_nesting_depth(
528        node_id: &uuid::Uuid,
529        workflow: &Workflow,
530        edges_by_source: &HashMap<uuid::Uuid, Vec<&Edge>>,
531        current_depth: usize,
532    ) -> usize {
533        let Some(edges) = edges_by_source.get(node_id) else {
534            return current_depth;
535        };
536
537        if edges.is_empty() {
538            return current_depth;
539        }
540
541        let mut max_depth = current_depth;
542        for edge in edges.iter() {
543            let target_node = workflow.nodes.iter().find(|n| n.id == edge.to);
544            if let Some(node) = target_node {
545                let depth = if matches!(node.kind, NodeKind::IfElse(_) | NodeKind::Switch(_)) {
546                    Self::calculate_nesting_depth(
547                        &node.id,
548                        workflow,
549                        edges_by_source,
550                        current_depth + 1,
551                    )
552                } else {
553                    Self::calculate_nesting_depth(
554                        &node.id,
555                        workflow,
556                        edges_by_source,
557                        current_depth,
558                    )
559                };
560                max_depth = max_depth.max(depth);
561            }
562        }
563
564        max_depth
565    }
566
567    /// Check for naming convention violations
568    fn check_naming_conventions(&self, workflow: &Workflow) -> Vec<LintFinding> {
569        let mut findings = Vec::new();
570
571        if !self.config.check_naming {
572            return findings;
573        }
574
575        for node in &workflow.nodes {
576            // Check for generic names
577            let generic_names = ["node", "step", "task", "action", "untitled"];
578            let name_lower = node.name.to_lowercase();
579            if generic_names.iter().any(|&n| name_lower.contains(n)) && name_lower.len() < 15 {
580                findings.push(
581                    LintFinding::new(
582                        "generic-node-name",
583                        LintSeverity::Info,
584                        LintCategory::Maintainability,
585                        format!("Node has generic name: '{}'", node.name),
586                    )
587                    .with_node_id(node.id.to_string())
588                    .with_suggestion(
589                        "Use a more descriptive name that explains what the node does",
590                    ),
591                );
592            }
593
594            // Check for very short names
595            if node.name.len() < 3 {
596                findings.push(
597                    LintFinding::new(
598                        "short-node-name",
599                        LintSeverity::Info,
600                        LintCategory::Maintainability,
601                        format!("Node has very short name: '{}'", node.name),
602                    )
603                    .with_node_id(node.id.to_string())
604                    .with_suggestion("Use a longer, more descriptive name"),
605                );
606            }
607        }
608
609        findings
610    }
611
612    /// Check for hardcoded values that should be parameterized
613    fn check_hardcoded_values(&self, workflow: &Workflow) -> Vec<LintFinding> {
614        let mut findings = Vec::new();
615
616        if !self.config.check_security {
617            return findings;
618        }
619
620        for node in &workflow.nodes {
621            // Check LLM nodes for hardcoded API keys (common security issue)
622            if let NodeKind::LLM(config) = &node.kind {
623                let prompt = &config.prompt_template;
624                // Look for patterns like "api_key", "secret", "password" in prompts
625                let sensitive_patterns = ["api_key", "secret", "password", "token", "credential"];
626                for pattern in &sensitive_patterns {
627                    if prompt.to_lowercase().contains(pattern) {
628                        findings.push(
629                            LintFinding::new(
630                                "potential-hardcoded-secret",
631                                LintSeverity::Error,
632                                LintCategory::Security,
633                                format!(
634                                    "Node '{}' may contain hardcoded secrets in prompt",
635                                    node.name
636                                ),
637                            )
638                            .with_node_id(node.id.to_string())
639                            .with_suggestion("Use template variables like {{API_KEY}} instead of hardcoding secrets"),
640                        );
641                        break; // Only report once per node
642                    }
643                }
644            }
645        }
646
647        findings
648    }
649
650    /// Check for loop safety issues
651    fn check_loop_safety(&self, workflow: &Workflow) -> Vec<LintFinding> {
652        let mut findings = Vec::new();
653
654        for node in &workflow.nodes {
655            if let NodeKind::Loop(config) = &node.kind {
656                if config.max_iterations > 10000 {
657                    let loop_type_name = match &config.loop_type {
658                        LoopType::ForEach { .. } => "ForEach",
659                        LoopType::While { .. } => "While",
660                        LoopType::Repeat { .. } => "Repeat",
661                    };
662                    findings.push(
663                        LintFinding::new(
664                            "high-loop-limit",
665                            LintSeverity::Warning,
666                            LintCategory::Performance,
667                            format!(
668                                "{} node '{}' has very high iteration limit: {}",
669                                loop_type_name, node.name, config.max_iterations
670                            ),
671                        )
672                        .with_node_id(node.id.to_string())
673                        .with_suggestion(
674                            "Consider reducing max_iterations to prevent resource exhaustion",
675                        ),
676                    );
677                }
678            }
679        }
680
681        findings
682    }
683
684    /// Check for dead-end paths (paths that don't lead to an End node)
685    fn check_dead_end_paths(&self, workflow: &Workflow) -> Vec<LintFinding> {
686        let mut findings = Vec::new();
687
688        // Build reverse adjacency list (incoming edges)
689        let edges_by_target: HashMap<uuid::Uuid, Vec<&Edge>> =
690            workflow.edges.iter().fold(HashMap::new(), |mut acc, edge| {
691                acc.entry(edge.to).or_default().push(edge);
692                acc
693            });
694
695        // Find all end nodes
696        let end_nodes: Vec<&Node> = workflow
697            .nodes
698            .iter()
699            .filter(|n| matches!(n.kind, NodeKind::End))
700            .collect();
701
702        // Mark all nodes that can reach an end node
703        let mut can_reach_end = HashSet::new();
704        for end_node in &end_nodes {
705            Self::mark_can_reach_end(&end_node.id, &edges_by_target, &mut can_reach_end);
706        }
707
708        // Check for nodes that can't reach any end node
709        for node in &workflow.nodes {
710            if !matches!(node.kind, NodeKind::End | NodeKind::Start) {
711                // Check if this node can reach an end node
712                if !can_reach_end.contains(&node.id) {
713                    findings.push(
714                        LintFinding::new(
715                            "dead-end-path",
716                            LintSeverity::Warning,
717                            LintCategory::Reliability,
718                            format!("Node '{}' cannot reach any End node", node.name),
719                        )
720                        .with_node_id(node.id.to_string())
721                        .with_suggestion("Add edges to connect this path to an End node"),
722                    );
723                }
724            }
725        }
726
727        findings
728    }
729
730    /// Mark all nodes that can reach a given node
731    fn mark_can_reach_end(
732        node_id: &uuid::Uuid,
733        edges_by_target: &HashMap<uuid::Uuid, Vec<&Edge>>,
734        can_reach: &mut HashSet<uuid::Uuid>,
735    ) {
736        if !can_reach.insert(*node_id) {
737            return; // Already visited
738        }
739
740        // Find incoming edges
741        if let Some(incoming) = edges_by_target.get(node_id) {
742            for edge in incoming {
743                Self::mark_can_reach_end(&edge.from, edges_by_target, can_reach);
744            }
745        }
746    }
747}
748
749impl Default for WorkflowLinter {
750    fn default() -> Self {
751        Self::new()
752    }
753}
754
755#[cfg(test)]
756mod tests {
757    use super::*;
758    use crate::{
759        LlmConfig, LoopConfig, LoopType, McpConfig, NodeKind, ParallelConfig, ParallelStrategy,
760        RetryConfig,
761    };
762
763    // Helper function to create a simple LLM node for testing
764    fn create_llm_node(name: &str) -> Node {
765        Node::new(
766            name.to_string(),
767            NodeKind::LLM(LlmConfig {
768                provider: "openai".to_string(),
769                model: "gpt-4".to_string(),
770                system_prompt: None,
771                prompt_template: "test".to_string(),
772                temperature: None,
773                max_tokens: None,
774                tools: vec![],
775                images: vec![],
776                extra_params: serde_json::Value::Null,
777            }),
778        )
779    }
780
781    #[test]
782    fn test_linter_creation() {
783        let linter = WorkflowLinter::new();
784        assert_eq!(linter.config.max_retry_count, 5);
785
786        let custom_config = LinterConfig {
787            max_retry_count: 3,
788            ..Default::default()
789        };
790        let custom_linter = WorkflowLinter::with_config(custom_config);
791        assert_eq!(custom_linter.config.max_retry_count, 3);
792    }
793
794    #[test]
795    fn test_lint_result_stats() {
796        let findings = vec![
797            LintFinding::new(
798                "test1",
799                LintSeverity::Error,
800                LintCategory::Security,
801                "Error",
802            ),
803            LintFinding::new(
804                "test2",
805                LintSeverity::Warning,
806                LintCategory::Performance,
807                "Warning",
808            ),
809            LintFinding::new(
810                "test3",
811                LintSeverity::Info,
812                LintCategory::Maintainability,
813                "Info",
814            ),
815        ];
816
817        let result = LintResult::new(findings);
818        assert_eq!(result.stats.total, 3);
819        assert_eq!(result.stats.errors, 1);
820        assert_eq!(result.stats.warnings, 1);
821        assert_eq!(result.stats.info, 1);
822        assert!(result.has_errors());
823        assert!(result.has_warnings());
824    }
825
826    #[test]
827    fn test_unreachable_nodes() {
828        let mut workflow = Workflow::new("test".to_string());
829        let start = Node::new("start".to_string(), NodeKind::Start);
830        let node1 = create_llm_node("node1");
831        let unreachable = create_llm_node("unreachable");
832
833        workflow.nodes = vec![start.clone(), node1.clone(), unreachable];
834        workflow.edges = vec![Edge::new(start.id, node1.id)];
835
836        let linter = WorkflowLinter::new();
837        let result = linter.lint(&workflow);
838
839        let unreachable_findings: Vec<_> = result
840            .findings
841            .iter()
842            .filter(|f| f.rule_id == "unreachable-node")
843            .collect();
844        assert_eq!(unreachable_findings.len(), 1);
845    }
846
847    #[test]
848    fn test_excessive_retries() {
849        let mut workflow = Workflow::new("test".to_string());
850        let node = create_llm_node("node1").with_retry(RetryConfig {
851            max_retries: 10,
852            ..Default::default()
853        });
854
855        workflow.nodes = vec![node];
856
857        let linter = WorkflowLinter::new();
858        let result = linter.lint(&workflow);
859
860        let retry_findings: Vec<_> = result
861            .findings
862            .iter()
863            .filter(|f| f.rule_id == "excessive-retries")
864            .collect();
865        assert_eq!(retry_findings.len(), 1);
866        assert_eq!(retry_findings[0].severity, LintSeverity::Warning);
867    }
868
869    #[test]
870    fn test_missing_timeouts() {
871        let mut workflow = Workflow::new("test".to_string());
872        let node = create_llm_node("node1");
873
874        workflow.nodes = vec![node];
875
876        let linter = WorkflowLinter::new();
877        let result = linter.lint(&workflow);
878
879        let timeout_findings: Vec<_> = result
880            .findings
881            .iter()
882            .filter(|f| f.rule_id == "missing-timeout")
883            .collect();
884        assert_eq!(timeout_findings.len(), 1);
885    }
886
887    #[test]
888    fn test_generic_node_names() {
889        let mut workflow = Workflow::new("test".to_string());
890        let node = create_llm_node("node");
891
892        workflow.nodes = vec![node];
893
894        let linter = WorkflowLinter::new();
895        let result = linter.lint(&workflow);
896
897        let naming_findings: Vec<_> = result
898            .findings
899            .iter()
900            .filter(|f| f.rule_id == "generic-node-name")
901            .collect();
902        assert_eq!(naming_findings.len(), 1);
903    }
904
905    #[test]
906    fn test_hardcoded_secrets() {
907        let mut workflow = Workflow::new("test".to_string());
908        let node = Node::new(
909            "node1".to_string(),
910            NodeKind::LLM(LlmConfig {
911                provider: "openai".to_string(),
912                model: "gpt-4".to_string(),
913                system_prompt: None,
914                prompt_template: "Use api_key: sk-1234567890".to_string(),
915                temperature: None,
916                max_tokens: None,
917                tools: vec![],
918                images: vec![],
919                extra_params: serde_json::Value::Null,
920            }),
921        );
922
923        workflow.nodes = vec![node];
924
925        let linter = WorkflowLinter::new();
926        let result = linter.lint(&workflow);
927
928        let secret_findings: Vec<_> = result
929            .findings
930            .iter()
931            .filter(|f| f.rule_id == "potential-hardcoded-secret")
932            .collect();
933        assert_eq!(secret_findings.len(), 1);
934        assert_eq!(secret_findings[0].severity, LintSeverity::Error);
935    }
936
937    #[test]
938    fn test_high_loop_limits() {
939        let mut workflow = Workflow::new("test".to_string());
940
941        let foreach_node = Node::new(
942            "foreach".to_string(),
943            NodeKind::Loop(LoopConfig {
944                loop_type: LoopType::ForEach {
945                    collection_path: "items".to_string(),
946                    item_variable: "item".to_string(),
947                    index_variable: Some("i".to_string()),
948                    body_expression: "body".to_string(),
949                    parallel: false,
950                    max_concurrency: None,
951                },
952                max_iterations: 15000, // Very high
953            }),
954        );
955
956        workflow.nodes = vec![foreach_node];
957
958        let linter = WorkflowLinter::new();
959        let result = linter.lint(&workflow);
960
961        let loop_findings: Vec<_> = result
962            .findings
963            .iter()
964            .filter(|f| f.rule_id == "high-loop-limit")
965            .collect();
966        assert_eq!(loop_findings.len(), 1);
967    }
968
969    #[test]
970    fn test_missing_error_handling() {
971        let mut workflow = Workflow::new("test".to_string());
972        let node = Node::new(
973            "risky".to_string(),
974            NodeKind::Tool(McpConfig {
975                server_id: "external_api".to_string(),
976                tool_name: "call".to_string(),
977                parameters: serde_json::json!({}),
978            }),
979        );
980
981        workflow.nodes = vec![node];
982
983        let linter = WorkflowLinter::new();
984        let result = linter.lint(&workflow);
985
986        let error_findings: Vec<_> = result
987            .findings
988            .iter()
989            .filter(|f| f.rule_id == "missing-error-handling")
990            .collect();
991        assert_eq!(error_findings.len(), 1);
992    }
993
994    #[test]
995    fn test_findings_by_severity() {
996        let findings = vec![
997            LintFinding::new(
998                "test1",
999                LintSeverity::Error,
1000                LintCategory::Security,
1001                "Error",
1002            ),
1003            LintFinding::new(
1004                "test2",
1005                LintSeverity::Warning,
1006                LintCategory::Performance,
1007                "Warning",
1008            ),
1009        ];
1010
1011        let result = LintResult::new(findings);
1012        let errors = result.findings_by_severity(LintSeverity::Error);
1013        assert_eq!(errors.len(), 1);
1014
1015        let warnings = result.findings_by_severity(LintSeverity::Warning);
1016        assert_eq!(warnings.len(), 1);
1017    }
1018
1019    #[test]
1020    fn test_findings_by_category() {
1021        let findings = vec![
1022            LintFinding::new(
1023                "test1",
1024                LintSeverity::Error,
1025                LintCategory::Security,
1026                "Security issue",
1027            ),
1028            LintFinding::new(
1029                "test2",
1030                LintSeverity::Warning,
1031                LintCategory::Performance,
1032                "Performance issue",
1033            ),
1034        ];
1035
1036        let result = LintResult::new(findings);
1037        let security = result.findings_by_category(LintCategory::Security);
1038        assert_eq!(security.len(), 1);
1039
1040        let performance = result.findings_by_category(LintCategory::Performance);
1041        assert_eq!(performance.len(), 1);
1042    }
1043
1044    #[test]
1045    fn test_long_sequential_chain() {
1046        let mut workflow = Workflow::new("test".to_string());
1047        let start = Node::new("start".to_string(), NodeKind::Start);
1048        let mut prev = start.clone();
1049        workflow.nodes.push(start);
1050
1051        // Create a chain of 12 nodes
1052        for i in 0..12 {
1053            let node = create_llm_node(&format!("node{}", i));
1054            workflow.edges.push(Edge::new(prev.id, node.id));
1055            workflow.nodes.push(node.clone());
1056            prev = node;
1057        }
1058
1059        let linter = WorkflowLinter::new();
1060        let result = linter.lint(&workflow);
1061
1062        let chain_findings: Vec<_> = result
1063            .findings
1064            .iter()
1065            .filter(|f| f.rule_id == "long-sequential-chain")
1066            .collect();
1067        assert!(!chain_findings.is_empty());
1068    }
1069
1070    #[test]
1071    fn test_deep_nesting() {
1072        let mut workflow = Workflow::new("test".to_string());
1073        let start = Node::new("start".to_string(), NodeKind::Start);
1074        workflow.nodes.push(start.clone());
1075
1076        let mut prev = start;
1077        // Create deeply nested if-else nodes
1078        for i in 0..6 {
1079            let then_node = Node::new(format!("then{}", i), NodeKind::End);
1080            let else_node = Node::new(format!("else{}", i), NodeKind::End);
1081
1082            let if_node = Node::new(
1083                format!("if{}", i),
1084                NodeKind::IfElse(crate::Condition {
1085                    expression: "true".to_string(),
1086                    true_branch: then_node.id,
1087                    false_branch: else_node.id,
1088                }),
1089            );
1090            workflow.edges.push(Edge::new(prev.id, if_node.id));
1091
1092            workflow.nodes.push(if_node.clone());
1093            workflow.nodes.push(then_node);
1094            workflow.nodes.push(else_node);
1095            prev = if_node;
1096        }
1097
1098        let linter = WorkflowLinter::new();
1099        let result = linter.lint(&workflow);
1100
1101        let nesting_findings: Vec<_> = result
1102            .findings
1103            .iter()
1104            .filter(|f| f.rule_id == "deep-nesting")
1105            .collect();
1106        assert!(!nesting_findings.is_empty());
1107    }
1108
1109    #[test]
1110    fn test_dead_end_paths() {
1111        let mut workflow = Workflow::new("test".to_string());
1112        let start = Node::new("start".to_string(), NodeKind::Start);
1113        let node1 = create_llm_node("node1");
1114        let dead_end = create_llm_node("dead_end");
1115        let end = Node::new("end".to_string(), NodeKind::End);
1116
1117        workflow.nodes = vec![start.clone(), node1.clone(), dead_end.clone(), end.clone()];
1118        workflow.edges = vec![
1119            Edge::new(start.id, node1.id),
1120            Edge::new(node1.id, end.id),
1121            Edge::new(start.id, dead_end.id),
1122            // Note: dead_end has no outgoing edge to End
1123        ];
1124
1125        let linter = WorkflowLinter::new();
1126        let result = linter.lint(&workflow);
1127
1128        let dead_end_findings: Vec<_> = result
1129            .findings
1130            .iter()
1131            .filter(|f| f.rule_id == "dead-end-path")
1132            .collect();
1133        assert_eq!(dead_end_findings.len(), 1);
1134    }
1135
1136    #[test]
1137    fn test_parallel_node_opportunity() {
1138        let mut workflow = Workflow::new("test".to_string());
1139        let start = Node::new("start".to_string(), NodeKind::Start);
1140        let parallel = Node::new(
1141            "parallel".to_string(),
1142            NodeKind::Parallel(ParallelConfig {
1143                tasks: vec![
1144                    crate::ParallelTask {
1145                        id: "task1".to_string(),
1146                        expression: "node1".to_string(),
1147                        description: None,
1148                    },
1149                    crate::ParallelTask {
1150                        id: "task2".to_string(),
1151                        expression: "node2".to_string(),
1152                        description: None,
1153                    },
1154                ],
1155                strategy: ParallelStrategy::WaitAll,
1156                max_concurrency: None,
1157                timeout_ms: None,
1158            }),
1159        );
1160
1161        workflow.nodes = vec![start.clone(), parallel.clone()];
1162        workflow.edges = vec![Edge::new(start.id, parallel.id)];
1163
1164        let linter = WorkflowLinter::new();
1165        let result = linter.lint(&workflow);
1166
1167        // Parallel nodes should not trigger sequential warnings
1168        let chain_findings: Vec<_> = result
1169            .findings
1170            .iter()
1171            .filter(|f| f.rule_id == "long-sequential-chain")
1172            .collect();
1173        assert!(chain_findings.is_empty());
1174    }
1175}
oxify_model/linter.rs

oxify_model/
linter.rs
