Skip to content

Commit 43c96d4

Browse files
committed
feat(rust-cli): wire SIGINT trap handler into REPL loops
The `trap 'handler' INT` mechanism was registering handlers in the TrapTable but never firing them. Now: - Both REPL variants (basic and enhanced/reedline) check for pending SIGINT after each command cycle and execute the registered INT trap handler if one exists. - The enhanced REPL also fires INT traps on Ctrl-C (reedline returns Signal::CtrlC directly, bypassing the ctrlc crate's async flag). - A new public helper `run_pending_traps(state)` in executable.rs encapsulates the check-and-fire logic. It checks the SIGINT flag, clears it, looks up the INT trap handler, parses and executes it. Returns true if the handler produced an Exit result. - EXIT trap was already firing at shell exit (main.rs). No change. Tests: 7 new tests covering trap registration, trap reset, INT trap firing via run_pending_traps (with and without handler), no-signal noop, and EXIT trap manual firing. Test results: 736 passing, 0 failing, 14 ignored (up from 729). https://claude.ai/code/session_01EMHrh5Jq32pb98KXoSKLA4
1 parent 2c992a0 commit 43c96d4

4 files changed

Lines changed: 185 additions & 2 deletions

File tree

impl/rust-cli/src/enhanced_repl.rs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use reedline::{
1919
use std::borrow::Cow;
2020
use std::path::PathBuf;
2121

22-
use crate::executable::{ExecutableCommand, ExecutionResult};
22+
use crate::executable::{self, ExecutableCommand, ExecutionResult};
2323
use crate::highlighter::VshHighlighter;
2424
use crate::parser;
2525
use crate::signals;
@@ -312,6 +312,17 @@ pub fn run(state: &mut ShellState) -> Result<()> {
312312
accumulated_input.clear();
313313
// Cancel multi-line input
314314
}
315+
// Fire INT trap if registered (e.g. trap 'echo caught' INT).
316+
// Reedline handles Ctrl-C itself (returns Signal::CtrlC) so
317+
// we fire the trap synchronously here rather than relying on
318+
// the SIGINT flag.
319+
if let Some(handler) = state.traps.get(crate::posix_builtins::TrapSignal::Int).map(|s| s.to_string()) {
320+
if let Ok(cmd) = parser::parse_command(&handler) {
321+
if let Ok(ExecutionResult::Exit) = cmd.execute(state) {
322+
break;
323+
}
324+
}
325+
}
315326
continue;
316327
}
317328
Err(err) => {
@@ -342,6 +353,10 @@ fn execute_segments(state: &mut ShellState, input: &str) -> Result<bool> {
342353
}
343354
}
344355
}
356+
// Fire any pending signal traps (e.g. trap 'handler' INT).
357+
if executable::run_pending_traps(state) {
358+
return Ok(true);
359+
}
345360
Ok(false)
346361
}
347362

impl/rust-cli/src/executable.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1575,3 +1575,32 @@ fn glob_match_inner(pattern: &[char], text: &[char], pi: usize, ti: usize) -> bo
15751575
}
15761576
}
15771577
}
1578+
1579+
/// Check for pending signal traps and fire them.
1580+
///
1581+
/// Call this from the REPL loop after each command completes. If SIGINT
1582+
/// was received and the user has set `trap '...' INT`, the trap handler
1583+
/// is executed. If no trap is set, the interrupt is silently cleared
1584+
/// (the current behaviour — just cancel the current line).
1585+
///
1586+
/// Returns `true` if an EXIT result was produced by the trap handler
1587+
/// (i.e. the shell should quit).
1588+
pub fn run_pending_traps(state: &mut ShellState) -> bool {
1589+
use crate::posix_builtins::TrapSignal;
1590+
1591+
if crate::signals::is_interrupt_requested() {
1592+
crate::signals::clear_interrupt();
1593+
1594+
if let Some(handler) = state.traps.get(TrapSignal::Int).map(|s| s.to_string()) {
1595+
if let Ok(cmd) = crate::parser::parse_command(&handler) {
1596+
if let Ok(result) = cmd.execute(state) {
1597+
if matches!(result, ExecutionResult::Exit) {
1598+
return true;
1599+
}
1600+
}
1601+
}
1602+
}
1603+
}
1604+
1605+
false
1606+
}

impl/rust-cli/src/repl.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use anyhow::Result;
1010
use colored::Colorize;
1111
use std::io::{self, BufRead, Write};
1212

