dgf1988/codewhale
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Merge pull request #3176 from Hmbown/scratch/v0.8.59-release-finish-gpt55-20260612

Browse Source 
fix(release): harden v0.8.59 terminal and file stability
This commit is contained in:
Hunter Bown
2026-06-12 13:29:34 -07:00
committed by GitHub
parent a448194b81 d921b5670d
commit 62e7f0e686
5 changed files with 242 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/tui/src/child_env.rs
+63
View File
@@ -35,6 +35,8 @@ where
value.as_ref().to_os_string(),
);
}
#[cfg(windows)]
fill_windows_common_program_files(&mut env);
env
}
@@ -184,6 +186,9 @@ fn is_allowed_parent_env_key(key: &OsStr) -> bool {
| "PROGRAMFILES"
| "PROGRAMFILES(X86)"
| "PROGRAMW6432"
| "COMMONPROGRAMFILES"
| "COMMONPROGRAMFILES(X86)"
| "COMMONPROGRAMW6432"
| "PROCESSOR_ARCHITECTURE"
| "NUGET_PACKAGES"
| "NUGET_HTTP_CACHE_PATH"
@@ -266,6 +271,26 @@ fn upsert_env(env: &mut Vec<(OsString, OsString)>, key: OsString, value: OsStrin
env.push((key, value));
}
#[cfg(any(windows, test))]
fn fill_windows_common_program_files(env: &mut Vec<(OsString, OsString)>) {
for (key, default) in [
("CommonProgramFiles", r"C:\Program Files\Common Files"),
(
"CommonProgramFiles(x86)",
r"C:\Program Files (x86)\Common Files",
),
("CommonProgramW6432", r"C:\Program Files\Common Files"),
] {
let existing = env
.iter()
.find(|(existing, _)| normalize_key(existing) == normalize_key(OsStr::new(key)))
.map(|(_, value)| value.to_string_lossy().trim().is_empty());
if existing.unwrap_or(true) {
upsert_env(env, OsString::from(key), OsString::from(default));
}
}
}
fn normalize_key(key: &OsStr) -> String {
key.to_string_lossy().to_ascii_uppercase()
}
@@ -387,6 +412,9 @@ mod tests {
"PROGRAMFILES",
"PROGRAMFILES(X86)",
"PROGRAMW6432",
"COMMONPROGRAMFILES",
"COMMONPROGRAMFILES(X86)",
"COMMONPROGRAMW6432",
"PROCESSOR_ARCHITECTURE",
"NUGET_PACKAGES",
"DOTNET_ROOT",
@@ -408,6 +436,41 @@ mod tests {
);
}
#[test]
fn windows_common_program_files_defaults_replace_empty_values() {
let mut env = vec![
(OsString::from("CommonProgramFiles"), OsString::new()),
(
OsString::from("CommonProgramFiles(x86)"),
OsString::from(" "),
),
(
OsString::from("CommonProgramW6432"),
OsString::from(r"D:\Common Files"),
),
];
fill_windows_common_program_files(&mut env);
let get = |name: &str| {
env.iter()
.find(|(key, _)| normalize_key(key) == normalize_key(OsStr::new(name)))
.map(|(_, value)| value.to_string_lossy().into_owned())
};
assert_eq!(
get("CommonProgramFiles").as_deref(),
Some(r"C:\Program Files\Common Files")
);
assert_eq!(
get("CommonProgramFiles(x86)").as_deref(),
Some(r"C:\Program Files (x86)\Common Files")
);
assert_eq!(
get("CommonProgramW6432").as_deref(),
Some(r"D:\Common Files")
);
}
#[test]
fn mcp_env_allowlist_includes_python_bootstrap_keys() {
for key in [
crates/tui/src/commands/groups/config/config.rs
+41
View File
@@ -658,6 +658,7 @@ pub fn set_config_value(app: &mut App, key: &str, value: &str, persist: bool) ->
if let Err(e) = settings.set(&key, value) {
return CommandResult::error(format!("{e}"));
}
settings.apply_env_overrides();
let mut action = None;
match key.as_str() {
@@ -835,6 +836,8 @@ pub fn set_config_value(app: &mut App, key: &str, value: &str, persist: bool) ->
},
),
"composer_vim_mode" | "vim_mode" | "vim" => settings.composer_vim_mode.clone(),
"low_motion" | "motion" => settings.low_motion.to_string(),
"fancy_animations" | "fancy" | "animations" => settings.fancy_animations.to_string(),
_ => value.to_string(),
};
@@ -1394,6 +1397,44 @@ mod tests {
);
}
#[test]
fn config_fancy_animations_obeys_ghostty_override() {
let temp_root = env::temp_dir().join(format!(
"codewhale-tui-ghostty-fancy-config-test-{}",
std::process::id()
));
fs::create_dir_all(&temp_root).unwrap();
let _guard = EnvGuard::new(&temp_root);
let prev_term_program = env::var_os("TERM_PROGRAM");
// Safety: test-only environment mutation guarded by EnvGuard's lock.
unsafe {
env::set_var("TERM_PROGRAM", "Ghostty");
}
let mut app = create_test_app();
assert!(!app.fancy_animations);
let result = set_config_value(&mut app, "fancy_animations", "true", false);
assert!(!result.is_error);
assert!(
!app.fancy_animations,
"Ghostty compatibility override must keep the water strip disabled"
);
assert_eq!(
result.message.as_deref(),
Some("fancy_animations = false (session only, add --save to persist)")
);
// Safety: cleanup under EnvGuard's lock.
unsafe {
match prev_term_program {
Some(v) => env::set_var("TERM_PROGRAM", v),
None => env::remove_var("TERM_PROGRAM"),
}
}
}
#[test]
fn config_model_accepts_future_deepseek_model_id() {
let mut app = create_test_app();
crates/tui/src/settings.rs
+61 -11
View File
@@ -456,18 +456,23 @@ impl Settings {
self.low_motion = true;
self.fancy_animations = false;
}
// VS Code (TERM_PROGRAM=vscode, #1356), Ghostty (TERM_PROGRAM=ghostty,
// #1445), and a few VTE terminals (#1470) produce visible flicker at
// 120 FPS. Drop to the 30 FPS low-motion cap for them automatically.
// VS Code (TERM_PROGRAM=vscode, #1356), Ghostty (#1445), and a few
// VTE terminals (#1470) produce visible flicker at 120 FPS. Drop to
// the 30 FPS low-motion cap for them automatically. Ghostty may report
// either TERM_PROGRAM=Ghostty/ghostty or TERM=xterm-ghostty.
// Like NO_ANIMATIONS above, this unconditionally overrides any
// disk-loaded value — consistent precedence: env signals always win.
let term_program = std::env::var("TERM_PROGRAM")
.unwrap_or_default()
.to_ascii_lowercase();
let term = std::env::var("TERM")
.unwrap_or_default()
.to_ascii_lowercase();
let term_forces_low_motion =
matches!(term_program.as_str(), "vscode" | "ghostty") || term.contains("ghostty");
let vte_env_forces_low_motion = std::env::var_os("TILIX_ID").is_some_and(|v| !v.is_empty())
|| std::env::var_os("TERMINATOR_UUID").is_some_and(|v| !v.is_empty());
if matches!(
std::env::var("TERM_PROGRAM").as_deref(),
Ok("vscode") | Ok("ghostty")
) || vte_env_forces_low_motion
{
if term_forces_low_motion || vte_env_forces_low_motion {
self.low_motion = true;
self.fancy_animations = false;
}
@@ -1673,6 +1678,7 @@ mod tests {
let prev_tmux = std::env::var_os("TMUX");
let prev_sty = std::env::var_os("STY");
let prev_term_program = std::env::var_os("TERM_PROGRAM");
let prev_term = std::env::var_os("TERM");
let prev_ssh_client = std::env::var_os("SSH_CLIENT");
let prev_ssh_tty = std::env::var_os("SSH_TTY");
let prev_tilix_id = std::env::var_os("TILIX_ID");
@@ -1690,6 +1696,7 @@ mod tests {
std::env::remove_var("TMUX");
std::env::remove_var("STY");
std::env::remove_var("TERM_PROGRAM");
std::env::remove_var("TERM");
std::env::remove_var("SSH_CLIENT");
std::env::remove_var("SSH_TTY");
std::env::remove_var("TILIX_ID");
@@ -1736,6 +1743,10 @@ mod tests {
Some(v) => std::env::set_var("TERM_PROGRAM", v),
None => std::env::remove_var("TERM_PROGRAM"),
}
match prev_term {
Some(v) => std::env::set_var("TERM", v),
None => std::env::remove_var("TERM"),
}
match prev_ssh_client {
Some(v) => std::env::set_var("SSH_CLIENT", v),
None => std::env::remove_var("SSH_CLIENT"),
@@ -1799,18 +1810,18 @@ mod tests {
let prev = std::env::var_os("TERM_PROGRAM");
// SAFETY: serialised by the guard.
unsafe {
std::env::set_var("TERM_PROGRAM", "ghostty");
std::env::set_var("TERM_PROGRAM", "Ghostty");
}
let mut settings = Settings::default();
assert!(!settings.low_motion, "default is animated");
settings.apply_env_overrides();
assert!(
settings.low_motion,
"TERM_PROGRAM=ghostty must enable low_motion to prevent flickering (#1445)"
"TERM_PROGRAM=Ghostty must enable low_motion to prevent flickering (#1445)"
);
assert!(
!settings.fancy_animations,
"TERM_PROGRAM=ghostty must disable fancy_animations"
"TERM_PROGRAM=Ghostty must disable fancy_animations"
);
// SAFETY: cleanup under the guard.
unsafe {
@@ -1821,10 +1832,44 @@ mod tests {
}
}
#[test]
fn ghostty_term_fallback_forces_low_motion_on() {
let _g = term_program_test_guard();
let prev_program = std::env::var_os("TERM_PROGRAM");
let prev_term = std::env::var_os("TERM");
// SAFETY: serialised by the guard.
unsafe {
std::env::remove_var("TERM_PROGRAM");
std::env::set_var("TERM", "xterm-ghostty");
}
let mut settings = Settings::default();
settings.apply_env_overrides();
assert!(
settings.low_motion,
"TERM=xterm-ghostty must enable low_motion when TERM_PROGRAM is absent"
);
assert!(
!settings.fancy_animations,
"TERM=xterm-ghostty must disable fancy_animations"
);
// SAFETY: cleanup under the guard.
unsafe {
match prev_program {
Some(v) => std::env::set_var("TERM_PROGRAM", v),
None => std::env::remove_var("TERM_PROGRAM"),
}
match prev_term {
Some(v) => std::env::set_var("TERM", v),
None => std::env::remove_var("TERM"),
}
}
}
#[test]
fn non_vscode_term_program_does_not_force_low_motion() {
let _g = term_program_test_guard();
let prev = std::env::var_os("TERM_PROGRAM");
let prev_term = std::env::var_os("TERM");
let prev_ssh_client = std::env::var_os("SSH_CLIENT");
let prev_ssh_tty = std::env::var_os("SSH_TTY");
let prev_tilix_id = std::env::var_os("TILIX_ID");
@@ -1838,6 +1883,7 @@ mod tests {
unsafe {
std::env::remove_var("SSH_CLIENT");
std::env::remove_var("SSH_TTY");
std::env::remove_var("TERM");
std::env::remove_var("TILIX_ID");
std::env::remove_var("TERMINATOR_UUID");
std::env::remove_var("TMUX");
@@ -1861,6 +1907,10 @@ mod tests {
Some(v) => std::env::set_var("TERM_PROGRAM", v),
None => std::env::remove_var("TERM_PROGRAM"),
}
match prev_term {
Some(v) => std::env::set_var("TERM", v),
None => std::env::remove_var("TERM"),
}
if let Some(v) = prev_ssh_client {
std::env::set_var("SSH_CLIENT", v);
}
crates/tui/src/tools/file.rs
+39 -2
View File
@@ -10,6 +10,7 @@ use super::spec::{
};
use async_trait::async_trait;
use serde_json::{Value, json};
use std::fmt::Display;
use std::fs;
use std::path::{Path, PathBuf};
use std::process::{Command, Stdio};
@@ -356,7 +357,7 @@ fn read_pdf_via_pdf_extract(
// pdf-extract returns pages in document order; `start`/`end` are
// 1-indexed inclusive (validated above), so we convert to a
// 0-indexed half-open slice with bounds clamping.
let pages = pdf_extract::extract_text_by_pages(path).map_err(|e| {
let pages = guard_pdf_extract(|| pdf_extract::extract_text_by_pages(path)).map_err(|e| {
ToolError::execution_failed(format!(
"pdf-extract failed on {}: {e} (set `prefer_external_pdftotext = true` in settings.toml to retry via pdftotext)",
path.display()
@@ -379,7 +380,7 @@ fn read_pdf_via_pdf_extract(
// extract_text uses an internal codepath that can hang on certain PDF
// cross-reference tables or font encodings (#2641). The per-page path
// avoids that hang and produces identical output when joined.
pdf_extract::extract_text_by_pages(path)
guard_pdf_extract(|| pdf_extract::extract_text_by_pages(path))
.map(|pages| pages.join("\n"))
.map_err(|e| {
ToolError::execution_failed(format!(
@@ -391,6 +392,31 @@ fn read_pdf_via_pdf_extract(
Ok(ToolResult::success(clean_pdf_text(&text)))
}
fn guard_pdf_extract<T, E, F>(extract: F) -> Result<T, String>
where
E: Display,
F: FnOnce() -> Result<T, E>,
{
match std::panic::catch_unwind(std::panic::AssertUnwindSafe(extract)) {
Ok(Ok(value)) => Ok(value),
Ok(Err(err)) => Err(err.to_string()),
Err(payload) => Err(format!(
"extractor panicked: {}",
panic_payload_message(payload.as_ref())
)),
}
}
fn panic_payload_message(payload: &(dyn std::any::Any + Send)) -> String {
if let Some(message) = payload.downcast_ref::<&str>() {
(*message).to_string()
} else if let Some(message) = payload.downcast_ref::<String>() {
message.clone()
} else {
"unknown panic".to_string()
}
}
fn read_pdf_via_pdftotext(
path: &Path,
page_range: Option<(u32, u32)>,
@@ -1380,6 +1406,17 @@ mod tests {
assert!(single.content.contains("Recursive Language Models"));
}
#[test]
fn pdf_extract_panic_is_returned_as_tool_error_text() {
let err = guard_pdf_extract(|| -> Result<String, &'static str> {
panic!("assertion failed: name == \"Identity-H\"");
})
.expect_err("panic should become an error");
assert!(err.contains("extractor panicked"));
assert!(err.contains("Identity-H"));
}
#[tokio::test]
async fn read_file_pdf_path_uses_pdf_extract_by_default() {
if !sample_pdf_present() {
crates/tui/src/tools/web_run.rs
+38 -1
View File
@@ -14,6 +14,7 @@ use regex::Regex;
use serde::{Deserialize, Serialize};
use serde_json::{Value, json};
use std::collections::{HashMap, VecDeque};
use std::fmt::Display;
use std::hash::{Hash, Hasher};
use std::sync::{Arc, OnceLock};
use std::time::{Duration, Instant};
@@ -1204,10 +1205,35 @@ fn parse_pdf_page(
}
fn pdf_extract_text(bytes: &[u8]) -> Result<String, ToolError> {
pdf_extract::extract_text_from_mem(bytes)
guard_pdf_extract(|| pdf_extract::extract_text_from_mem(bytes))
.map_err(|e| ToolError::execution_failed(format!("PDF extract failed: {e}")))
}
fn guard_pdf_extract<T, E, F>(extract: F) -> Result<T, String>
where
E: Display,
F: FnOnce() -> Result<T, E>,
{
match std::panic::catch_unwind(std::panic::AssertUnwindSafe(extract)) {
Ok(Ok(value)) => Ok(value),
Ok(Err(err)) => Err(err.to_string()),
Err(payload) => Err(format!(
"extractor panicked: {}",
panic_payload_message(payload.as_ref())
)),
}
}
fn panic_payload_message(payload: &(dyn std::any::Any + Send)) -> String {
if let Some(message) = payload.downcast_ref::<&str>() {
(*message).to_string()
} else if let Some(message) = payload.downcast_ref::<String>() {
message.clone()
} else {
"unknown panic".to_string()
}
}
fn split_pdf_pages(text: &str) -> Vec<Vec<String>> {
let raw_pages: Vec<&str> = text.split('\x0C').collect();
raw_pages
@@ -1794,6 +1820,17 @@ mod tests {
assert_eq!(percent_decode("foo+bar%20baz"), "foo+bar baz");
}
#[test]
fn pdf_extract_panic_is_returned_as_tool_error_text() {
let err = guard_pdf_extract(|| -> Result<String, &'static str> {
panic!("assertion failed: name == \"Identity-H\"");
})
.expect_err("panic should become an error");
assert!(err.contains("extractor panicked"));
assert!(err.contains("Identity-H"));
}
#[test]
fn scoped_ref_prefix_is_session_specific() {
let _lock = lock_web_run_test_state();