Skip to content

Commit 870abb7

Browse files
committed
fix: batch merge of bug fixes (PR #286) - issues #2805, #2806, #2808, #2811, #2817
Fixes: - #2805: Install panic hook to track background thread panics and propagate to main thread exit code - #2806: Add /reload-config command to reload configuration from disk without restart - #2808: Use bash instead of sh when bash-specific syntax is detected (e.g., [[, <(), **, source) - #2811: Add ANSI code stripping utilities for piped/redirected output - #2817: /clear command now clears terminal scrollback buffer for privacy
1 parent 7db4c4a commit 870abb7

9 files changed

Lines changed: 368 additions & 3 deletions

File tree

cortex-cli/src/lib.rs

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,18 @@
1919
//! - Terminal cleanup for graceful shutdown
2020
//! - SIGTERM handling for graceful process termination
2121
22-
use std::sync::atomic::{AtomicBool, Ordering};
22+
use std::panic;
23+
use std::sync::atomic::{AtomicBool, AtomicI32, Ordering};
2324

2425
static CLEANUP_REGISTERED: AtomicBool = AtomicBool::new(false);
26+
static PANIC_HOOK_INSTALLED: AtomicBool = AtomicBool::new(false);
27+
28+
/// Global flag to track if any background thread has panicked.
29+
/// This allows the main thread to detect panics and exit with a non-zero code.
30+
static BACKGROUND_PANIC_OCCURRED: AtomicBool = AtomicBool::new(false);
31+
32+
/// Exit code to use when a background panic was detected (101 is conventional for panics).
33+
static PANIC_EXIT_CODE: AtomicI32 = AtomicI32::new(101);
2534

2635
/// Install a Ctrl+C handler that restores the terminal before exiting.
2736
/// This ensures the cursor is visible and terminal state is restored
@@ -34,6 +43,9 @@ pub fn install_cleanup_handler() {
3443
return;
3544
}
3645

46+
// Install panic hook to track background thread panics (#2805)
47+
install_panic_hook();
48+
3749
// Install Ctrl+C (SIGINT) handler
3850
let _ = ctrlc::set_handler(move || {
3951
// Perform cleanup
@@ -125,6 +137,51 @@ fn cleanup_temp_files(temp_dir: &std::path::Path) {
125137
}
126138
}
127139

140+
/// Install a panic hook that tracks panics in background threads.
141+
/// This ensures the main thread can detect background panics and exit
142+
/// with an appropriate error code (#2805).
143+
fn install_panic_hook() {
144+
// Only install once
145+
if PANIC_HOOK_INSTALLED.swap(true, Ordering::SeqCst) {
146+
return;
147+
}
148+
149+
let original_hook = panic::take_hook();
150+
151+
panic::set_hook(Box::new(move |panic_info| {
152+
// Mark that a panic occurred
153+
BACKGROUND_PANIC_OCCURRED.store(true, Ordering::SeqCst);
154+
155+
// Restore terminal state before printing panic message
156+
restore_terminal();
157+
158+
// Log the panic location for debugging
159+
if let Some(location) = panic_info.location() {
160+
eprintln!(
161+
"Panic in thread '{}' at {}:{}:{}",
162+
std::thread::current().name().unwrap_or("<unnamed>"),
163+
location.file(),
164+
location.line(),
165+
location.column()
166+
);
167+
}
168+
169+
// Call original hook for standard panic output
170+
original_hook(panic_info);
171+
}));
172+
}
173+
174+
/// Check if any background thread has panicked.
175+
/// Call this before exiting to ensure proper exit code propagation.
176+
pub fn has_background_panic() -> bool {
177+
BACKGROUND_PANIC_OCCURRED.load(Ordering::SeqCst)
178+
}
179+
180+
/// Get the exit code to use if a background panic occurred.
181+
pub fn get_panic_exit_code() -> i32 {
182+
PANIC_EXIT_CODE.load(Ordering::SeqCst)
183+
}
184+
128185
/// Restore terminal state (cursor visibility, etc.).
129186
/// Called on Ctrl+C or panic to ensure clean terminal state.
130187
pub fn restore_terminal() {

cortex-common/src/ansi.rs

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
//! ANSI escape code handling utilities.
2+
//!
3+
//! Provides functions to detect terminal capabilities and strip ANSI codes
4+
//! when output is piped or redirected (#2811).
5+
6+
use std::io::IsTerminal;
7+
8+
/// Check if stdout should output colors/ANSI codes.
9+
///
10+
/// Returns false when:
11+
/// - stdout is not a terminal (piped/redirected)
12+
/// - NO_COLOR environment variable is set
13+
///
14+
/// This implements the NO_COLOR standard: https://no-color.org/
15+
pub fn should_colorize() -> bool {
16+
// Check NO_COLOR environment variable first (https://no-color.org/)
17+
if std::env::var("NO_COLOR").is_ok() {
18+
return false;
19+
}
20+
21+
// Check if stdout is a terminal
22+
std::io::stdout().is_terminal()
23+
}
24+
25+
/// Check if stderr should output colors/ANSI codes.
26+
pub fn should_colorize_stderr() -> bool {
27+
// Check NO_COLOR environment variable first
28+
if std::env::var("NO_COLOR").is_ok() {
29+
return false;
30+
}
31+
32+
// Check if stderr is a terminal
33+
std::io::stderr().is_terminal()
34+
}
35+
36+
/// Strip ANSI escape codes from a string.
37+
///
38+
/// This removes all ANSI escape sequences including:
39+
/// - Color codes (\x1b[...m)
40+
/// - Cursor movement (\x1b[...H, \x1b[...A, etc.)
41+
/// - Screen clearing (\x1b[2J, etc.)
42+
/// - Other CSI sequences
43+
pub fn strip_ansi_codes(s: &str) -> String {
44+
let mut result = String::with_capacity(s.len());
45+
let mut chars = s.chars().peekable();
46+
47+
while let Some(c) = chars.next() {
48+
if c == '\x1b' {
49+
// Skip the escape sequence
50+
if let Some(&next) = chars.peek() {
51+
if next == '[' {
52+
chars.next(); // consume '['
53+
// Skip until we hit a letter (the terminator)
54+
while let Some(&c) = chars.peek() {
55+
chars.next();
56+
if c.is_ascii_alphabetic() {
57+
break;
58+
}
59+
}
60+
continue;
61+
} else if next == ']' {
62+
// OSC sequence (e.g., terminal title)
63+
chars.next(); // consume ']'
64+
// Skip until BEL (\x07) or ST (\x1b\\)
65+
while let Some(c) = chars.next() {
66+
if c == '\x07' {
67+
break;
68+
}
69+
if c == '\x1b' {
70+
if chars.peek() == Some(&'\\') {
71+
chars.next();
72+
break;
73+
}
74+
}
75+
}
76+
continue;
77+
}
78+
}
79+
}
80+
result.push(c);
81+
}
82+
83+
result
84+
}
85+
86+
/// Conditionally wrap a string with ANSI color codes.
87+
///
88+
/// Returns the colored string if colors are enabled, otherwise returns the plain string.
89+
pub fn maybe_color(text: &str, color_code: &str, reset: &str) -> String {
90+
if should_colorize() {
91+
format!("{}{}{}", color_code, text, reset)
92+
} else {
93+
text.to_string()
94+
}
95+
}
96+
97+
/// Conditionally wrap a string with ANSI color codes for stderr.
98+
pub fn maybe_color_stderr(text: &str, color_code: &str, reset: &str) -> String {
99+
if should_colorize_stderr() {
100+
format!("{}{}{}", color_code, text, reset)
101+
} else {
102+
text.to_string()
103+
}
104+
}
105+
106+
/// ANSI color codes for common colors.
107+
pub mod colors {
108+
pub const RESET: &str = "\x1b[0m";
109+
pub const BOLD: &str = "\x1b[1m";
110+
pub const DIM: &str = "\x1b[2m";
111+
112+
pub const RED: &str = "\x1b[31m";
113+
pub const GREEN: &str = "\x1b[32m";
114+
pub const YELLOW: &str = "\x1b[33m";
115+
pub const BLUE: &str = "\x1b[34m";
116+
pub const MAGENTA: &str = "\x1b[35m";
117+
pub const CYAN: &str = "\x1b[36m";
118+
pub const WHITE: &str = "\x1b[37m";
119+
120+
pub const BOLD_RED: &str = "\x1b[1;31m";
121+
pub const BOLD_GREEN: &str = "\x1b[1;32m";
122+
pub const BOLD_YELLOW: &str = "\x1b[1;33m";
123+
pub const BOLD_BLUE: &str = "\x1b[1;34m";
124+
pub const BOLD_MAGENTA: &str = "\x1b[1;35m";
125+
pub const BOLD_CYAN: &str = "\x1b[1;36m";
126+
}
127+
128+
#[cfg(test)]
129+
mod tests {
130+
use super::*;
131+
132+
#[test]
133+
fn test_strip_ansi_codes_basic() {
134+
let colored = "\x1b[31mRed\x1b[0m Normal";
135+
assert_eq!(strip_ansi_codes(colored), "Red Normal");
136+
}
137+
138+
#[test]
139+
fn test_strip_ansi_codes_complex() {
140+
let colored = "\x1b[1;32mBold Green\x1b[0m \x1b[33mYellow\x1b[0m";
141+
assert_eq!(strip_ansi_codes(colored), "Bold Green Yellow");
142+
}
143+
144+
#[test]
145+
fn test_strip_ansi_codes_cursor() {
146+
let with_cursor = "Hello\x1b[2JWorld\x1b[H";
147+
assert_eq!(strip_ansi_codes(with_cursor), "HelloWorld");
148+
}
149+
150+
#[test]
151+
fn test_strip_ansi_codes_no_codes() {
152+
let plain = "Hello World";
153+
assert_eq!(strip_ansi_codes(plain), "Hello World");
154+
}
155+
156+
#[test]
157+
fn test_strip_ansi_codes_empty() {
158+
assert_eq!(strip_ansi_codes(""), "");
159+
}
160+
161+
#[test]
162+
fn test_strip_ansi_codes_only_codes() {
163+
let only_codes = "\x1b[31m\x1b[0m\x1b[32m\x1b[0m";
164+
assert_eq!(strip_ansi_codes(only_codes), "");
165+
}
166+
}

cortex-common/src/lib.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#![allow(unused_imports)]
22
//! Common utilities shared across Cortex CLI crates.
33
4+
pub mod ansi;
45
pub mod approval_presets;
56
pub mod config_substitution;
67
pub mod cwd_guard;
@@ -21,6 +22,10 @@ pub mod text_sanitize;
2122
#[cfg(feature = "cli")]
2223
pub mod config_override;
2324

25+
pub use ansi::{
26+
colors as ansi_colors, maybe_color, maybe_color_stderr, should_colorize,
27+
should_colorize_stderr, strip_ansi_codes,
28+
};
2429
pub use approval_presets::*;
2530
pub use config_substitution::{
2631
ConfigSubstitution, SubstitutionError, SubstitutionParseError, substitute_toml_string,

cortex-engine/src/tools/handlers/local_shell.rs

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,28 @@ const SHELL_METACHAR_PATTERNS: &[&str] = &[
2929
"~", "!", "#", "&",
3030
];
3131

32+
/// Patterns that indicate bash-specific syntax not supported by dash/sh.
33+
/// When detected, we prefer bash over /bin/sh to avoid compatibility issues (#2808).
34+
const BASH_SPECIFIC_PATTERNS: &[&str] = &[
35+
"[[", // Bash conditional expressions
36+
"]]", // Bash conditional expressions (closing)
37+
"<(", // Process substitution
38+
">(", // Process substitution
39+
"${!", // Indirect expansion
40+
"${#", // String length
41+
"**", // Bash exponentiation (2**10)
42+
"source ", // Bash-specific (POSIX uses '.')
43+
"shopt", // Bash-specific shell options
44+
"declare", // Bash-specific variable declaration
45+
"local ", // While POSIX has local, some sh don't
46+
"typeset", // Bash/ksh specific
47+
"+=", // Append assignment
48+
";&", // Bash case fall-through
49+
";;&", // Bash case pattern testing
50+
"&>>", // Bash append redirect both streams
51+
"|&", // Bash pipe both streams
52+
];
53+
3254
/// Handler for local_shell tool.
3355
pub struct LocalShellHandler;
3456

@@ -81,7 +103,30 @@ impl LocalShellHandler {
81103
}
82104
#[cfg(not(target_os = "windows"))]
83105
{
84-
vec!["/bin/sh".to_string(), "-c".to_string(), full_cmd]
106+
// Detect if bash-specific syntax is used (#2808)
107+
// On Ubuntu and similar systems, /bin/sh is dash which doesn't support
108+
// bash-specific features like [[, <(), **, source, etc.
109+
let needs_bash = args.command.iter().any(|arg| {
110+
BASH_SPECIFIC_PATTERNS
111+
.iter()
112+
.any(|pattern| arg.contains(pattern))
113+
});
114+
115+
if needs_bash {
116+
// Use bash explicitly for bash-specific syntax
117+
// Try /bin/bash first, fall back to bash in PATH
118+
let bash_path = if std::path::Path::new("/bin/bash").exists() {
119+
"/bin/bash".to_string()
120+
} else if std::path::Path::new("/usr/bin/bash").exists() {
121+
"/usr/bin/bash".to_string()
122+
} else {
123+
"bash".to_string()
124+
};
125+
vec![bash_path, "-c".to_string(), full_cmd]
126+
} else {
127+
// Use POSIX sh for simple shell commands
128+
vec!["/bin/sh".to_string(), "-c".to_string(), full_cmd]
129+
}
85130
}
86131
} else {
87132
// Direct execution without shell - arguments are passed safely as array

cortex-tui/src/app.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -601,6 +601,8 @@ pub struct AppState {
601601
pub session_history: Vec<SessionSummary>,
602602
pub terminal_size: (u16, u16),
603603
pub running: bool,
604+
/// Flag to request clearing terminal scrollback buffer (#2817)
605+
pub pending_scrollback_clear: bool,
604606
pub last_ctrl_c: Option<Instant>,
605607
pub last_esc: Option<Instant>,
606608
pub active_modal: Option<ActiveModal>,
@@ -734,6 +736,7 @@ impl AppState {
734736
session_history: Vec::new(),
735737
terminal_size: (80, 24),
736738
running: true,
739+
pending_scrollback_clear: false,
737740
last_ctrl_c: None,
738741
last_esc: None,
739742
active_modal: None,
@@ -864,10 +867,19 @@ impl AppState {
864867
self.messages.last_mut()
865868
}
866869

867-
/// Clear all messages
870+
/// Clear all messages and request terminal scrollback clear (#2817)
868871
pub fn clear_messages(&mut self) {
869872
self.messages.clear();
870873
self.chat_scroll = 0;
874+
// Request clearing terminal scrollback buffer for privacy
875+
self.pending_scrollback_clear = true;
876+
}
877+
878+
/// Check if scrollback clear is pending and reset the flag
879+
pub fn take_pending_scrollback_clear(&mut self) -> bool {
880+
let pending = self.pending_scrollback_clear;
881+
self.pending_scrollback_clear = false;
882+
pending
871883
}
872884

873885
/// Set the focus target

cortex-tui/src/commands/executor.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ impl CommandExecutor {
7676
"version" | "v" => self.cmd_version(),
7777
"upgrade" | "update" => CommandResult::OpenModal(ModalType::Upgrade),
7878
"settings" | "config" | "prefs" => CommandResult::OpenModal(ModalType::Settings),
79+
"reload-config" | "reload" => CommandResult::Async("config:reload".to_string()),
7980
"theme" => self.cmd_theme(cmd),
8081
"compact" => CommandResult::Toggle("compact".to_string()),
8182
"palette" | "cmd" => CommandResult::OpenModal(ModalType::CommandPalette),

cortex-tui/src/commands/registry.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,16 @@ pub fn register_builtin_commands(registry: &mut CommandRegistry) {
206206
false,
207207
));
208208

209+
// Config reload command (#2806)
210+
registry.register(CommandDef::new(
211+
"reload-config",
212+
&["reload"],
213+
"Reload configuration from disk",
214+
"/reload-config",
215+
CommandCategory::General,
216+
false,
217+
));
218+
209219
registry.register(CommandDef::new(
210220
"copy",
211221
&["cp"],

0 commit comments

Comments
 (0)