mirror of
https://github.com/codewhale-hq/Codewhale.git
synced 2026-11-11 01:51:34 -10:00
fix(tui): harden /cache, /stash, /config and session prune edge cases
- session prune: a max_age too large for chrono (from_std failure or
DateTime underflow) now returns Ok(0) and keeps every session instead
of panicking or silently falling back to a 10-year horizon.
- /cache: `inspect` matches only as the whole word or `inspect <flags>`
(so `/cache inspector` no longer runs inspect), and an unknown
non-numeric argument (e.g. `/cache stat`) is a usage error instead of
silently showing the default 10-turn view.
- /stash list: entries are numbered from 1 via a pure format_stash_line.
- /config <unknown>: suggests the nearest key from
Settings::available_settings() ("Did you mean `/config <key>`?") and
points at `/settings text`, since `/help config` never listed keys.
best_suggestion_score in commands/mod.rs is now pub(crate) for reuse.
No tracking issue found for these; no Refs.
Checks: cargo test -p codewhale-tui --lib -- commands:: session_manager::
-> 1204 passed, 0 failed, 1 ignored (includes 6 new tests).
Full `npm test && npm run check:web` gate not run (targeted lane).
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01TvKGZz4LrKfrQiAXip48JS
This commit is contained in:
1 parent
afb96ba079
commit
b9a3f14dc8
5 files changed
+154
-15
No files matched your search
@@ -523,9 +523,25 @@ fn show_single_setting(app: &App, key: &str) -> CommandResult {
|
||||
};
|
||||
match value {
|
||||
Some(v) => CommandResult::message(format!("{key} = {v}")),
|
||||
None => CommandResult::error(format!(
|
||||
"Unknown setting '{key}'. See `/help config` for available settings."
|
||||
)),
|
||||
None => CommandResult::error(unknown_setting_message(&key)),
|
||||
}
|
||||
}
|
||||
|
||||
/// Error for `/config <key>` when `key` is not a known setting: name the
|
||||
/// closest real key when there is one, and point at the full list.
|
||||
fn unknown_setting_message(key: &str) -> String {
|
||||
let nearest = Settings::available_settings()
|
||||
.into_iter()
|
||||
.filter_map(|(candidate, _)| {
|
||||
crate::commands::best_suggestion_score(key, [candidate]).map(|score| (score, candidate))
|
||||
})
|
||||
.min_by_key(|(score, _)| *score)
|
||||
.map(|(_, candidate)| candidate);
|
||||
match nearest {
|
||||
Some(candidate) => format!(
|
||||
"Unknown setting '{key}'. Did you mean `/config {candidate}`? Run `/settings text` to list every setting."
|
||||
),
|
||||
None => format!("Unknown setting '{key}'. Run `/settings text` to list every setting."),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3540,6 +3556,25 @@ mod tests {
|
||||
assert!(rejected.is_error, "/inline takes no argument");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn config_unknown_setting_suggests_nearest_key() {
|
||||
let mut app = create_test_app();
|
||||
let result = config_command(&mut app, Some("auto_compcat"));
|
||||
assert!(result.is_error);
|
||||
let text = result.message.as_deref().unwrap_or_default();
|
||||
assert!(
|
||||
text.contains("Did you mean `/config auto_compact`?"),
|
||||
"{text}"
|
||||
);
|
||||
assert!(text.contains("/settings text"), "{text}");
|
||||
|
||||
let result = config_command(&mut app, Some("zzqqxxyy"));
|
||||
assert!(result.is_error);
|
||||
let text = result.message.as_deref().unwrap_or_default();
|
||||
assert!(!text.contains("Did you mean"), "{text}");
|
||||
assert!(text.contains("/settings text"), "{text}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn config_workflow_and_goal_explain_the_effective_tables() {
|
||||
let mut app = create_test_app();
|
||||
|
||||
@@ -61,13 +61,7 @@ fn list() -> CommandResult {
|
||||
let mut out = String::new();
|
||||
out.push_str(&format!("{} parked draft(s):\n\n", entries.len()));
|
||||
for (idx, entry) in entries.iter().enumerate() {
|
||||
let preview = preview_first_line(&entry.text, 80);
|
||||
let ts = if entry.ts.is_empty() {
|
||||
"(no ts)".to_string()
|
||||
} else {
|
||||
entry.ts.clone()
|
||||
};
|
||||
out.push_str(&format!(" {idx}. [{ts}] {preview}\n"));
|
||||
out.push_str(&format_stash_line(idx, &entry.ts, &entry.text));
|
||||
}
|
||||
out.push_str("\nUse `/stash pop` to restore the most recent draft.");
|
||||
CommandResult::message(out)
|
||||
@@ -110,6 +104,13 @@ fn pop(app: &mut App) -> CommandResult {
|
||||
/// Take a one-line preview of `text`, capped at `max_chars`.
|
||||
/// Multi-line drafts get a single-line summary so the listing
|
||||
/// stays scannable.
|
||||
/// One `/stash list` row. `idx` is the 0-based position; users see 1-based.
|
||||
fn format_stash_line(idx: usize, ts: &str, text: &str) -> String {
|
||||
let ts = if ts.is_empty() { "(no ts)" } else { ts };
|
||||
let preview = preview_first_line(text, 80);
|
||||
format!(" {}. [{ts}] {preview}\n", idx + 1)
|
||||
}
|
||||
|
||||
fn preview_first_line(text: &str, max_chars: usize) -> String {
|
||||
let head = text.lines().next().unwrap_or("").trim();
|
||||
if head.chars().count() <= max_chars {
|
||||
@@ -124,6 +125,15 @@ fn preview_first_line(text: &str, max_chars: usize) -> String {
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn stash_list_numbers_entries_from_one() {
|
||||
assert_eq!(
|
||||
format_stash_line(0, "2026-09-22T10:00:00Z", "first draft\nmore"),
|
||||
" 1. [2026-09-22T10:00:00Z] first draft\n"
|
||||
);
|
||||
assert_eq!(format_stash_line(2, "", "third"), " 3. [(no ts)] third\n");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preview_first_line_truncates_to_cap() {
|
||||
let body = "x".repeat(200);
|
||||
|
||||
@@ -14,7 +14,15 @@ use codewhale_models::MessageRequest;
|
||||
/// Renders a fixed-width table the user can paste into a bug report.
|
||||
pub fn cache(app: &mut App, arg: Option<&str>) -> CommandResult {
|
||||
let arg = arg.map(str::trim).filter(|s| !s.is_empty());
|
||||
if let Some(flags) = arg.and_then(|a| a.strip_prefix("inspect")) {
|
||||
let inspect_flags = arg.and_then(|a| {
|
||||
if a == "inspect" {
|
||||
Some("")
|
||||
} else {
|
||||
a.strip_prefix("inspect")
|
||||
.filter(|rest| rest.starts_with(char::is_whitespace))
|
||||
}
|
||||
});
|
||||
if let Some(flags) = inspect_flags {
|
||||
let flags = flags.trim();
|
||||
let verbose = flags.split_whitespace().any(|flag| flag == "--verbose");
|
||||
let json_mode = flags.split_whitespace().any(|flag| flag == "--json");
|
||||
@@ -30,7 +38,17 @@ pub fn cache(app: &mut App, arg: Option<&str>) -> CommandResult {
|
||||
return CommandResult::message(format_cache_zones(app));
|
||||
}
|
||||
|
||||
let want = arg.and_then(|s| s.parse::<usize>().ok()).unwrap_or(10);
|
||||
let want = match arg {
|
||||
None => 10,
|
||||
Some(raw) => match raw.parse::<usize>() {
|
||||
Ok(n) => n,
|
||||
Err(_) => {
|
||||
return CommandResult::error(format!(
|
||||
"Unknown /cache argument `{raw}`. Usage: /cache [count|inspect [--verbose|--json]|stats|zones|warmup]"
|
||||
));
|
||||
}
|
||||
},
|
||||
};
|
||||
let cap = app.session.turn_cache_history.len();
|
||||
let count = want
|
||||
.min(cap)
|
||||
@@ -901,3 +919,52 @@ Cache Zones (#2264 three-zone contract)
|
||||
assert_eq!(format_cache_zones(&app), expected);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod arg_tests {
|
||||
use super::*;
|
||||
use crate::config::Config;
|
||||
use std::path::PathBuf;
|
||||
|
||||
fn app() -> App {
|
||||
App::new(
|
||||
crate::test_support::test_tui_options(PathBuf::from(".")),
|
||||
&Config::default(),
|
||||
)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cache_rejects_unknown_word_args() {
|
||||
let mut app = app();
|
||||
for arg in ["stat", "inspector", "inspect--json"] {
|
||||
let result = cache(&mut app, Some(arg));
|
||||
assert!(result.is_error, "/cache {arg} must be a usage error");
|
||||
let text = result.message.as_deref().unwrap_or_default();
|
||||
assert!(
|
||||
text.contains(arg) && text.contains("Usage: /cache"),
|
||||
"{text}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cache_inspect_matches_whole_word_with_optional_flags() {
|
||||
let mut app = app();
|
||||
for arg in ["inspect", "inspect --json", "inspect --verbose"] {
|
||||
let result = cache(&mut app, Some(arg));
|
||||
let text = result.message.as_deref().unwrap_or_default();
|
||||
assert!(!result.is_error, "/cache {arg}: {text}");
|
||||
assert!(
|
||||
!text.contains("Unknown /cache argument"),
|
||||
"/cache {arg}: {text}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cache_numeric_arg_still_selects_count() {
|
||||
let mut app = app();
|
||||
let result = cache(&mut app, Some("5"));
|
||||
assert!(!result.is_error);
|
||||
}
|
||||
}
|
||||
@@ -405,7 +405,7 @@ fn edit_distance(a: &str, b: &str) -> usize {
|
||||
previous[b_chars.len()]
|
||||
}
|
||||
|
||||
fn best_suggestion_score<'a>(
|
||||
pub(crate) fn best_suggestion_score<'a>(
|
||||
query: &str,
|
||||
candidates: impl IntoIterator<Item = &'a str>,
|
||||
) -> Option<(u8, usize)> {
|
||||
|
||||
@@ -2889,8 +2889,15 @@ impl SessionManager {
|
||||
max_age: std::time::Duration,
|
||||
keep: Option<&str>,
|
||||
) -> std::io::Result<usize> {
|
||||
let cutoff = Utc::now()
|
||||
- chrono::Duration::from_std(max_age).unwrap_or(chrono::Duration::days(365 * 10));
|
||||
// A max_age too large to represent (or to subtract from now) means
|
||||
// nothing can be old enough to prune — keep everything instead of
|
||||
// panicking on DateTime underflow or inventing a fallback horizon.
|
||||
let Some(cutoff) = chrono::Duration::from_std(max_age)
|
||||
.ok()
|
||||
.and_then(|age| Utc::now().checked_sub_signed(age))
|
||||
else {
|
||||
return Ok(0);
|
||||
};
|
||||
let sessions = self.list_sessions()?;
|
||||
let mut pruned = 0usize;
|
||||
for session in sessions {
|
||||
@@ -7610,6 +7617,26 @@ mod tests {
|
||||
assert_eq!(manager.list_sessions().expect("list").len(), 2);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn prune_sessions_older_than_huge_max_age_keeps_everything() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
let manager = SessionManager::new(tmp.path().join("sessions")).expect("new");
|
||||
write_session_with_updated_at(&manager, "old", Utc::now() - chrono::Duration::days(3650));
|
||||
write_session_with_updated_at(&manager, "new", Utc::now());
|
||||
// Both overflow paths: from_std rejects u64::MAX seconds, and a
|
||||
// representable-but-enormous age underflows the DateTime subtraction.
|
||||
for max_age in [
|
||||
std::time::Duration::MAX,
|
||||
std::time::Duration::from_secs(i64::MAX as u64 / 1_000),
|
||||
] {
|
||||
let pruned = manager
|
||||
.prune_sessions_older_than(max_age)
|
||||
.expect("huge max_age must not error or panic");
|
||||
assert_eq!(pruned, 0, "{max_age:?}");
|
||||
assert_eq!(manager.list_sessions().expect("list").len(), 2);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn prune_sessions_older_than_removes_stale_records() {
|
||||
let tmp = tempdir().expect("tempdir");
|
||||
|
||||
Reference in new issue
Block a user