From 7188cee3e22cc27c8b0d6a8f39c04445b62755fd Mon Sep 17 00:00:00 2001 From: 44r0n7 <44r0n7+gitea@pm.me> Date: Wed, 31 Dec 2025 22:38:24 -0500 Subject: [PATCH] Add scan depth, exit codes, and fix validation test --- vid-repair-core/src/fix/executor.rs | 18 +++-- vid-repair-core/src/scan/decode.rs | 17 ++++- vid-repair-core/src/scan/mod.rs | 15 +++- vid-repair-core/tests/fix.rs | 80 +++++++++++++++++++ vid-repair/src/main.rs | 114 +++++++++++++++++++++------- vid-repair/tests/cli.rs | 7 +- 6 files changed, 215 insertions(+), 36 deletions(-) create mode 100644 vid-repair-core/tests/fix.rs diff --git a/vid-repair-core/src/fix/executor.rs b/vid-repair-core/src/fix/executor.rs index cb9dd32..295ea84 100644 --- a/vid-repair-core/src/fix/executor.rs +++ b/vid-repair-core/src/fix/executor.rs @@ -3,7 +3,6 @@ use std::process::Command; use anyhow::{Context, Result}; use fs_err as fs; -use tempfile::NamedTempFile; use crate::config::Config; use crate::fix::{FixKind, FixOutcome, FixPlan}; @@ -102,11 +101,19 @@ struct OutputPaths { } fn prepare_output_path(path: &Path, config: &Config) -> Result { + let suffix = path + .extension() + .and_then(|ext| ext.to_str()) + .map(|ext| format!(".{}", ext)) + .unwrap_or_else(|| ".tmp".to_string()); + if config.repair.output_dir.is_empty() { let parent = path .parent() .context("Input file has no parent directory")?; - let temp = NamedTempFile::new_in(parent) + let temp = tempfile::Builder::new() + .suffix(&suffix) + .tempfile_in(parent) .with_context(|| format!("Failed to create temp file in {}", parent.display()))?; let temp_path = temp.path().to_path_buf(); temp.keep()?; @@ -126,9 +133,10 @@ fn prepare_output_path(path: &Path, config: &Config) -> Result { .to_os_string(); let final_path = output_dir.join(file_name); - let temp = NamedTempFile::new_in(&output_dir).with_context(|| { - format!("Failed to create temp file in {}", output_dir.display()) - })?; + let temp = tempfile::Builder::new() + .suffix(&suffix) + .tempfile_in(&output_dir) + .with_context(|| format!("Failed to create temp file in {}", output_dir.display()))?; let temp_path = temp.path().to_path_buf(); temp.keep()?; diff --git a/vid-repair-core/src/scan/decode.rs b/vid-repair-core/src/scan/decode.rs index cc375f9..3e75800 100644 --- a/vid-repair-core/src/scan/decode.rs +++ b/vid-repair-core/src/scan/decode.rs @@ -5,6 +5,7 @@ use std::sync::{Arc, atomic::{AtomicBool, Ordering}}; use anyhow::{Context, Result}; +use crate::config::ScanDepth; use crate::rules::RuleSet; #[derive(Debug)] @@ -13,12 +14,18 @@ pub struct DecodeOutput { pub early_stop: bool, } -pub fn run_decode(path: &Path, ffmpeg_path: &str, ruleset: &RuleSet) -> Result { +pub fn run_decode( + path: &Path, + ffmpeg_path: &str, + ruleset: &RuleSet, + depth: ScanDepth, +) -> Result { let mut child = Command::new(ffmpeg_path) .arg("-v") .arg("error") .arg("-i") .arg(path) + .args(depth_args(depth)) .arg("-f") .arg("null") .arg("-") @@ -58,6 +65,14 @@ pub fn run_decode(path: &Path, ffmpeg_path: &str, ruleset: &RuleSet) -> Result Vec<&'static str> { + match depth { + ScanDepth::Quick => vec!["-t", "5"], + ScanDepth::Standard => vec!["-t", "30"], + ScanDepth::Deep => vec![], + } +} + fn should_stop(line: &str, ruleset: &RuleSet) -> bool { for rule in &ruleset.rules { if !rule.rule.stop_scan { diff --git a/vid-repair-core/src/scan/mod.rs b/vid-repair-core/src/scan/mod.rs index 07a91f7..0bc2ac5 100644 --- a/vid-repair-core/src/scan/mod.rs +++ b/vid-repair-core/src/scan/mod.rs @@ -2,7 +2,7 @@ use std::path::Path; use anyhow::Result; -use crate::config::Config; +use crate::config::{Config, ScanDepth}; use crate::rules::{build_context, RuleMatch, RuleSet}; mod decode; @@ -14,10 +14,19 @@ pub use types::{Issue, ProbeData, ScanOutcome, ScanRequest}; pub fn scan_file(path: &Path, config: &Config, ruleset: &RuleSet) -> Result { let probe = ffprobe::run_ffprobe(path, &config.ffprobe_path)?; - let decode = decode::run_decode(path, &config.ffmpeg_path, ruleset)?; + let mut decode = decode::run_decode(path, &config.ffmpeg_path, ruleset, config.scan.depth)?; let context = build_context(&probe); - let matches = ruleset.match_lines(&decode.lines, &context); + let mut matches = ruleset.match_lines(&decode.lines, &context); + + if config.scan.auto_escalate + && config.scan.depth != ScanDepth::Deep + && !decode.early_stop + && !matches.is_empty() + { + decode = decode::run_decode(path, &config.ffmpeg_path, ruleset, ScanDepth::Deep)?; + matches = ruleset.match_lines(&decode.lines, &context); + } let issues = matches .iter() diff --git a/vid-repair-core/tests/fix.rs b/vid-repair-core/tests/fix.rs new file mode 100644 index 0000000..2bd709b --- /dev/null +++ b/vid-repair-core/tests/fix.rs @@ -0,0 +1,80 @@ +use std::path::PathBuf; +use std::process::Command; + +use tempfile::tempdir; + +use vid_repair_core::config::Config; +use vid_repair_core::fix::{FixAction, FixKind, FixPlan}; +use vid_repair_core::rules::RuleSet; +use vid_repair_core::scan::scan_file; +use vid_repair_core::{fix, ConfigOverrides}; + +fn command_available(cmd: &str) -> bool { + Command::new(cmd) + .arg("-version") + .output() + .map(|out| out.status.success()) + .unwrap_or(false) +} + +fn fixture_dir() -> PathBuf { + let manifest_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + manifest_dir + .parent() + .expect("workspace root") + .join("tests") + .join("fixtures") + .join("generated") +} + +#[test] +fn remux_fix_succeeds_on_clean_fixture() { + if !command_available("ffmpeg") || !command_available("ffprobe") { + eprintln!("ffmpeg/ffprobe not available; skipping fix test"); + return; + } + + let fixture = fixture_dir().join("clean.mp4"); + if !fixture.exists() { + eprintln!("fixture not found: {}; skipping", fixture.display()); + return; + } + + let temp = tempdir().expect("tempdir"); + + let mut config = Config::default(); + let mut overrides = ConfigOverrides::default(); + overrides.output_dir = Some(temp.path().to_string_lossy().to_string()); + config.apply_overrides(&overrides); + + let ruleset_dir = fixture_dir() + .parent() + .unwrap() + .parent() + .unwrap() + .parent() + .unwrap() + .join("rulesets"); + let ruleset = RuleSet::load_from_dir(&ruleset_dir).expect("ruleset load"); + + let plan = FixPlan { + policy: config.repair.policy, + recommended: Some(FixKind::Remux), + actions: vec![FixAction { + kind: FixKind::Remux, + command: Vec::new(), + destructive: true, + }], + blocked_reason: None, + }; + + let outcome = fix::executor::apply_fix(&fixture, &plan, &config, &ruleset) + .expect("apply fix"); + + assert!(outcome.success, "Expected remux to succeed"); + let output_path = outcome.output_path.expect("output path"); + + let scan = scan_file(PathBuf::from(output_path).as_path(), &config, &ruleset) + .expect("scan output"); + assert!(scan.issues.is_empty(), "Output should be clean"); +} diff --git a/vid-repair/src/main.rs b/vid-repair/src/main.rs index 71543f2..bc7b702 100644 --- a/vid-repair/src/main.rs +++ b/vid-repair/src/main.rs @@ -1,4 +1,5 @@ use std::path::PathBuf; +use std::sync::atomic::{AtomicUsize, Ordering}; use anyhow::Result; use clap::{Parser, Subcommand, ValueEnum}; @@ -108,8 +109,12 @@ struct ScanArgs { scan_depth: Option, /// Override recursive scanning - #[arg(long)] + #[arg(long, conflicts_with = "no_recursive")] recursive: bool, + + /// Disable recursive scanning + #[arg(long)] + no_recursive: bool, } #[derive(Parser, Debug)] @@ -131,9 +136,13 @@ struct FixArgs { scan_depth: Option, /// Override recursive scanning - #[arg(long)] + #[arg(long, conflicts_with = "no_recursive")] recursive: bool, + /// Disable recursive scanning + #[arg(long)] + no_recursive: bool, + /// Override repair policy (safe|aggressive) #[arg(long)] policy: Option, @@ -173,7 +182,17 @@ enum ConfigCommand { }, } -fn main() -> Result<()> { +fn main() { + match run() { + Ok(code) => std::process::exit(code), + Err(err) => { + eprintln!("[FATAL] {}", err); + std::process::exit(3); + } + } +} + +fn run() -> Result { let cli = Cli::parse(); let Cli { @@ -194,8 +213,8 @@ fn main() -> Result<()> { }; match command { - Commands::Config(args) => handle_config(args, common.config.clone()), - Commands::Rules(args) => handle_rules(args), + Commands::Config(args) => handle_config(args, common.config.clone()).map(|_| 0), + Commands::Rules(args) => handle_rules(args).map(|_| 0), Commands::Scan(args) => handle_scan(args, &common), Commands::Report(args) => handle_report(args, &common), Commands::Fix(args) => handle_fix(args, &common), @@ -236,7 +255,7 @@ fn handle_rules(args: RulesArgs) -> Result<()> { } } -fn handle_scan(args: ScanArgs, common: &CommonArgs) -> Result<()> { +fn handle_scan(args: ScanArgs, common: &CommonArgs) -> Result { let (mut config, _config_path) = Config::load_or_init(common.config.clone())?; let mut overrides = ConfigOverrides::default(); @@ -251,6 +270,8 @@ fn handle_scan(args: ScanArgs, common: &CommonArgs) -> Result<()> { overrides.scan_depth = args.scan_depth.map(ScanDepth::from); if args.recursive { overrides.scan_recursive = Some(true); + } else if args.no_recursive { + overrides.scan_recursive = Some(false); } overrides.watch = Some(args.watch); @@ -265,21 +286,24 @@ fn handle_scan(args: ScanArgs, common: &CommonArgs) -> Result<()> { } if config.watch.enabled { - return watch_scan(args.paths, &config, &ruleset); + watch_scan(args.paths, &config, &ruleset)?; + return Ok(0); } let files = fs::collect_files(&args.paths, &config)?; if files.is_empty() { println!("No matching files found."); - return Ok(()); + return Ok(0); } - let scans = run_scans(files, &config, &ruleset)?; + let batch = run_scans(files, &config, &ruleset)?; + let scans = batch.scans; + let has_issues = scans.iter().any(|scan| !scan.issues.is_empty()); if config.report.json { let payload = ScanJsonReport { schema_version: SCHEMA_VERSION.to_string(), - scans, + scans: scans.clone(), }; let json = render_json(&payload, config.report.pretty)?; println!("{}", json); @@ -290,16 +314,23 @@ fn handle_scan(args: ScanArgs, common: &CommonArgs) -> Result<()> { println!("{}", render_summary(&scans, None)); } - Ok(()) + if batch.errors > 0 { + eprintln!("[ERROR] {} file(s) failed to scan", batch.errors); + } + + if scans.is_empty() && batch.errors > 0 { + return Ok(3); + } + Ok(if has_issues { 1 } else { 0 }) } -fn handle_report(args: ScanArgs, common: &CommonArgs) -> Result<()> { +fn handle_report(args: ScanArgs, common: &CommonArgs) -> Result { let mut common = common.clone(); common.json = true; handle_scan(args, &common) } -fn handle_fix(args: FixArgs, common: &CommonArgs) -> Result<()> { +fn handle_fix(args: FixArgs, common: &CommonArgs) -> Result { let (mut config, _config_path) = Config::load_or_init(common.config.clone())?; let mut overrides = ConfigOverrides::default(); @@ -312,6 +343,8 @@ fn handle_fix(args: FixArgs, common: &CommonArgs) -> Result<()> { overrides.scan_depth = args.scan_depth.map(ScanDepth::from); if args.recursive { overrides.scan_recursive = Some(true); + } else if args.no_recursive { + overrides.scan_recursive = Some(false); } overrides.policy = args.policy.map(FixPolicy::from); overrides.output_dir = args.output_dir; @@ -329,22 +362,25 @@ fn handle_fix(args: FixArgs, common: &CommonArgs) -> Result<()> { } if config.watch.enabled { - return watch_fix(args.paths, &config, &ruleset, args.dry_run); + watch_fix(args.paths, &config, &ruleset, args.dry_run)?; + return Ok(0); } let files = fs::collect_files(&args.paths, &config)?; if files.is_empty() { println!("No matching files found."); - return Ok(()); + return Ok(0); } - let (scans, fixes) = run_fixes(files, &config, &ruleset, args.dry_run)?; + let (scans, fixes, errors) = run_fixes(files, &config, &ruleset, args.dry_run)?; + let fix_failed = fixes.iter().any(|fix| fix.applied && !fix.success); + let has_issues = scans.iter().any(|scan| !scan.issues.is_empty()); if config.report.json { let payload = FixJsonReport { schema_version: SCHEMA_VERSION.to_string(), - scans, - fixes, + scans: scans.clone(), + fixes: fixes.clone(), }; let json = render_json(&payload, config.report.pretty)?; println!("{}", json); @@ -355,16 +391,33 @@ fn handle_fix(args: FixArgs, common: &CommonArgs) -> Result<()> { println!("{}", render_summary(&scans, Some(&fixes))); } - Ok(()) + if errors > 0 { + eprintln!("[ERROR] {} file(s) failed to scan", errors); + } + + if scans.is_empty() && errors > 0 { + return Ok(3); + } + if fix_failed { + return Ok(2); + } + Ok(if has_issues { 1 } else { 0 }) } -fn run_scans(files: Vec, config: &Config, ruleset: &RuleSet) -> Result> { +struct ScanBatch { + scans: Vec, + errors: usize, +} + +fn run_scans(files: Vec, config: &Config, ruleset: &RuleSet) -> Result { let jobs = if config.performance.jobs == 0 { None } else { Some(config.performance.jobs) }; + let errors = AtomicUsize::new(0); + let scans = if let Some(jobs) = jobs { let pool = ThreadPoolBuilder::new().num_threads(jobs).build()?; pool.install(|| { @@ -374,6 +427,7 @@ fn run_scans(files: Vec, config: &Config, ruleset: &RuleSet) -> Result< Ok(scan) => Some(scan), Err(err) => { eprintln!("[ERROR] {}: {}", path.display(), err); + errors.fetch_add(1, Ordering::SeqCst); None } }) @@ -386,13 +440,17 @@ fn run_scans(files: Vec, config: &Config, ruleset: &RuleSet) -> Result< Ok(scan) => Some(scan), Err(err) => { eprintln!("[ERROR] {}: {}", path.display(), err); + errors.fetch_add(1, Ordering::SeqCst); None } }) .collect::>() }; - Ok(scans) + Ok(ScanBatch { + scans, + errors: errors.load(Ordering::SeqCst), + }) } fn run_fixes( @@ -400,7 +458,7 @@ fn run_fixes( config: &Config, ruleset: &RuleSet, dry_run: bool, -) -> Result<(Vec, Vec)> { +) -> Result<(Vec, Vec, usize)> { let jobs = if config.performance.jobs == 0 { None } else { @@ -420,15 +478,17 @@ fn process_fix_batch( config: &Config, ruleset: &RuleSet, dry_run: bool, -) -> Result<(Vec, Vec)> { +) -> Result<(Vec, Vec, usize)> { let mut scans = Vec::new(); let mut fixes = Vec::new(); + let mut errors = 0usize; for path in files { let scan = match scan_file(&path, config, ruleset) { Ok(scan) => scan, Err(err) => { eprintln!("[ERROR] {}: {}", path.display(), err); + errors += 1; continue; } }; @@ -452,7 +512,7 @@ fn process_fix_batch( fixes.push(outcome); } - Ok((scans, fixes)) + Ok((scans, fixes, errors)) } fn process_fix_batch_parallel( @@ -460,7 +520,8 @@ fn process_fix_batch_parallel( config: &Config, ruleset: &RuleSet, dry_run: bool, -) -> Result<(Vec, Vec)> { +) -> Result<(Vec, Vec, usize)> { + let errors = AtomicUsize::new(0); let results = files .par_iter() .filter_map(|path| { @@ -468,6 +529,7 @@ fn process_fix_batch_parallel( Ok(scan) => scan, Err(err) => { eprintln!("[ERROR] {}: {}", path.display(), err); + errors.fetch_add(1, Ordering::SeqCst); return None; } }; @@ -492,7 +554,7 @@ fn process_fix_batch_parallel( .collect::>(); let (scans, fixes): (Vec<_>, Vec<_>) = results.into_iter().unzip(); - Ok((scans, fixes)) + Ok((scans, fixes, errors.load(Ordering::SeqCst))) } fn watch_scan(paths: Vec, config: &Config, ruleset: &RuleSet) -> Result<()> { diff --git a/vid-repair/tests/cli.rs b/vid-repair/tests/cli.rs index f412973..d9d5d91 100644 --- a/vid-repair/tests/cli.rs +++ b/vid-repair/tests/cli.rs @@ -45,7 +45,12 @@ fn cli_scan_summarizes_fixtures() { .output() .expect("run vid-repair"); - assert!(output.status.success(), "cli scan failed"); + let code = output.status.code().unwrap_or(3); + assert!( + code == 0 || code == 1, + "cli scan returned unexpected status {}", + code + ); let stdout = String::from_utf8_lossy(&output.stdout); assert!(stdout.contains("Summary:"), "missing summary in output");