refactor: [#107] apply GitHub Copilot PR review suggestions

- Fix error mapping: Use CreateSubcommandError::UserOutputLockFailed instead of ProgressReporterError::UserOutputMutexPoisoned
- Eliminate unnecessary clones: Use ctx.user_output().clone() directly for ProgressReporter initialization
- Enhance panic message with detailed context about UserOutput lock poisoning
- Document fallback error formatting rationale in handle_error()

All changes address PR #108 review comments from GitHub Copilot.

Author: josecelano

src/presentation/commands/create/subcommands/environment.rs

@@ -54,7 +54,7 @@ pub fn handle_environment_creation(
     let ctx = factory.create_context(working_dir.to_path_buf(), user_output.clone());
 
     // Create progress reporter for 3 main steps
-    let mut progress = ProgressReporter::new(user_output.clone(), 3);
+    let mut progress = ProgressReporter::new(ctx.user_output().clone(), 3);
 
     // Step 1: Load configuration
     progress.start_step("Loading configuration")?;
@@ -117,7 +117,7 @@ fn load_configuration(
 ) -> Result<EnvironmentCreationConfig, CreateSubcommandError> {
     user_output
         .lock()
-        .map_err(|_| crate::presentation::progress::ProgressReporterError::UserOutputMutexPoisoned)?
+        .map_err(|_| CreateSubcommandError::UserOutputLockFailed)?
         .progress(&format!(
             "Loading configuration from '{}'...",
             env_file.display()
@@ -159,15 +159,15 @@ fn execute_create_command(
 ) -> Result<Environment, CreateSubcommandError> {
     user_output
         .lock()
-        .map_err(|_| crate::presentation::progress::ProgressReporterError::UserOutputMutexPoisoned)?
+        .map_err(|_| CreateSubcommandError::UserOutputLockFailed)?
         .progress(&format!(
             "Creating environment '{}'...",
             config.environment.name
         ));
 
     user_output
         .lock()
-        .map_err(|_| crate::presentation::progress::ProgressReporterError::UserOutputMutexPoisoned)?
+        .map_err(|_| CreateSubcommandError::UserOutputLockFailed)?
         .progress("Validating configuration and creating environment...");
 
     #[allow(clippy::manual_inspect)]
@@ -199,10 +199,16 @@ fn execute_create_command(
 /// This function will panic if the `UserOutput` mutex is poisoned. Since this is
 /// called after successful environment creation (when operation is complete),
 /// a poisoned mutex indicates an irrecoverable state and panicking is acceptable.
+///
+/// The panic message provides detailed context matching our error handling principles:
+/// clear explanation of what happened, why it's critical, and that it indicates a bug.
 fn display_creation_results(user_output: &Arc<Mutex<UserOutput>>, environment: &Environment) {
-    let mut output = user_output
-        .lock()
-        .expect("UserOutput mutex poisoned after successful environment creation");
+    let mut output = user_output.lock().expect(
+        "CRITICAL: UserOutput mutex poisoned after successful environment creation. \
+         This indicates a panic occurred in another thread while holding the output lock. \
+         The environment was created successfully, but we cannot display the results. \
+         This is a bug - please report it with full logs using --log-output file-and-stderr",
+    );
 
     output.success(&format!(
         "Environment '{}' created successfully",

src/presentation/commands/destroy/handler.rs

@@ -65,7 +65,7 @@ pub fn handle_destroy_command(
     let ctx = factory.create_context(working_dir.to_path_buf(), user_output.clone());
 
     // Create progress reporter for 3 main steps
-    let mut progress = ProgressReporter::new(user_output.clone(), 3);
+    let mut progress = ProgressReporter::new(ctx.user_output().clone(), 3);
 
     // Step 1: Validate environment name
     progress.start_step("Validating environment")?;

src/presentation/commands/mod.rs

@@ -144,6 +144,13 @@ pub fn handle_error(error: &CommandError, user_output: &Arc<Mutex<UserOutput>>)
         output.info_block("For detailed troubleshooting:", &[help_text]);
     } else {
         // Cannot acquire lock - print to stderr directly as fallback
+        //
+        // RATIONALE: Plain text formatting without emojis/styling is intentional.
+        // When the mutex is poisoned, we're in a degraded error state where another
+        // thread has panicked. Using plain eprintln! ensures maximum compatibility
+        // and avoids any additional complexity that could fail in this critical path.
+        // The goal here is reliability over aesthetics - get the error message to
+        // the user no matter what, even if it's not pretty.
         eprintln!("ERROR: {error}");
         eprintln!();
         eprintln!("CRITICAL: Failed to acquire user output lock.");