Skip to main content

brainwires_agents/
validation_loop.rs

1//! Validation Loop - Enforces quality checks before agent completion
2//!
3//! Wraps task agent execution to automatically validate work before allowing completion.
4//! If validation fails, forces the agent to fix issues before succeeding.
5//!
6//! When the `tools` feature is enabled, uses brainwires-tool-system validation functions
7//! (check_duplicates, verify_build, check_syntax). Without it, those checks are skipped.
8
9use anyhow::Result;
10use serde::{Deserialize, Serialize};
11use std::path::PathBuf;
12#[cfg(feature = "native")]
13use std::process::Command;
14
15const DEFAULT_VALIDATION_MAX_RETRIES: usize = 3;
16
17/// Validation checks to enforce
18#[derive(Debug, Clone, Serialize, Deserialize)]
19pub enum ValidationCheck {
20    /// Check for duplicate exports/constants
21    NoDuplicates,
22    /// Verify build succeeds
23    BuildSuccess {
24        /// Build system type (e.g. "typescript", "rust").
25        build_type: String,
26    },
27    /// Check syntax validity
28    SyntaxValid,
29    /// Custom validation command
30    CustomCommand {
31        /// Command to run.
32        command: String,
33        /// Arguments for the command.
34        args: Vec<String>,
35    },
36}
37
38/// Result of validation checks
39#[derive(Debug, Clone)]
40pub struct ValidationResult {
41    /// Whether all checks passed.
42    pub passed: bool,
43    /// Issues found during validation.
44    pub issues: Vec<ValidationIssue>,
45}
46
47/// A single issue found during validation
48#[derive(Debug, Clone)]
49pub struct ValidationIssue {
50    /// Name of the check that found this issue.
51    pub check: String,
52    /// Severity of the issue.
53    pub severity: ValidationSeverity,
54    /// Human-readable description.
55    pub message: String,
56    /// File where the issue was found.
57    pub file: Option<String>,
58    /// Line number of the issue.
59    pub line: Option<usize>,
60}
61
62/// Severity level for a validation issue
63#[derive(Debug, Clone, Copy, PartialEq, Eq)]
64pub enum ValidationSeverity {
65    /// Blocks completion.
66    Error,
67    /// Non-blocking but notable.
68    Warning,
69    /// Informational only (does not block completion)
70    Info,
71}
72
73/// Configuration for validation loop
74#[derive(Debug, Clone)]
75pub struct ValidationConfig {
76    /// Checks to run
77    pub checks: Vec<ValidationCheck>,
78    /// Working directory for validation
79    pub working_directory: String,
80    /// Maximum validation retry attempts
81    pub max_retries: usize,
82    /// Whether to run validation (can disable for testing)
83    pub enabled: bool,
84    /// Specific files to validate (from working set). If empty, falls back to git diff.
85    pub working_set_files: Vec<String>,
86}
87
88impl Default for ValidationConfig {
89    fn default() -> Self {
90        Self {
91            checks: vec![ValidationCheck::NoDuplicates, ValidationCheck::SyntaxValid],
92            working_directory: ".".to_string(),
93            max_retries: DEFAULT_VALIDATION_MAX_RETRIES,
94            enabled: true,
95            working_set_files: Vec::new(),
96        }
97    }
98}
99
100impl ValidationConfig {
101    /// Create config with build validation
102    pub fn with_build(mut self, build_type: impl Into<String>) -> Self {
103        self.checks.push(ValidationCheck::BuildSuccess {
104            build_type: build_type.into(),
105        });
106        self
107    }
108
109    /// Disable validation (for testing)
110    pub fn disabled() -> Self {
111        Self {
112            enabled: false,
113            ..Default::default()
114        }
115    }
116
117    /// Set the working set files to validate (from agent's working set)
118    pub fn with_working_set_files(mut self, files: Vec<String>) -> Self {
119        self.working_set_files = files;
120        self
121    }
122}
123
124/// Run validation checks on changed files
125#[tracing::instrument(name = "agent.validate", skip(config), fields(working_dir = %config.working_directory))]
126pub async fn run_validation(config: &ValidationConfig) -> Result<ValidationResult> {
127    if !config.enabled {
128        return Ok(ValidationResult {
129            passed: true,
130            issues: vec![],
131        });
132    }
133
134    let mut issues = Vec::new();
135
136    // Get list of modified files - prefer working set, fallback to git
137    let changed_files = if !config.working_set_files.is_empty() {
138        tracing::debug!(
139            "Using working set files for validation: {:?}",
140            config.working_set_files
141        );
142        config.working_set_files.clone()
143    } else {
144        tracing::debug!("No working set provided, falling back to git diff");
145        get_modified_files(&config.working_directory)?
146    };
147    tracing::debug!("Validating {} changed files", changed_files.len());
148
149    // CRITICAL: Verify that all files in the working set actually exist on disk
150    // This catches Bug #5 where agents report success without creating files
151    for file in &changed_files {
152        let file_path = PathBuf::from(&config.working_directory).join(file);
153        if !file_path.exists() {
154            issues.push(ValidationIssue {
155                check: "file_existence".to_string(),
156                severity: ValidationSeverity::Error,
157                message: format!(
158                    "File '{}' is in working set but does not exist on disk. Agent must create file before completing.",
159                    file
160                ),
161                file: Some(file.clone()),
162                line: None,
163            });
164            tracing::error!(
165                "Validation failed: File {} does not exist but is in working set",
166                file
167            );
168        }
169    }
170
171    for check in &config.checks {
172        match check {
173            ValidationCheck::NoDuplicates => {
174                run_duplicates_check(&changed_files, &mut issues).await;
175            }
176
177            ValidationCheck::SyntaxValid => {
178                run_syntax_check(&changed_files, &mut issues).await;
179            }
180
181            ValidationCheck::BuildSuccess { build_type } => {
182                run_build_check(&config.working_directory, build_type, &mut issues).await;
183            }
184
185            ValidationCheck::CustomCommand { command, args } => {
186                #[cfg(feature = "native")]
187                {
188                    match Command::new(command)
189                        .args(args)
190                        .current_dir(&config.working_directory)
191                        .output()
192                    {
193                        Ok(output) => {
194                            if !output.status.success() {
195                                let stderr = String::from_utf8_lossy(&output.stderr);
196                                issues.push(ValidationIssue {
197                                    check: "custom_command".to_string(),
198                                    severity: ValidationSeverity::Error,
199                                    message: format!("Command '{}' failed: {}", command, stderr),
200                                    file: None,
201                                    line: None,
202                                });
203                            }
204                        }
205                        Err(e) => {
206                            issues.push(ValidationIssue {
207                                check: "custom_command".to_string(),
208                                severity: ValidationSeverity::Error,
209                                message: format!("Failed to run command '{}': {}", command, e),
210                                file: None,
211                                line: None,
212                            });
213                        }
214                    }
215                }
216                #[cfg(not(feature = "native"))]
217                {
218                    let _ = (command, args);
219                    issues.push(ValidationIssue {
220                        check: "custom_command".to_string(),
221                        severity: ValidationSeverity::Warning,
222                        message: "Custom command validation not available in WASM".to_string(),
223                        file: None,
224                        line: None,
225                    });
226                }
227            }
228        }
229    }
230
231    Ok(ValidationResult {
232        passed: issues.is_empty(),
233        issues,
234    })
235}
236
237// ── Validation tool dispatch (feature-gated) ─────────────────────────────────
238
239async fn run_duplicates_check(changed_files: &[String], issues: &mut Vec<ValidationIssue>) {
240    use brainwires_tool_system::validation::check_duplicates;
241
242    for file in changed_files {
243        if !is_source_file(file) {
244            continue;
245        }
246
247        match check_duplicates(file).await {
248            Ok(result) => {
249                if let Ok(result_value) = serde_json::from_str::<serde_json::Value>(&result.content)
250                    && result_value["has_duplicates"].as_bool().unwrap_or(false)
251                    && let Some(duplicates) = result_value["duplicates"].as_array()
252                {
253                    for dup in duplicates {
254                        issues.push(ValidationIssue {
255                            check: "duplicate_check".to_string(),
256                            severity: ValidationSeverity::Error,
257                            message: format!(
258                                "Duplicate export '{}' found at lines {} and {}",
259                                dup["name"].as_str().unwrap_or("unknown"),
260                                dup["first_line"].as_u64().unwrap_or(0),
261                                dup["duplicate_line"].as_u64().unwrap_or(0)
262                            ),
263                            file: Some(file.clone()),
264                            line: dup["duplicate_line"].as_u64().map(|n| n as usize),
265                        });
266                    }
267                }
268            }
269            Err(e) => {
270                tracing::warn!("Failed to check duplicates in {}: {}", file, e);
271            }
272        }
273    }
274}
275
276async fn run_syntax_check(changed_files: &[String], issues: &mut Vec<ValidationIssue>) {
277    use brainwires_tool_system::validation::check_syntax;
278
279    for file in changed_files {
280        if !is_source_file(file) {
281            continue;
282        }
283
284        match check_syntax(file).await {
285            Ok(result) => {
286                if let Ok(result_value) = serde_json::from_str::<serde_json::Value>(&result.content)
287                    && !result_value["valid_syntax"].as_bool().unwrap_or(true)
288                    && let Some(errors) = result_value["errors"].as_array()
289                {
290                    for error in errors {
291                        issues.push(ValidationIssue {
292                            check: "syntax_check".to_string(),
293                            severity: ValidationSeverity::Error,
294                            message: error["message"]
295                                .as_str()
296                                .unwrap_or("Unknown syntax error")
297                                .to_string(),
298                            file: Some(file.clone()),
299                            line: None,
300                        });
301                    }
302                }
303            }
304            Err(e) => {
305                tracing::warn!("Failed to check syntax in {}: {}", file, e);
306            }
307        }
308    }
309}
310
311async fn run_build_check(
312    working_directory: &str,
313    build_type: &str,
314    issues: &mut Vec<ValidationIssue>,
315) {
316    use brainwires_tool_system::validation::verify_build;
317
318    match verify_build(working_directory, build_type).await {
319        Ok(result) => {
320            if let Ok(result_value) = serde_json::from_str::<serde_json::Value>(&result.content)
321                && !result_value["success"].as_bool().unwrap_or(false)
322            {
323                let error_count = result_value["error_count"].as_u64().unwrap_or(0);
324
325                if let Some(errors) = result_value["errors"].as_array() {
326                    for error in errors.iter().take(5) {
327                        issues.push(ValidationIssue {
328                            check: "build_check".to_string(),
329                            severity: ValidationSeverity::Error,
330                            message: error["message"]
331                                .as_str()
332                                .or_else(|| error["line"].as_str())
333                                .unwrap_or("Build error")
334                                .to_string(),
335                            file: error["location"].as_str().map(|s| s.to_string()),
336                            line: None,
337                        });
338                    }
339                }
340
341                if error_count > 5 {
342                    issues.push(ValidationIssue {
343                        check: "build_check".to_string(),
344                        severity: ValidationSeverity::Error,
345                        message: format!("... and {} more build errors", error_count - 5),
346                        file: None,
347                        line: None,
348                    });
349                }
350            }
351        }
352        Err(e) => {
353            issues.push(ValidationIssue {
354                check: "build_check".to_string(),
355                severity: ValidationSeverity::Error,
356                message: format!("Build validation failed: {}", e),
357                file: None,
358                line: None,
359            });
360        }
361    }
362}
363
364// ── Helpers ──────────────────────────────────────────────────────────────────
365
366/// Format validation result as feedback for agent
367pub fn format_validation_feedback(result: &ValidationResult) -> String {
368    if result.passed {
369        return "All validation checks passed!".to_string();
370    }
371
372    let mut feedback = String::from("VALIDATION FAILED - You must fix these issues:\n\n");
373
374    for (idx, issue) in result.issues.iter().enumerate() {
375        feedback.push_str(&format!("{}. [{}] ", idx + 1, issue.check));
376
377        if let Some(file) = &issue.file {
378            feedback.push_str(&format!("{}:", file));
379            if let Some(line) = issue.line {
380                feedback.push_str(&format!("{}:", line));
381            }
382            feedback.push(' ');
383        }
384
385        feedback.push_str(&issue.message);
386        feedback.push('\n');
387    }
388
389    feedback.push('\n');
390    feedback
391        .push_str("IMPORTANT: You MUST fix ALL of these issues before the task can complete.\n");
392    feedback.push_str("After fixing, verify your changes by reading the files back.\n");
393
394    feedback
395}
396
397/// Get list of files modified in working directory (git-aware)
398#[cfg(feature = "native")]
399fn get_modified_files(working_directory: &str) -> Result<Vec<String>> {
400    if let Ok(output) = Command::new("git")
401        .args(["diff", "--name-only", "HEAD"])
402        .current_dir(working_directory)
403        .output()
404        && output.status.success()
405    {
406        let files: Vec<String> = String::from_utf8_lossy(&output.stdout)
407            .lines()
408            .map(|s| s.to_string())
409            .filter(|s| !s.is_empty())
410            .collect();
411
412        if !files.is_empty() {
413            return Ok(files);
414        }
415    }
416
417    // Fallback: check for recently modified files
418    let path = PathBuf::from(working_directory);
419    let mut files = Vec::new();
420
421    if let Ok(entries) = std::fs::read_dir(&path) {
422        for entry in entries.flatten() {
423            if let Ok(metadata) = entry.metadata()
424                && metadata.is_file()
425                && let Some(file_name) = entry.file_name().to_str()
426            {
427                files.push(file_name.to_string());
428            }
429        }
430    }
431
432    Ok(files)
433}
434
435/// Get list of files modified in working directory (WASM fallback)
436#[cfg(not(feature = "native"))]
437fn get_modified_files(_working_directory: &str) -> Result<Vec<String>> {
438    Ok(Vec::new())
439}
440
441/// Check if file is a source code file worth validating
442#[allow(dead_code)]
443fn is_source_file(path: &str) -> bool {
444    let path_lower = path.to_lowercase();
445
446    path_lower.ends_with(".rs")
447        || path_lower.ends_with(".ts")
448        || path_lower.ends_with(".tsx")
449        || path_lower.ends_with(".js")
450        || path_lower.ends_with(".jsx")
451        || path_lower.ends_with(".py")
452        || path_lower.ends_with(".java")
453        || path_lower.ends_with(".cpp")
454        || path_lower.ends_with(".c")
455        || path_lower.ends_with(".go")
456        || path_lower.ends_with(".rb")
457}
458
459#[cfg(test)]
460mod tests {
461    use super::*;
462
463    #[test]
464    fn test_is_source_file() {
465        assert!(is_source_file("src/main.rs"));
466        assert!(is_source_file("app.ts"));
467        assert!(is_source_file("Component.tsx"));
468        assert!(!is_source_file("README.md"));
469        assert!(!is_source_file("package.json"));
470    }
471
472    #[test]
473    fn test_format_validation_feedback() {
474        let result = ValidationResult {
475            passed: false,
476            issues: vec![ValidationIssue {
477                check: "duplicate_check".to_string(),
478                severity: ValidationSeverity::Error,
479                message: "Duplicate export 'FOO'".to_string(),
480                file: Some("src/test.ts".to_string()),
481                line: Some(42),
482            }],
483        };
484
485        let feedback = format_validation_feedback(&result);
486        assert!(feedback.contains("VALIDATION FAILED"));
487        assert!(feedback.contains("src/test.ts:42"));
488        assert!(feedback.contains("Duplicate export 'FOO'"));
489    }
490}