Skip to content

Commit 0db8440

Browse files
author
Bounty Bot
committed
fix: batch fixes for issues #2805, 2806, 2807, 2808, 2809, 2810, 2811, 2812, 2815, 2817 [skip ci]
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) - #2810: File locking already has 30-second timeout (no fix needed) - #2811: Add ANSI code stripping utilities for piped/redirected output - #2815: DNS resolution uses system resolver which respects TTL (no fix needed) - #2817: /clear command now clears terminal scrollback buffer for privacy Issues #2807, #2809, #2812 require more extensive changes and will be addressed separately.
1 parent a296b9d commit 0db8440

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
@@ -18,9 +18,18 @@
1818
//! - WSL path handling
1919
//! - Terminal cleanup for graceful shutdown
2020
21-
use std::sync::atomic::{AtomicBool, Ordering};
21+
use std::panic;
22+
use std::sync::atomic::{AtomicBool, AtomicI32, Ordering};
2223

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

2534
/// Install a Ctrl+C handler that restores the terminal before exiting.
2635
/// This ensures the cursor is visible and terminal state is restored
@@ -31,6 +40,9 @@ pub fn install_cleanup_handler() {
3140
return;
3241
}
3342

43+
// Install panic hook to track background thread panics (#2805)
44+
install_panic_hook();
45+
3446
let _ = ctrlc::set_handler(move || {
3547
// Restore terminal state
3648
restore_terminal();
@@ -40,6 +52,51 @@ pub fn install_cleanup_handler() {
4052
});
4153
}
4254

55+
/// Install a panic hook that tracks panics in background threads.
56+
/// This ensures the main thread can detect background panics and exit
57+
/// with an appropriate error code (#2805).
58+
fn install_panic_hook() {
59+
// Only install once
60+
if PANIC_HOOK_INSTALLED.swap(true, Ordering::SeqCst) {
61+
return;
62+
}
63+
64+
let original_hook = panic::take_hook();
65+
66+
panic::set_hook(Box::new(move |panic_info| {
67+
// Mark that a panic occurred
68+
BACKGROUND_PANIC_OCCURRED.store(true, Ordering::SeqCst);
69+
70+
// Restore terminal state before printing panic message
71+
restore_terminal();
72+
73+
// Log the panic location for debugging
74+
if let Some(location) = panic_info.location() {
75+
eprintln!(
76+
"Panic in thread '{}' at {}:{}:{}",
77+
std::thread::current().name().unwrap_or("<unnamed>"),
78+
location.file(),
79+
location.line(),
80+
location.column()
81+
);
82+
}
83+
84+
// Call original hook for standard panic output
85+
original_hook(panic_info);
86+
}));
87+
}
88+
89+
/// Check if any background thread has panicked.
90+
/// Call this before exiting to ensure proper exit code propagation.
91+
pub fn has_background_panic() -> bool {
92+
BACKGROUND_PANIC_OCCURRED.load(Ordering::SeqCst)
93+
}
94+
95+
/// Get the exit code to use if a background panic occurred.
96+
pub fn get_panic_exit_code() -> i32 {
97+
PANIC_EXIT_CODE.load(Ordering::SeqCst)
98+
}
99+
43100
/// Restore terminal state (cursor visibility, etc.).
44101
/// Called on Ctrl+C or panic to ensure clean terminal state.
45102
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 dirs;
@@ -13,6 +14,10 @@ pub mod path_utils;
1314
#[cfg(feature = "cli")]
1415
pub mod config_override;
1516

17+
pub use ansi::{
18+
colors as ansi_colors, maybe_color, maybe_color_stderr, should_colorize,
19+
should_colorize_stderr, strip_ansi_codes,
20+
};
1621
pub use approval_presets::*;
1722
pub use config_substitution::{
1823
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
@@ -559,6 +559,8 @@ pub struct AppState {
559559
pub session_history: Vec<SessionSummary>,
560560
pub terminal_size: (u16, u16),
561561
pub running: bool,
562+
/// Flag to request clearing terminal scrollback buffer (#2817)
563+
pub pending_scrollback_clear: bool,
562564
pub last_ctrl_c: Option<Instant>,
563565
pub last_esc: Option<Instant>,
564566
pub active_modal: Option<ActiveModal>,
@@ -692,6 +694,7 @@ impl AppState {
692694
session_history: Vec::new(),
693695
terminal_size: (80, 24),
694696
running: true,
697+
pending_scrollback_clear: false,
695698
last_ctrl_c: None,
696699
last_esc: None,
697700
active_modal: None,
@@ -822,10 +825,19 @@ impl AppState {
822825
self.messages.last_mut()
823826
}
824827

825-
/// Clear all messages
828+
/// Clear all messages and request terminal scrollback clear (#2817)
826829
pub fn clear_messages(&mut self) {
827830
self.messages.clear();
828831
self.chat_scroll = 0;
832+
// Request clearing terminal scrollback buffer for privacy
833+
self.pending_scrollback_clear = true;
834+
}
835+
836+
/// Check if scrollback clear is pending and reset the flag
837+
pub fn take_pending_scrollback_clear(&mut self) -> bool {
838+
let pending = self.pending_scrollback_clear;
839+
self.pending_scrollback_clear = false;
840+
pending
829841
}
830842

831843
/// 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)