diff --git a/src/analyze/cli.rs b/src/analyze/cli.rs index 733112f..228870c 100644 --- a/src/analyze/cli.rs +++ b/src/analyze/cli.rs @@ -42,6 +42,8 @@ struct AnalyzeCommandArgs { command: Vec, } +// `scope analyze` is interactive by design — users review and approve each fix, +// so auto-approving (yolo) isn't implemented here. pub async fn analyze_root(found_config: &FoundConfig, args: &AnalyzeArgs) -> Result { match &args.command { AnalyzeCommands::Logs(args) => analyze_logs(found_config, args).await, @@ -56,6 +58,7 @@ async fn analyze_logs(found_config: &FoundConfig, args: &AnalyzeLogsArgs) -> Res &found_config.known_error, &found_config.working_dir, read_from_stdin().await?, + false, ) .await? } @@ -64,6 +67,7 @@ async fn analyze_logs(found_config: &FoundConfig, args: &AnalyzeLogsArgs) -> Res &found_config.known_error, &found_config.working_dir, read_from_file(file_path).await?, + false, ) .await? } @@ -91,6 +95,7 @@ async fn analyze_command(found_config: &FoundConfig, args: &AnalyzeCommandArgs) &found_config.known_error, &found_config.working_dir, read_from_command(&exec_runner, capture_opts).await?, + false, ) .await?; diff --git a/src/bin/scope-intercept.rs b/src/bin/scope-intercept.rs index 2fb16fb..db4663c 100644 --- a/src/bin/scope-intercept.rs +++ b/src/bin/scope-intercept.rs @@ -1,9 +1,13 @@ use clap::Parser; use dev_scope::prelude::*; +use dev_scope::shared::analyze; +use dev_scope::shared::analyze::AnalyzeStatus; use human_panic::setup_panic; use std::env; +use std::io::Cursor; use std::sync::Arc; -use tracing::{Level, debug, enabled, error, info, warn}; +use tokio::io::BufReader; +use tracing::{Level, enabled, error, info, warn}; /// A wrapper CLI that can be used to capture output from a program, check if there are known errors /// and let the user know. @@ -25,6 +29,10 @@ struct Cli { #[clap(flatten)] config_options: ConfigOptions, + /// Automatically approve all fix prompts without asking + #[arg(long, short = 'y', default_value = "false")] + yolo: bool, + /// Command to execute withing scope-intercept. #[arg(required = true)] utility: String, @@ -62,6 +70,7 @@ async fn main() -> anyhow::Result<()> { } async fn run_command(opts: Cli) -> anyhow::Result { + let yolo = opts.yolo; let mut command = vec![opts.utility]; command.extend(opts.args); let current_dir = std::env::current_dir()?; @@ -90,20 +99,49 @@ async fn run_command(opts: Cli) -> anyhow::Result { FoundConfig::empty(env::current_dir().unwrap()) }); - let command_output = capture.generate_output(); + let analyze_status = analyze::process_lines( + &found_config.known_error, + &found_config.working_dir, + BufReader::new(Cursor::new(capture.generate_user_output())), + yolo, + ) + .await?; - for known_error in found_config.known_error.values() { - debug!("Checking known error {}", known_error.name()); - if known_error.regex.is_match(&command_output) { - info!(target: "always", "Known error '{}' found", known_error.name()); - info!(target: "always", "\t==> {}", known_error.help_text); - } - } + analyze::report_result(&analyze_status); + + let (capture, exit_code) = + if matches!(analyze_status, AnalyzeStatus::KnownErrorFoundFixSucceeded) { + info!(target: "always", "Fix succeeded, retrying command"); + let retry_capture = OutputCapture::capture_output(CaptureOpts { + working_dir: ¤t_dir, + args: &command, + output_dest: OutputDisplay::Visible, + path: &path, + env_vars: Default::default(), + }) + .await?; + + let retry_exit_code = retry_capture.exit_code.unwrap_or(-1); + if accepted_exit_codes.contains(&retry_exit_code) { + return Ok(retry_exit_code); + } - if found_config.report_upload.is_empty() { - return Ok(exit_code); + (retry_capture, retry_exit_code) + } else { + (capture, exit_code) + }; + + if !found_config.report_upload.is_empty() { + offer_bug_report(&found_config, &command, &capture).await?; } + Ok(exit_code) +} +async fn offer_bug_report( + found_config: &FoundConfig, + command: &[String], + capture: &OutputCapture, +) -> anyhow::Result<()> { let ans = inquire::Confirm::new("Do you want to upload a bug report?") .with_default(false) .with_help_message( @@ -115,13 +153,13 @@ async fn run_command(opts: Cli) -> anyhow::Result { let entrypoint = command.join(" "); let exec_runner = Arc::new(DefaultExecutionProvider::default()); - let builder = DefaultUnstructuredReportBuilder::new(&entrypoint, &capture); + let builder = DefaultUnstructuredReportBuilder::new(&entrypoint, capture); for location in found_config.report_upload.values() { let mut builder = builder.clone(); builder .run_and_append_additional_data( - &found_config, + found_config, exec_runner.clone(), &location.additional_data, ) @@ -140,5 +178,5 @@ async fn run_command(opts: Cli) -> anyhow::Result { } } } - Ok(exit_code) + Ok(()) } diff --git a/src/doctor/check.rs b/src/doctor/check.rs index fb47480..94b58b3 100644 --- a/src/doctor/check.rs +++ b/src/doctor/check.rs @@ -165,6 +165,7 @@ pub struct DefaultDoctorActionRun { pub working_dir: PathBuf, pub file_cache: Arc, pub run_fix: bool, + pub yolo: bool, #[educe(Debug(ignore))] pub exec_runner: Arc, #[educe(Debug(ignore))] @@ -672,7 +673,7 @@ impl DefaultDoctorActionRun { .collect::>(); let buffer = BufReader::new(StringVecReader::new(&lines)); - analyze::process_lines(&self.known_errors, &self.working_dir, buffer).await + analyze::process_lines(&self.known_errors, &self.working_dir, buffer, self.yolo).await } } @@ -920,6 +921,7 @@ pub(crate) mod tests { working_dir: path, file_cache, run_fix: true, + yolo: false, exec_runner: Arc::new(exec_runner), glob_walker: Arc::new(glob_walker), known_errors: BTreeMap::new(), @@ -1566,4 +1568,132 @@ pub(crate) mod tests { assert_eq!(home_dir.join("foo.txt").display().to_string(), actual); } } + + mod analyze_known_errors_spec { + use super::*; + use crate::models::prelude::{ModelMetadata, ModelMetadataAnnotations}; + use crate::shared::analyze::AnalyzeStatus; + use regex::Regex; + + fn make_known_error(pattern: &str, with_fix: bool) -> KnownError { + let fix = if with_fix { + Some(DoctorFix { + command: Some(DoctorCommands::from(vec!["true"])), + help_text: None, + help_url: None, + prompt: None, + }) + } else { + None + }; + + KnownError { + full_name: "ScopeKnownError/test-error".to_string(), + metadata: ModelMetadata { + name: "test-error".to_string(), + description: "a test known error".to_string(), + annotations: ModelMetadataAnnotations { + file_path: None, + file_dir: None, + working_dir: Some("/tmp".to_string()), + bin_path: None, + extra: BTreeMap::new(), + }, + labels: BTreeMap::new(), + }, + pattern: pattern.to_string(), + regex: Regex::new(pattern).unwrap(), + help_text: "test help".to_string(), + fix, + } + } + + fn task_report(output: &str) -> ActionTaskReport { + ActionTaskReportBuilder::default() + .output(Some(output.to_string())) + .exit_code(Some(1)) + .command("test".to_string()) + .build() + .unwrap() + } + + #[tokio::test] + async fn yolo_auto_approves_known_error_fix() -> Result<()> { + let action = build_run_fail_fix_succeed_action(); + let exec_runner = MockExecutionProvider::new(); + let glob_walker = MockGlobWalker::new(); + let mut run = setup_test(vec![action], exec_runner, glob_walker); + + run.working_dir = PathBuf::from("/tmp"); + run.yolo = true; + run.known_errors.insert( + "ScopeKnownError/test-error".to_string(), + make_known_error("error-pattern", true), + ); + + let report = task_report("error-pattern found in output"); + let status = run.analyze_known_errors(&[report]).await?; + + assert!(matches!(status, AnalyzeStatus::KnownErrorFoundFixSucceeded)); + Ok(()) + } + + #[tokio::test] + async fn no_tty_without_yolo_returns_user_denied() -> Result<()> { + let action = build_run_fail_fix_succeed_action(); + let exec_runner = MockExecutionProvider::new(); + let glob_walker = MockGlobWalker::new(); + let mut run = setup_test(vec![action], exec_runner, glob_walker); + + run.yolo = false; + run.known_errors.insert( + "ScopeKnownError/test-error".to_string(), + make_known_error("error-pattern", true), + ); + + let report = task_report("error-pattern found in output"); + let status = run.analyze_known_errors(&[report]).await?; + + assert!(matches!(status, AnalyzeStatus::KnownErrorFoundUserDenied)); + Ok(()) + } + + #[tokio::test] + async fn no_pattern_match_returns_no_known_errors() -> Result<()> { + let action = build_run_fail_fix_succeed_action(); + let exec_runner = MockExecutionProvider::new(); + let glob_walker = MockGlobWalker::new(); + let mut run = setup_test(vec![action], exec_runner, glob_walker); + + run.known_errors.insert( + "ScopeKnownError/test-error".to_string(), + make_known_error("error-pattern", true), + ); + + let report = task_report("totally unrelated output"); + let status = run.analyze_known_errors(&[report]).await?; + + assert!(matches!(status, AnalyzeStatus::NoKnownErrorsFound)); + Ok(()) + } + + #[tokio::test] + async fn pattern_match_without_fix_returns_no_fix_found() -> Result<()> { + let action = build_run_fail_fix_succeed_action(); + let exec_runner = MockExecutionProvider::new(); + let glob_walker = MockGlobWalker::new(); + let mut run = setup_test(vec![action], exec_runner, glob_walker); + + run.known_errors.insert( + "ScopeKnownError/test-error".to_string(), + make_known_error("error-pattern", false), + ); + + let report = task_report("error-pattern found in output"); + let status = run.analyze_known_errors(&[report]).await?; + + assert!(matches!(status, AnalyzeStatus::KnownErrorFoundNoFixFound)); + Ok(()) + } + } } diff --git a/src/doctor/commands/run.rs b/src/doctor/commands/run.rs index 7fc75fd..dc2d99b 100644 --- a/src/doctor/commands/run.rs +++ b/src/doctor/commands/run.rs @@ -199,6 +199,7 @@ fn transform_inputs(found_config: &FoundConfig, args: &DoctorRunArgs) -> RunTran working_dir: found_config.working_dir.clone(), file_cache: file_cache.clone(), run_fix: args.fix.unwrap_or(true), + yolo: args.yolo, exec_runner: exec_runner.clone(), glob_walker: glob_walker.clone(), known_errors: found_config.known_error.clone(), diff --git a/src/shared/analyze/mod.rs b/src/shared/analyze/mod.rs index 86a978b..04f2d70 100644 --- a/src/shared/analyze/mod.rs +++ b/src/shared/analyze/mod.rs @@ -17,6 +17,7 @@ pub async fn process_lines( known_errors: &BTreeMap, working_dir: &PathBuf, input: T, + yolo: bool, ) -> Result where T: AsyncRead, @@ -43,7 +44,7 @@ where tracing_indicatif::suspend_tracing_indicatif(|| { let exec_path = ke.metadata.exec_path(); - prompt_and_run_fix(working_dir, exec_path, fix) + prompt_and_run_fix(working_dir, exec_path, fix, yolo) }) .await? } @@ -73,6 +74,7 @@ async fn prompt_and_run_fix( working_dir: &PathBuf, exec_path: String, fix: &DoctorFix, + yolo: bool, ) -> Result { let fix_prompt = &fix.prompt.as_ref(); let prompt_text = fix_prompt @@ -80,51 +82,59 @@ async fn prompt_and_run_fix( .unwrap_or("Would you like to run it?".to_string()); let extra_context = &fix_prompt.map(|p| p.extra_context.clone()).flatten(); - let prompt = { - let base_prompt = inquire::Confirm::new(&prompt_text).with_default(false); - match extra_context { - Some(help_text) => base_prompt.with_help_message(help_text), - None => base_prompt, + let user_accepted = if yolo { + info!(target: "always", "{} Yes (auto-approved)", prompt_text); + if let Some(ctx) = extra_context { + info!(target: "always", "[{}]", ctx); } + true + } else { + let prompt = { + let base_prompt = inquire::Confirm::new(&prompt_text).with_default(false); + match extra_context { + Some(help_text) => base_prompt.with_help_message(help_text), + None => base_prompt, + } + }; + + match prompt.prompt() { + Ok(accepted) => Ok::(accepted), + Err(InquireError::NotTTY) => { + warn!(target: "user", "Prompting user for fix, but input device is not a TTY. Skipping fix."); + Ok(false) + } + Err(e) => { + error!(target: "user", "Error prompting user for fix: {}", e); + Err(e.into()) + } + }? }; - match prompt.prompt() { - Ok(user_accepted) => { - if user_accepted { - // failure indicates an issue with us actually executing it, - // not the success/failure of the command itself. - let outputs = run_fix(working_dir, &exec_path, fix).await?; - let max_exit_code = outputs - .iter() - .map(|c| c.exit_code.unwrap_or(-1)) - .max() - .unwrap(); - - match max_exit_code { - 0 => Ok(AnalyzeStatus::KnownErrorFoundFixSucceeded), - _ => { - if let Some(help_text) = &fix.help_text { - error!(target: "user", "Fix Help: {}", help_text); - } - if let Some(help_url) = &fix.help_url { - error!(target: "user", "For more help, please visit {}", help_url); - } - - Ok(AnalyzeStatus::KnownErrorFoundFixFailed) - } + if user_accepted { + // failure indicates an issue with us actually executing it, + // not the success/failure of the command itself. + let outputs = run_fix(working_dir, &exec_path, fix).await?; + let max_exit_code = outputs + .iter() + .map(|c| c.exit_code.unwrap_or(-1)) + .max() + .unwrap(); + + match max_exit_code { + 0 => Ok(AnalyzeStatus::KnownErrorFoundFixSucceeded), + _ => { + if let Some(help_text) = &fix.help_text { + error!(target: "user", "Fix Help: {}", help_text); + } + if let Some(help_url) = &fix.help_url { + error!(target: "user", "For more help, please visit {}", help_url); } - } else { - Ok(AnalyzeStatus::KnownErrorFoundUserDenied) + + Ok(AnalyzeStatus::KnownErrorFoundFixFailed) } } - Err(InquireError::NotTTY) => { - warn!(target: "user", "Prompting user for fix, but input device is not a TTY. Skipping fix."); - Ok(AnalyzeStatus::KnownErrorFoundUserDenied) - } - Err(e) => { - error!(target: "user", "Error prompting user for fix: {}", e); - Err(e.into()) - } + } else { + Ok(AnalyzeStatus::KnownErrorFoundUserDenied) } } diff --git a/src/shared/mod.rs b/src/shared/mod.rs index 3769ee6..ef6cb76 100644 --- a/src/shared/mod.rs +++ b/src/shared/mod.rs @@ -9,7 +9,7 @@ mod capture; mod config_load; mod logging; -pub(crate) mod analyze; +pub mod analyze; pub mod directories; mod models; mod redact; diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 3de68ee..f28d70a 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -72,6 +72,22 @@ impl<'a> ScopeTestHelper<'a> { self.run_command(&run_command) } + pub fn intercept_command(&self, args: &[&str]) -> Assert { + let mut cmd = Command::cargo_bin("scope-intercept").unwrap(); + cmd.current_dir(self.work_dir.path()) + .env( + "SCOPE_RUN_ID", + format!( + "{}-{}", + self.name, + self.counter.fetch_add(1, Ordering::Relaxed) + ), + ) + .env("NO_COLOR", "1") + .args(args) + .assert() + } + pub fn clean_work_dir(self) { self.work_dir.close().unwrap(); } diff --git a/tests/scope_intercept.rs b/tests/scope_intercept.rs new file mode 100644 index 0000000..75a74db --- /dev/null +++ b/tests/scope_intercept.rs @@ -0,0 +1,160 @@ +use assert_fs::fixture::{FileWriteStr, PathChild}; +use predicates::boolean::PredicateBooleanExt; +use predicates::prelude::predicate; + +#[allow(dead_code)] +mod common; +use common::*; + +#[test] +fn test_intercept_fix_succeeds_retry_succeeds() { + let helper = ScopeTestHelper::new( + "test_intercept_fix_succeeds_retry_succeeds", + "intercept-known-error-with-fix", + ); + + helper + .intercept_command(&["--yolo", "cat", "status.txt"]) + .success() + .stdout(predicate::str::contains( + "Known error 'missing-status-file' found", + )) + .stdout(predicate::str::contains("Fix succeeded, retrying command")) + .stdout(predicate::str::contains("ready")); + + helper.clean_work_dir(); +} + +#[test] +fn test_intercept_fix_succeeds_retry_succeeds_via_script() { + let helper = ScopeTestHelper::new( + "test_intercept_fix_succeeds_retry_succeeds_via_script", + "intercept-known-error-with-fix", + ); + + // Mirrors the shebang use case: `#!/path/to/scope-intercept bash` + helper + .work_dir + .child("setup.sh") + .write_str("#!/bin/bash\nset -e\necho 'Running setup...'\ncat status.txt\necho 'Done!'\n") + .unwrap(); + + helper + .intercept_command(&["--yolo", "bash", "setup.sh"]) + .success() + .stdout(predicate::str::contains("Running setup...")) + .stdout(predicate::str::contains("Fix succeeded, retrying command")) + .stdout(predicate::str::contains("Done!")); + + helper.clean_work_dir(); +} + +// Fix resolves the first failure (status.txt) but the command also requires +// other.txt, which the fix does not create. Retry still fails. +#[test] +fn test_intercept_fix_succeeds_retry_fails() { + let helper = ScopeTestHelper::new( + "test_intercept_fix_succeeds_retry_fails", + "intercept-known-error-fix-retry-fails", + ); + + helper + .work_dir + .child("check.sh") + .write_str("#!/bin/bash\nset -e\ncat status.txt\ncat other.txt\n") + .unwrap(); + + helper + .intercept_command(&["--yolo", "bash", "check.sh"]) + .failure() + .stdout(predicate::str::contains( + "Known error 'missing-status-file' found", + )) + .stdout(predicate::str::contains("Fix succeeded, retrying command")) + .stdout(predicate::str::contains("ready")); + + helper.clean_work_dir(); +} + +#[test] +fn test_intercept_known_error_no_fix() { + let helper = ScopeTestHelper::new( + "test_intercept_known_error_no_fix", + "intercept-known-error-no-fix", + ); + + helper + .intercept_command(&["--", "bash", "-c", "echo 'something went wrong'; exit 1"]) + .failure() + .stdout(predicate::str::contains( + "Known error 'something-broke' found", + )) + .stdout(predicate::str::contains( + "This is a known issue. Check the wiki for manual steps.", + )) + .stdout(predicate::str::contains("No automatic fix available")) + .stdout(predicate::str::contains("Fix succeeded").not()); + + helper.clean_work_dir(); +} + +// Without --yolo, assert_cmd pipes stdin (no TTY), so inquire returns NotTTY +// which maps to KnownErrorFoundUserDenied — same as the user answering "No". +#[test] +fn test_intercept_no_tty_skips_fix() { + let helper = ScopeTestHelper::new( + "test_intercept_no_tty_skips_fix", + "intercept-known-error-with-fix", + ); + + helper + .intercept_command(&["cat", "status.txt"]) + .failure() + .stdout(predicate::str::contains( + "Known error 'missing-status-file' found", + )) + .stdout(predicate::str::contains("User denied fix")) + .stdout(predicate::str::contains("Fix succeeded").not()); + + helper.clean_work_dir(); +} + +#[test] +fn test_intercept_succeeds_first_try() { + let helper = ScopeTestHelper::new( + "test_intercept_succeeds_first_try", + "intercept-known-error-with-fix", + ); + + helper + .work_dir + .child("status.txt") + .write_str("ready\n") + .unwrap(); + + helper + .intercept_command(&["cat", "status.txt"]) + .success() + .stdout(predicate::str::contains("ready")) + .stdout(predicate::str::contains("Command failed").not()); + + helper.clean_work_dir(); +} + +// Exit code 42 is non-standard; intercept must preserve it. +#[test] +fn test_intercept_no_known_errors_match() { + let helper = ScopeTestHelper::new( + "test_intercept_no_known_errors_match", + "intercept-known-error-with-fix", + ); + + helper + .intercept_command(&["--", "bash", "-c", "echo 'totally unexpected'; exit 42"]) + .failure() + .code(42) + .stdout(predicate::str::contains("No known errors found")) + .stdout(predicate::str::contains("Fix succeeded").not()); + + helper.clean_work_dir(); +} diff --git a/tests/test-cases/doctor-known-error-with-fix/.scope/doctor-group.yaml b/tests/test-cases/doctor-known-error-with-fix/.scope/doctor-group.yaml new file mode 100644 index 0000000..f8d4b06 --- /dev/null +++ b/tests/test-cases/doctor-known-error-with-fix/.scope/doctor-group.yaml @@ -0,0 +1,14 @@ +apiVersion: scope.github.com/v1alpha +kind: ScopeDoctorGroup +metadata: + name: known-error-test +spec: + actions: + - name: needs-file + check: + commands: + - test -f status.txt + fix: + commands: + - echo fix-failed-status-missing + - test -f status.txt diff --git a/tests/test-cases/doctor-known-error-with-fix/.scope/known-error.yaml b/tests/test-cases/doctor-known-error-with-fix/.scope/known-error.yaml new file mode 100644 index 0000000..14ad773 --- /dev/null +++ b/tests/test-cases/doctor-known-error-with-fix/.scope/known-error.yaml @@ -0,0 +1,13 @@ +apiVersion: scope.github.com/v1alpha +kind: ScopeKnownError +metadata: + name: missing-status-file + description: Detects when the fix fails because status.txt is missing +spec: + pattern: fix-failed-status-missing + help: The status.txt file is missing. + fix: + prompt: + text: "Create the status.txt file?" + commands: + - bash -c 'echo "ready" > status.txt' diff --git a/tests/test-cases/intercept-known-error-fix-retry-fails/.scope/known-error.yaml b/tests/test-cases/intercept-known-error-fix-retry-fails/.scope/known-error.yaml new file mode 100644 index 0000000..4a745c5 --- /dev/null +++ b/tests/test-cases/intercept-known-error-fix-retry-fails/.scope/known-error.yaml @@ -0,0 +1,13 @@ +apiVersion: scope.github.com/v1alpha +kind: ScopeKnownError +metadata: + name: missing-status-file + description: Detects when status.txt is missing and creates it +spec: + pattern: "status.txt: No such file" + help: The status.txt file is missing. + fix: + prompt: + text: "Create the status.txt file?" + commands: + - bash -c 'echo "ready" > status.txt' diff --git a/tests/test-cases/intercept-known-error-no-fix/.scope/known-error.yaml b/tests/test-cases/intercept-known-error-no-fix/.scope/known-error.yaml new file mode 100644 index 0000000..f88017b --- /dev/null +++ b/tests/test-cases/intercept-known-error-no-fix/.scope/known-error.yaml @@ -0,0 +1,8 @@ +apiVersion: scope.github.com/v1alpha +kind: ScopeKnownError +metadata: + name: something-broke + description: A known error with no automatic fix +spec: + pattern: "something went wrong" + help: "This is a known issue. Check the wiki for manual steps." diff --git a/tests/test-cases/intercept-known-error-with-fix/.scope/known-error.yaml b/tests/test-cases/intercept-known-error-with-fix/.scope/known-error.yaml new file mode 100644 index 0000000..4a745c5 --- /dev/null +++ b/tests/test-cases/intercept-known-error-with-fix/.scope/known-error.yaml @@ -0,0 +1,13 @@ +apiVersion: scope.github.com/v1alpha +kind: ScopeKnownError +metadata: + name: missing-status-file + description: Detects when status.txt is missing and creates it +spec: + pattern: "status.txt: No such file" + help: The status.txt file is missing. + fix: + prompt: + text: "Create the status.txt file?" + commands: + - bash -c 'echo "ready" > status.txt'