Revert 79 .expect("TODO: handle error") sites across 3 files (batch 3/N)

Continues the per-site reversal of the .unwrap() → .expect("TODO: handle
error") anti-pattern flagged by CRG-AUDIT-2026-04-18.

Per-site policy:

  * Test code → revert to .unwrap() (test convention; preceding asserts
    or short-circuits already prove the Some/Ok case). This batch:
      - quotes.rs       (27 sites in tests::test_*)
      - redirection.rs  (28 sites in tests::test_redirect_*, test_modification_*)
      - external.rs     (21 sites in tests::test_execute_*, test_pipeline_*)

  * Production code, external.rs (3 sites):
    - L164 stdin_cfg: kept `expect` with documented invariant — when
      idx > 0, the previous iteration spawned a non-last child with
      Stdio::piped() stdout and stored child.stdout into prev_stdout.
      The expect now records the unreachable case rather than a TODO.
    - L168-185 stdout_cfg + stderr_cfg: restructured to pivot on
      `match &redirect_setup` instead of double-checking
      `if !redirects.is_empty()` and then `.expect()`-ing the Option.
      `redirect_setup` is Some iff `!redirects.is_empty()` (set above).
      Both panic sites eliminated; one duplicated emptiness check removed.

Verified locally:
  cargo build  -> EXIT 0
  cargo test   -> 703 passed / 0 failed / 14 ignored (unchanged across
                  batches 1+2+3; restructure preserves behaviour exactly)

Cumulative progress: 136 / ~530 sites cleared (~26%) across 13 files.

Remaining big-3 in impl/rust-cli/src/: parser.rs (116), commands.rs (66),
arith.rs (55) — each warrants its own dedicated session. Mid-tier left:
state.rs (35). Plus impl/rust-ffi/ ↔ ffi/rust/ duplicate tree (~50 more,
separate finding).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: hyperpolymath

impl/rust-cli/src/external.rs

@@ -160,28 +160,34 @@ pub fn execute_pipeline(
             // First stage: inherit from shell
             Stdio::inherit()
         } else {
-            // Middle/last stages: read from previous stdout
-            Stdio::from(prev_stdout.take().expect("TODO: handle error"))
+            // Middle/last stages: read from previous stdout.
+            // Invariant: when idx > 0, the previous iteration spawned a
+            // non-last child with Stdio::piped() stdout (line below) and
+            // stored child.stdout into prev_stdout after spawning. So the
+            // expect documents an unreachable branch rather than a TODO.
+            Stdio::from(prev_stdout.take().expect(
+                "prev_stdout invariant: previous non-last stage stored its piped stdout",
+            ))
         };
 
-        // Configure stdout
+        // Configure stdout. Pivot on redirect_setup directly: it is Some
+        // iff redirects is non-empty (constructed above), so matching the
+        // Option both removes a duplicated emptiness check and eliminates
+        // a previous panic site.
         let stdout_cfg = if is_last {
-            // Last stage: redirect to file or inherit
-            if !redirects.is_empty() {
-                stdio_config_from_redirects(redirects, redirect_setup.as_ref().expect("TODO: handle error"), state)?.1
-            } else {
-                Stdio::inherit()
+            match &redirect_setup {
+                Some(setup) => stdio_config_from_redirects(redirects, setup, state)?.1,
+                None => Stdio::inherit(),
             }
         } else {
             // First/middle stages: pipe to next
             Stdio::piped()
         };
 
-        // Configure stderr (inherit unless redirected in last stage)
-        let stderr_cfg = if is_last && !redirects.is_empty() {
-            stdio_config_from_redirects(redirects, redirect_setup.as_ref().expect("TODO: handle error"), state)?.2
-        } else {
-            Stdio::inherit()
+        // Configure stderr (inherit unless redirected in last stage).
+        let stderr_cfg = match (is_last, &redirect_setup) {
+            (true, Some(setup)) => stdio_config_from_redirects(redirects, setup, state)?.2,
+            _ => Stdio::inherit(),
         };
 
         // Spawn child
@@ -694,67 +700,67 @@ mod tests {
     #[test]
     fn test_external_command_success() {
         // Test successful command execution
-        let exit_code = execute_external("true", &[]).expect("TODO: handle error");
+        let exit_code = execute_external("true", &[]).unwrap();
         assert_eq!(exit_code, 0, "true command should return 0");
     }
 
     #[test]
     fn test_external_command_failure() {
         // Test failed command execution
-        let exit_code = execute_external("false", &[]).expect("TODO: handle error");
+        let exit_code = execute_external("false", &[]).unwrap();
         assert_eq!(exit_code, 1, "false command should return 1");
     }
 
     #[test]
     fn test_external_command_with_args() {
         // Test command with arguments
-        let exit_code = execute_external("echo", &["hello".to_string()]).expect("TODO: handle error");
+        let exit_code = execute_external("echo", &["hello".to_string()]).unwrap();
         assert_eq!(exit_code, 0, "echo should return 0");
     }
 
     #[test]
     fn test_pipeline_simple() {
         // Test simple 2-stage pipeline: true | true
         use tempfile::TempDir;
-        let temp = TempDir::new().expect("TODO: handle error");
-        let mut state = crate::state::ShellState::new(temp.path().to_str().expect("TODO: handle error")).expect("TODO: handle error");
+        let temp = TempDir::new().unwrap();
+        let mut state = crate::state::ShellState::new(temp.path().to_str().unwrap()).unwrap();
 
         let stages = vec![
             ("true".to_string(), vec![]),
             ("true".to_string(), vec![]),
         ];
-        let exit_code = execute_pipeline(&stages, &[], &mut state).expect("TODO: handle error");
+        let exit_code = execute_pipeline(&stages, &[], &mut state).unwrap();
         assert_eq!(exit_code, 0, "true | true should return 0");
     }
 
     #[test]
     fn test_pipeline_exit_code() {
         // Test that pipeline returns exit code from last stage
         use tempfile::TempDir;
-        let temp = TempDir::new().expect("TODO: handle error");
-        let mut state = crate::state::ShellState::new(temp.path().to_str().expect("TODO: handle error")).expect("TODO: handle error");
+        let temp = TempDir::new().unwrap();
+        let mut state = crate::state::ShellState::new(temp.path().to_str().unwrap()).unwrap();
 
         let stages = vec![
             ("true".to_string(), vec![]),
             ("false".to_string(), vec![]),
         ];
-        let exit_code = execute_pipeline(&stages, &[], &mut state).expect("TODO: handle error");
+        let exit_code = execute_pipeline(&stages, &[], &mut state).unwrap();
         assert_eq!(exit_code, 1, "true | false should return 1 (from false)");
     }
 
     #[test]
     fn test_pipeline_three_stages() {
         // Test 3-stage pipeline
         use tempfile::TempDir;
-        let temp = TempDir::new().expect("TODO: handle error");
-        let mut state = crate::state::ShellState::new(temp.path().to_str().expect("TODO: handle error")).expect("TODO: handle error");
+        let temp = TempDir::new().unwrap();
+        let mut state = crate::state::ShellState::new(temp.path().to_str().unwrap()).unwrap();
 
         let stages = vec![
             ("echo".to_string(), vec!["hello".to_string()]),
             ("cat".to_string(), vec![]),
             ("cat".to_string(), vec![]),
         ];
-        let exit_code = execute_pipeline(&stages, &[], &mut state).expect("TODO: handle error");
+        let exit_code = execute_pipeline(&stages, &[], &mut state).unwrap();
         assert_eq!(exit_code, 0, "echo hello | cat | cat should return 0");
     }
 
@@ -763,8 +769,8 @@ mod tests {
         // Test pipeline with output redirection
         use tempfile::TempDir;
         use std::fs;
-        let temp = TempDir::new().expect("TODO: handle error");
-        let mut state = crate::state::ShellState::new(temp.path().to_str().expect("TODO: handle error")).expect("TODO: handle error");
+        let temp = TempDir::new().unwrap();
+        let mut state = crate::state::ShellState::new(temp.path().to_str().unwrap()).unwrap();
 
         let stages = vec![
             ("echo".to_string(), vec!["test".to_string()]),
@@ -773,23 +779,23 @@ mod tests {
         let redirects = vec![crate::redirection::Redirection::Output {
             file: "output.txt".to_string(),
         }];
-        let exit_code = execute_pipeline(&stages, &redirects, &mut state).expect("TODO: handle error");
+        let exit_code = execute_pipeline(&stages, &redirects, &mut state).unwrap();
         assert_eq!(exit_code, 0, "Pipeline with redirect should return 0");
 
         // Verify file was created
         let output_file = temp.path().join("output.txt");
         assert!(output_file.exists(), "Output file should be created");
 
-        let content = fs::read_to_string(output_file).expect("TODO: handle error");
+        let content = fs::read_to_string(output_file).unwrap();
         assert_eq!(content.trim(), "test", "Output should be 'test'");
     }
 
     #[test]
     fn test_pipeline_error_empty_stages() {
         // Test that empty stages array returns error
         use tempfile::TempDir;
-        let temp = TempDir::new().expect("TODO: handle error");
-        let mut state = crate::state::ShellState::new(temp.path().to_str().expect("TODO: handle error")).expect("TODO: handle error");
+        let temp = TempDir::new().unwrap();
+        let mut state = crate::state::ShellState::new(temp.path().to_str().unwrap()).unwrap();
 
         let stages: Vec<(String, Vec<String>)> = vec![];
         let result = execute_pipeline(&stages, &[], &mut state);
@@ -800,11 +806,11 @@ mod tests {
     fn test_pipeline_single_stage_delegates() {
         // Test that single-stage pipeline delegates to execute_external_with_redirects
         use tempfile::TempDir;
-        let temp = TempDir::new().expect("TODO: handle error");
-        let mut state = crate::state::ShellState::new(temp.path().to_str().expect("TODO: handle error")).expect("TODO: handle error");
+        let temp = TempDir::new().unwrap();
+        let mut state = crate::state::ShellState::new(temp.path().to_str().unwrap()).unwrap();
 
         let stages = vec![("true".to_string(), vec![])];
-        let exit_code = execute_pipeline(&stages, &[], &mut state).expect("TODO: handle error");
+        let exit_code = execute_pipeline(&stages, &[], &mut state).unwrap();
         assert_eq!(exit_code, 0, "Single-stage pipeline should work");
     }
 }

impl/rust-cli/src/quotes.rs

@@ -305,31 +305,31 @@ mod tests {
 
     #[test]
     fn test_single_quotes_literal() {
-        let segments = parse_quotes("'$HOME'").expect("TODO: handle error");
+        let segments = parse_quotes("'$HOME'").unwrap();
         assert_eq!(segments.len(), 1);
         assert_eq!(segments[0].content, "$HOME");
         assert_eq!(segments[0].state, QuoteState::SingleQuoted);
     }
 
     #[test]
     fn test_double_quotes() {
-        let segments = parse_quotes("\"$HOME\"").expect("TODO: handle error");
+        let segments = parse_quotes("\"$HOME\"").unwrap();
         assert_eq!(segments.len(), 1);
         assert_eq!(segments[0].content, "$HOME");
         assert_eq!(segments[0].state, QuoteState::DoubleQuoted);
     }
 
     #[test]
     fn test_unquoted() {
-        let segments = parse_quotes("hello").expect("TODO: handle error");
+        let segments = parse_quotes("hello").unwrap();
         assert_eq!(segments.len(), 1);
         assert_eq!(segments[0].content, "hello");
         assert_eq!(segments[0].state, QuoteState::Unquoted);
     }
 
     #[test]
     fn test_adjacent_quotes() {
-        let segments = parse_quotes("'hello'\"world\"").expect("TODO: handle error");
+        let segments = parse_quotes("'hello'\"world\"").unwrap();
         assert_eq!(segments.len(), 2);
         assert_eq!(segments[0].content, "hello");
         assert_eq!(segments[0].state, QuoteState::SingleQuoted);
@@ -339,7 +339,7 @@ mod tests {
 
     #[test]
     fn test_mixed_quotes() {
-        let segments = parse_quotes("pre'quoted'post").expect("TODO: handle error");
+        let segments = parse_quotes("pre'quoted'post").unwrap();
         assert_eq!(segments.len(), 3);
         assert_eq!(segments[0].content, "pre");
         assert_eq!(segments[0].state, QuoteState::Unquoted);
@@ -351,40 +351,40 @@ mod tests {
 
     #[test]
     fn test_backslash_escape_unquoted() {
-        let segments = parse_quotes("hello\\ world").expect("TODO: handle error");
+        let segments = parse_quotes("hello\\ world").unwrap();
         assert_eq!(segments.len(), 1);
         assert_eq!(segments[0].content, "hello world");
         assert_eq!(segments[0].state, QuoteState::Unquoted);
     }
 
     #[test]
     fn test_backslash_in_single_quotes() {
-        let segments = parse_quotes("'back\\slash'").expect("TODO: handle error");
+        let segments = parse_quotes("'back\\slash'").unwrap();
         assert_eq!(segments.len(), 1);
         assert_eq!(segments[0].content, "back\\slash");
         assert_eq!(segments[0].state, QuoteState::SingleQuoted);
     }
 
     #[test]
     fn test_backslash_in_double_quotes() {
-        let segments = parse_quotes("\"\\$HOME\"").expect("TODO: handle error");
+        let segments = parse_quotes("\"\\$HOME\"").unwrap();
         assert_eq!(segments.len(), 1);
         assert_eq!(segments[0].content, "$HOME");
         assert_eq!(segments[0].state, QuoteState::DoubleQuoted);
 
         // Backslash before non-special character stays
-        let segments = parse_quotes("\"\\a\"").expect("TODO: handle error");
+        let segments = parse_quotes("\"\\a\"").unwrap();
         assert_eq!(segments[0].content, "\\a");
     }
 
     #[test]
     fn test_empty_quotes() {
-        let segments = parse_quotes("''").expect("TODO: handle error");
+        let segments = parse_quotes("''").unwrap();
         assert_eq!(segments.len(), 1);
         assert_eq!(segments[0].content, "");
         assert_eq!(segments[0].state, QuoteState::SingleQuoted);
 
-        let segments = parse_quotes("\"\"").expect("TODO: handle error");
+        let segments = parse_quotes("\"\"").unwrap();
         assert_eq!(segments.len(), 1);
         assert_eq!(segments[0].content, "");
         assert_eq!(segments[0].state, QuoteState::DoubleQuoted);
@@ -407,74 +407,74 @@ mod tests {
     #[test]
     fn test_glob_no_expansion_in_quotes() {
         // Unquoted - should expand
-        let segments = parse_quotes("*.txt").expect("TODO: handle error");
+        let segments = parse_quotes("*.txt").unwrap();
         assert!(should_expand_glob(&segments));
 
         // Single quoted - no expansion
-        let segments = parse_quotes("'*.txt'").expect("TODO: handle error");
+        let segments = parse_quotes("'*.txt'").unwrap();
         assert!(!should_expand_glob(&segments));
 
         // Double quoted - no expansion
-        let segments = parse_quotes("\"*.txt\"").expect("TODO: handle error");
+        let segments = parse_quotes("\"*.txt\"").unwrap();
         assert!(!should_expand_glob(&segments));
     }
 
     #[test]
     fn test_reconstruct_string() {
-        let segments = parse_quotes("'hello'\" world\"").expect("TODO: handle error");
+        let segments = parse_quotes("'hello'\" world\"").unwrap();
         let result = reconstruct_string(&segments);
         assert_eq!(result, "hello world");
 
-        let segments = parse_quotes("one'two'three").expect("TODO: handle error");
+        let segments = parse_quotes("one'two'three").unwrap();
         let result = reconstruct_string(&segments);
         assert_eq!(result, "onetwothree");
     }
 
     #[test]
     fn test_allows_variable_expansion() {
-        let segments = parse_quotes("'$VAR'").expect("TODO: handle error");
+        let segments = parse_quotes("'$VAR'").unwrap();
         assert!(!segments[0].allows_variable_expansion());
 
-        let segments = parse_quotes("\"$VAR\"").expect("TODO: handle error");
+        let segments = parse_quotes("\"$VAR\"").unwrap();
         assert!(segments[0].allows_variable_expansion());
 
-        let segments = parse_quotes("$VAR").expect("TODO: handle error");
+        let segments = parse_quotes("$VAR").unwrap();
         assert!(segments[0].allows_variable_expansion());
     }
 
     #[test]
     fn test_allows_glob_expansion() {
-        let segments = parse_quotes("'*.txt'").expect("TODO: handle error");
+        let segments = parse_quotes("'*.txt'").unwrap();
         assert!(!segments[0].allows_glob_expansion());
 
-        let segments = parse_quotes("\"*.txt\"").expect("TODO: handle error");
+        let segments = parse_quotes("\"*.txt\"").unwrap();
         assert!(!segments[0].allows_glob_expansion());
 
-        let segments = parse_quotes("*.txt").expect("TODO: handle error");
+        let segments = parse_quotes("*.txt").unwrap();
         assert!(segments[0].allows_glob_expansion());
     }
 
     #[test]
     fn test_line_continuation() {
         // Backslash-newline in unquoted context
-        let segments = parse_quotes("hello\\\nworld").expect("TODO: handle error");
+        let segments = parse_quotes("hello\\\nworld").unwrap();
         assert_eq!(segments[0].content, "helloworld");
 
         // Backslash-newline in double quotes
-        let segments = parse_quotes("\"hello\\\nworld\"").expect("TODO: handle error");
+        let segments = parse_quotes("\"hello\\\nworld\"").unwrap();
         assert_eq!(segments[0].content, "helloworld");
 
         // Backslash-newline in single quotes (literal)
-        let segments = parse_quotes("'hello\\\nworld'").expect("TODO: handle error");
+        let segments = parse_quotes("'hello\\\nworld'").unwrap();
         assert_eq!(segments[0].content, "hello\\\nworld");
     }
 
     #[test]
     fn test_whitespace_preservation() {
-        let segments = parse_quotes("'hello  world'").expect("TODO: handle error");
+        let segments = parse_quotes("'hello  world'").unwrap();
         assert_eq!(segments[0].content, "hello  world");
 
-        let segments = parse_quotes("\"hello  world\"").expect("TODO: handle error");
+        let segments = parse_quotes("\"hello  world\"").unwrap();
         assert_eq!(segments[0].content, "hello  world");
     }
 }