13-
use crate::executable::{ExecutableCommand, ExecutionResult};
13+
use crate::executable::{self, ExecutableCommand, ExecutionResult};
1414
use crate::parser;
1515
use crate::signals;
1616
use crate::state::ShellState;
@@ -91,6 +91,12 @@ pub fn run(state: &mut ShellState) -> Result<()> {
9191
}
9292
}
9393
}
94+
95+
// Fire any pending signal traps (e.g. trap 'handler' INT).
96+
if executable::run_pending_traps(state) {
97+
break;
98+
}
99+
94100
if should_break {
95101
break;
96102
}
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
// SPDX-License-Identifier: PMPL-1.0-or-later
2+
//! Tests for trap handler execution.
3+
//!
4+
//! Verifies that:
5+
//! - EXIT trap fires at shell exit
6+
//! - INT trap fires when SIGINT is received
7+
//! - Trap handlers can execute shell commands
8+
//! - `trap -` resets signal handling
9+
10+
use anyhow::Result;
11+
use tempfile::tempdir;
12+
use vsh::executable::{self, ExecutableCommand};
13+
use vsh::parser::parse_command;
14+
use vsh::posix_builtins::TrapSignal;
15+
use vsh::state::ShellState;
16+
17+
// -------------------------------------------------------------------------
18+
// Trap registration
19+
// -------------------------------------------------------------------------
20+
21+
#[test]
22+
fn trap_set_and_retrieve() -> Result<()> {
23+
let temp = tempdir()?;
24+
let mut state = ShellState::new(temp.path().to_str().unwrap())?;
25+
26+
parse_command("trap 'echo bye' EXIT")?.execute(&mut state)?;
27+
28+
assert_eq!(state.traps.get(TrapSignal::Exit), Some("echo bye"));
29+
assert!(state.traps.is_trapped(TrapSignal::Exit));
30+
Ok(())
31+
}
32+
33+
#[test]
34+
fn trap_reset_with_dash() -> Result<()> {
35+
let temp = tempdir()?;
36+
let mut state = ShellState::new(temp.path().to_str().unwrap())?;
37+
38+
parse_command("trap 'echo bye' EXIT")?.execute(&mut state)?;
39+
assert!(state.traps.is_trapped(TrapSignal::Exit));
40+
41+
parse_command("trap - EXIT")?.execute(&mut state)?;
42+
assert!(!state.traps.is_trapped(TrapSignal::Exit));
43+
Ok(())
44+
}
45+
46+
#[test]
47+
fn trap_set_int_handler() -> Result<()> {
48+
let temp = tempdir()?;
49+
let mut state = ShellState::new(temp.path().to_str().unwrap())?;
50+
51+
parse_command("trap 'echo caught' INT")?.execute(&mut state)?;
52+
53+
assert_eq!(state.traps.get(TrapSignal::Int), Some("echo caught"));
54+
Ok(())
55+
}
56+
57+
// -------------------------------------------------------------------------
58+
// INT trap firing via run_pending_traps
59+
// -------------------------------------------------------------------------
60+
61+
#[test]
62+
fn run_pending_traps_fires_int_handler() -> Result<()> {
63+
let temp = tempdir()?;
64+
let mut state = ShellState::new(temp.path().to_str().unwrap())?;
65+
66+
// Set an INT trap that creates a directory
67+
parse_command("trap 'mkdir trap_fired' INT")?.execute(&mut state)?;
68+
69+
// Simulate SIGINT
70+
vsh::signals::request_interrupt();
71+
72+
// Fire pending traps
73+
let should_exit = executable::run_pending_traps(&mut state);
74+
assert!(!should_exit);
75+
76+
// The trap handler should have executed
77+
assert!(state.resolve_path("trap_fired").exists());
78+
Ok(())
79+
}
80+
81+
#[test]
82+
fn run_pending_traps_no_handler_clears_flag() -> Result<()> {
83+
let temp = tempdir()?;
84+
let mut state = ShellState::new(temp.path().to_str().unwrap())?;
85+
86+
// No trap set — simulate SIGINT
87+
vsh::signals::request_interrupt();
88+
assert!(vsh::signals::is_interrupt_requested());
89+
90+
// run_pending_traps should clear the flag even with no handler
91+
executable::run_pending_traps(&mut state);
92+
assert!(!vsh::signals::is_interrupt_requested());
93+
Ok(())
94+
}
95+
96+
#[test]
97+
fn run_pending_traps_no_signal_is_noop() -> Result<()> {
98+
let temp = tempdir()?;
99+
let mut state = ShellState::new(temp.path().to_str().unwrap())?;
100+
101+
parse_command("trap 'mkdir should_not_exist' INT")?.execute(&mut state)?;
102+
103+
// No SIGINT simulated
104+
vsh::signals::clear_interrupt();
105+
executable::run_pending_traps(&mut state);
106+
107+
assert!(!state.resolve_path("should_not_exist").exists());
108+
Ok(())
109+
}
110+
111+
// -------------------------------------------------------------------------
112+
// EXIT trap firing
113+
// -------------------------------------------------------------------------
114+
115+
#[test]
116+
fn exit_trap_is_registered_for_later_firing() -> Result<()> {
117+
let temp = tempdir()?;
118+
let mut state = ShellState::new(temp.path().to_str().unwrap())?;
119+
120+
parse_command("trap 'mkdir exit_cleanup' EXIT")?.execute(&mut state)?;
121+
122+
// The trap is registered but not yet fired
123+
assert!(!state.resolve_path("exit_cleanup").exists());
124+
assert!(state.traps.is_trapped(TrapSignal::Exit));
125+
126+
// Manually fire the EXIT trap (simulating shell exit)
127+
if let Some(handler) = state.traps.get(TrapSignal::Exit).map(|s| s.to_string()) {
128+
parse_command(&handler)?.execute(&mut state)?;
129+
}
130+
131+
assert!(state.resolve_path("exit_cleanup").exists());
132+
Ok(())
133+
}

0 commit comments

Comments
 (0)