mirror of
https://github.com/codewhale-hq/Codewhale.git
synced 2026-11-11 01:51:34 -10:00
fix(tui): stop the trust screen from cutting its own question in half
At 40x12 the workspace-trust screen rendered:
| Should Codewhale work with the instruc|
| Workspace /private/tmp/claude-501/|
| -Volumes-VIXinSSD-CW-codewhale/|
The question is severed mid-word with nothing marking the cut, and the path
below it wraps correctly — because the path goes through
`wrap_on_path_separators` while the prose went through nothing at all. The
question, the prompt-injection risk hint, and the trust-effect hint were each
pushed as a single `Line::from(Span::styled(...))` and left for the terminal to
clip.
`welcome.rs` and `language.rs` both wrap their prose with `super::wrap_words`.
`trust_directory.rs` was the one onboarding screen that did not, and it is the
screen that asks for filesystem trust. Someone is being asked to authorise an
agent over their files while the sentence explaining what they are authorising
is cut off. That is the worst place in the product for this bug to live.
All three strings now go through the same helper, which also means they inherit
its handling of scripts that do not put spaces between words — Japanese and
Chinese wrap by display width on grapheme clusters instead of overflowing.
| Should Codewhale work with the|
| instructions and files in this|
| workspace?|
`every_locale_keeps_the_trust_prose_whole_at_forty_columns` walks all fifteen
shipped locales at 40/60/80/120, asserts no row exceeds the lane, and asserts
the question survives whole. It compares with whitespace removed rather than
collapsed: Japanese wraps between characters that have no space between them,
so rejoining with a space would make a correctly wrapped line look like a
changed one. Confirmed the test fails with the wrapping reverted.
`prompt_names_the_workspace_boundary_and_effects` asserted `contains("prompt
injection")` against lines joined with newlines, which wrapping now splits at
70 columns. The fact it protects — that the screen names prompt injection — is
unchanged, so the assertion is matched on collapsed whitespace instead of
being narrowed or deleted.
Known limit, not fixed here: at twelve rows the risk and effect hints do not
fit alongside the question and the three actions, so they are dropped. Showing
the complete question is the better trade at that size, but a trust screen that
cannot state its risk is worth revisiting.
Verified: cargo test -p codewhale-tui --lib -> 10885 passed, 0 failed,
13 ignored. Frames above captured from the built binary.
Implemented with agent assistance.
Signed-off-by: Hunter Bown <[email protected]>
This commit is contained in:
1 parent
ad921888e1
commit
777cbd592a
3 files changed
+119
-16
No files matched your search
@@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
- The workspace-trust screen no longer cuts its own question in half on a small
|
||||
terminal. The question, the prompt-injection risk hint, and the trust-effect
|
||||
hint were each pushed as one unwrapped line, so at 40 columns the screen read
|
||||
"Should Codewhale work with the instruc" — severed mid-word with nothing
|
||||
marking the cut, while the workspace path directly beneath it wrapped
|
||||
correctly. Asking someone to grant filesystem trust while the question itself
|
||||
is truncated is the worst place in the product for that to happen. All three
|
||||
now wrap through the same helper the rest of onboarding uses, which also
|
||||
means they wrap correctly in Japanese and Chinese. Verified across all
|
||||
fifteen shipped locales at 40, 60, 80 and 120 columns.
|
||||
|
||||
- The bottom status rail is no longer one run-on sentence. At 120 columns it
|
||||
read `▌· idle · Ollama · deepseek-v4-flash · max · Anonymous usage counts are
|
||||
on. … ⌥V:output · /context:context · fn+F1:keys` — live state, route
|
||||
|
||||
@@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
- The workspace-trust screen no longer cuts its own question in half on a small
|
||||
terminal. The question, the prompt-injection risk hint, and the trust-effect
|
||||
hint were each pushed as one unwrapped line, so at 40 columns the screen read
|
||||
"Should Codewhale work with the instruc" — severed mid-word with nothing
|
||||
marking the cut, while the workspace path directly beneath it wrapped
|
||||
correctly. Asking someone to grant filesystem trust while the question itself
|
||||
is truncated is the worst place in the product for that to happen. All three
|
||||
now wrap through the same helper the rest of onboarding uses, which also
|
||||
means they wrap correctly in Japanese and Chinese. Verified across all
|
||||
fifteen shipped locales at 40, 60, 80 and 120 columns.
|
||||
|
||||
- The bottom status rail is no longer one run-on sentence. At 120 columns it
|
||||
read `▌· idle · Ollama · deepseek-v4-flash · max · Anonymous usage counts are
|
||||
on. … ⌥V:output · /context:context · fn+F1:keys` — live state, route
|
||||
|
||||
@@ -57,10 +57,21 @@ pub fn lines(app: &App, content_width: usize) -> Vec<Line<'static>> {
|
||||
.add_modifier(Modifier::BOLD),
|
||||
)));
|
||||
lines.push(Line::from(""));
|
||||
lines.push(Line::from(Span::styled(
|
||||
app.tr(MessageId::OnboardTrustQuestion).to_string(),
|
||||
Style::default().fg(palette::TEXT_PRIMARY),
|
||||
)));
|
||||
// Prose on this screen wraps like prose on every other onboarding screen.
|
||||
// It used to be pushed as one unwrapped line, so at 40 columns the trust
|
||||
// question rendered as "Should Codewhale work with the instruc" — severed
|
||||
// mid-word, with nothing marking the cut. Asking someone to grant
|
||||
// filesystem trust while the question itself is truncated is the worst
|
||||
// place in the product for this to happen.
|
||||
for segment in super::wrap_words(
|
||||
app.tr(MessageId::OnboardTrustQuestion).as_ref(),
|
||||
content_width,
|
||||
) {
|
||||
lines.push(Line::from(Span::styled(
|
||||
segment,
|
||||
Style::default().fg(palette::TEXT_PRIMARY),
|
||||
)));
|
||||
}
|
||||
let location = format!(
|
||||
"{}{}",
|
||||
app.tr(MessageId::OnboardTrustLocationPrefix),
|
||||
@@ -73,14 +84,17 @@ pub fn lines(app: &App, content_width: usize) -> Vec<Line<'static>> {
|
||||
)));
|
||||
}
|
||||
lines.push(Line::from(""));
|
||||
lines.push(Line::from(Span::styled(
|
||||
app.tr(MessageId::OnboardTrustRiskHint).to_string(),
|
||||
Style::default().fg(palette::TEXT_MUTED),
|
||||
)));
|
||||
lines.push(Line::from(Span::styled(
|
||||
app.tr(MessageId::OnboardTrustEffectHint).to_string(),
|
||||
Style::default().fg(palette::TEXT_MUTED),
|
||||
)));
|
||||
for id in [
|
||||
MessageId::OnboardTrustRiskHint,
|
||||
MessageId::OnboardTrustEffectHint,
|
||||
] {
|
||||
for segment in super::wrap_words(app.tr(id).as_ref(), content_width) {
|
||||
lines.push(Line::from(Span::styled(
|
||||
segment,
|
||||
Style::default().fg(palette::TEXT_MUTED),
|
||||
)));
|
||||
}
|
||||
}
|
||||
if let Some(message) = app.status_message.as_deref() {
|
||||
lines.push(Line::from(""));
|
||||
lines.push(Line::from(Span::styled(
|
||||
@@ -112,11 +126,15 @@ mod tests {
|
||||
.flat_map(|line| line.spans.into_iter().map(|span| span.content.to_string()))
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
// The prose wraps, so a phrase can straddle a line break. Match on
|
||||
// collapsed whitespace: this test is about which facts the screen
|
||||
// states, not about where the lane happens to break them.
|
||||
let flat = body.split_whitespace().collect::<Vec<_>>().join(" ");
|
||||
|
||||
assert!(body.contains("Know this workspace"));
|
||||
assert!(body.contains("instructions and files"));
|
||||
assert!(body.contains("prompt injection"));
|
||||
assert!(body.contains("tools and hooks"));
|
||||
assert!(flat.contains("Know this workspace"), "{body}");
|
||||
assert!(flat.contains("instructions and files"), "{body}");
|
||||
assert!(flat.contains("prompt injection"), "{body}");
|
||||
assert!(flat.contains("tools and hooks"), "{body}");
|
||||
}
|
||||
|
||||
/// Trust keys stay explicit and in the action rail: Enter must never
|
||||
@@ -156,3 +174,66 @@ mod tests {
|
||||
assert!(rail.contains("quit Codewhale"), "{rail}");
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod narrow_terminal_tests {
|
||||
use super::*;
|
||||
use crate::config::Config;
|
||||
use crate::localization::{Locale, MessageId, tr};
|
||||
use crate::tui::app::TuiOptions;
|
||||
use std::path::PathBuf;
|
||||
use unicode_width::UnicodeWidthStr;
|
||||
|
||||
fn app_at(workspace: &str) -> App {
|
||||
let options = TuiOptions {
|
||||
model: "test-model".to_string(),
|
||||
..crate::test_support::test_tui_options(PathBuf::from(workspace))
|
||||
};
|
||||
App::new(options, &Config::default())
|
||||
}
|
||||
|
||||
/// The trust screen asks for filesystem trust. Every locale's prose has to
|
||||
/// survive a small terminal intact: a question cut mid-word is not a
|
||||
/// question the reader can answer.
|
||||
#[test]
|
||||
fn every_locale_keeps_the_trust_prose_whole_at_forty_columns() {
|
||||
let mut app = app_at("/tmp/probe/ws");
|
||||
for locale in Locale::shipped().iter().copied() {
|
||||
app.ui_locale = locale;
|
||||
for width in [40usize, 60, 80, 120] {
|
||||
let rendered: Vec<String> = lines(&app, width)
|
||||
.iter()
|
||||
.map(|line| {
|
||||
line.spans
|
||||
.iter()
|
||||
.map(|span| span.content.as_ref())
|
||||
.collect::<String>()
|
||||
})
|
||||
.collect();
|
||||
|
||||
for row in &rendered {
|
||||
assert!(
|
||||
row.width() <= width,
|
||||
"{locale:?} at {width}: row overflows the lane: {row:?}",
|
||||
);
|
||||
}
|
||||
|
||||
// The question must be present in full, not clipped. Compare on
|
||||
// the rejoined prose so a wrap is fine and a cut is not.
|
||||
let joined = rendered.join(" ");
|
||||
let question = tr(locale, MessageId::OnboardTrustQuestion);
|
||||
// Compare with whitespace removed entirely, not collapsed.
|
||||
// Japanese and Chinese wrap between characters that have no
|
||||
// space between them in the source, so rejoining with a space
|
||||
// would make a correctly wrapped line look like a changed one.
|
||||
let normalize =
|
||||
|s: &str| s.chars().filter(|c| !c.is_whitespace()).collect::<String>();
|
||||
assert!(
|
||||
normalize(&joined).contains(&normalize(question.as_ref()))
|
||||
|| question.as_ref().chars().all(|c| c.is_whitespace()),
|
||||
"{locale:?} at {width}: the trust question was cut.\nwanted: {question}\ngot: {joined}",
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user