mirror of
https://github.com/hanzoai/dev.git
synced 2026-10-25 03:32:32 +00:00
fix(tui/update): harden guided upgrade flows
This commit is contained in:
@@ -1874,7 +1874,7 @@ impl App<'_> {
|
||||
SlashCommand::Plan | SlashCommand::Solve | SlashCommand::Code => {
|
||||
// These should have been expanded already, but handle them anyway
|
||||
if let AppState::Chat { widget } = &mut self.app_state {
|
||||
let expanded = command.expand_prompt(&command_text);
|
||||
let expanded = command.expand_prompt(&command_args);
|
||||
if let Some(prompt) = expanded {
|
||||
widget.submit_prompt_with_display(command_text.clone(), prompt);
|
||||
}
|
||||
|
||||
@@ -8655,7 +8655,7 @@ impl ChatWidget<'_> {
|
||||
crate::updates::UpgradeResolution::Command { command, display } => {
|
||||
if command.is_empty() {
|
||||
self.history_push(history_cell::new_error_event(
|
||||
"`/upgrade` — no upgrade command available for this install.".to_string(),
|
||||
"`/update` — no upgrade command available for this install.".to_string(),
|
||||
));
|
||||
self.request_redraw();
|
||||
return;
|
||||
@@ -10366,6 +10366,7 @@ impl ChatWidget<'_> {
|
||||
Some(cwd),
|
||||
controller,
|
||||
controller_rx,
|
||||
self.config.clone(),
|
||||
self.config.debug,
|
||||
);
|
||||
|
||||
@@ -10732,6 +10733,7 @@ impl ChatWidget<'_> {
|
||||
if pending_id == id {
|
||||
self.bottom_pane
|
||||
.flash_footer_notice(format!("Upgraded to {version}"));
|
||||
self.latest_upgrade_version = None;
|
||||
} else {
|
||||
self.pending_upgrade_notice = Some((pending_id, version));
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use std::sync::mpsc::{channel, Receiver};
|
||||
use std::sync::{Arc, Mutex};
|
||||
use std::time::Duration;
|
||||
|
||||
use anyhow::{anyhow, Context, Result};
|
||||
use codex_core::config::Config;
|
||||
@@ -72,6 +73,7 @@ pub(super) fn start_agent_install_session(
|
||||
cwd,
|
||||
controller,
|
||||
controller_rx,
|
||||
None,
|
||||
debug_enabled,
|
||||
);
|
||||
}
|
||||
@@ -92,6 +94,7 @@ pub(super) fn start_prompt_terminal_session(
|
||||
cwd,
|
||||
controller,
|
||||
controller_rx,
|
||||
None,
|
||||
debug_enabled,
|
||||
);
|
||||
}
|
||||
@@ -112,6 +115,7 @@ pub(super) fn start_direct_terminal_session(
|
||||
cwd,
|
||||
controller,
|
||||
controller_rx,
|
||||
None,
|
||||
debug_enabled,
|
||||
);
|
||||
}
|
||||
@@ -124,6 +128,7 @@ pub(super) fn start_upgrade_terminal_session(
|
||||
cwd: Option<String>,
|
||||
controller: TerminalRunController,
|
||||
controller_rx: Receiver<TerminalRunEvent>,
|
||||
config: Config,
|
||||
debug_enabled: bool,
|
||||
) {
|
||||
#[cfg(test)]
|
||||
@@ -136,6 +141,7 @@ pub(super) fn start_upgrade_terminal_session(
|
||||
cwd,
|
||||
controller,
|
||||
controller_rx,
|
||||
config,
|
||||
debug_enabled,
|
||||
);
|
||||
return;
|
||||
@@ -151,6 +157,7 @@ pub(super) fn start_upgrade_terminal_session(
|
||||
cwd,
|
||||
controller,
|
||||
controller_rx,
|
||||
Some(config),
|
||||
debug_enabled,
|
||||
);
|
||||
}
|
||||
@@ -162,6 +169,7 @@ fn start_guided_terminal_session(
|
||||
cwd: Option<String>,
|
||||
controller: TerminalRunController,
|
||||
controller_rx: Receiver<TerminalRunEvent>,
|
||||
config: Option<Config>,
|
||||
debug_enabled: bool,
|
||||
) {
|
||||
std::thread::spawn(move || {
|
||||
@@ -200,6 +208,7 @@ fn start_guided_terminal_session(
|
||||
cwd.as_deref(),
|
||||
controller,
|
||||
&mut controller_rx,
|
||||
config,
|
||||
debug_enabled,
|
||||
) {
|
||||
let helper = match &mode {
|
||||
@@ -234,10 +243,14 @@ fn run_guided_loop(
|
||||
cwd: Option<&str>,
|
||||
controller: TerminalRunController,
|
||||
controller_rx: &mut Receiver<TerminalRunEvent>,
|
||||
provided_config: Option<Config>,
|
||||
debug_enabled: bool,
|
||||
) -> Result<()> {
|
||||
let cfg = Config::load_with_cli_overrides(vec![], ConfigOverrides::default())
|
||||
.context("loading config")?;
|
||||
let cfg = match provided_config {
|
||||
Some(cfg) => cfg,
|
||||
None => Config::load_with_cli_overrides(vec![], ConfigOverrides::default())
|
||||
.context("loading config")?,
|
||||
};
|
||||
let preferred_auth = if cfg.using_chatgpt_auth {
|
||||
codex_protocol::mcp_protocol::AuthMode::ChatGPT
|
||||
} else {
|
||||
@@ -423,6 +436,8 @@ fn run_guided_loop(
|
||||
_ => 0,
|
||||
};
|
||||
|
||||
let sandbox_restricted = !matches!(cfg.sandbox_policy, SandboxPolicy::DangerFullAccess);
|
||||
|
||||
if let GuidedTerminalMode::DirectCommand { command } = mode {
|
||||
let wrapped = wrap_command(command);
|
||||
if wrapped.is_empty() {
|
||||
@@ -468,6 +483,35 @@ fn run_guided_loop(
|
||||
message: "Analyzing output…".to_string(),
|
||||
});
|
||||
} else if let GuidedTerminalMode::Upgrade { initial_command, .. } = mode {
|
||||
if sandbox_restricted {
|
||||
let notice = format!(
|
||||
"Automatic upgrades require Full Access. Run `{initial_command}` manually in your own shell, then re-run `/update` once it finishes.\n"
|
||||
);
|
||||
app_event_tx.send(AppEvent::TerminalChunk {
|
||||
id: terminal_id,
|
||||
chunk: notice.clone().into_bytes(),
|
||||
_is_stderr: true,
|
||||
});
|
||||
app_event_tx.send(AppEvent::TerminalUpdateMessage {
|
||||
id: terminal_id,
|
||||
message: notice.trim().to_string(),
|
||||
});
|
||||
app_event_tx.send(AppEvent::TerminalSetAssistantMessage {
|
||||
id: terminal_id,
|
||||
message: "Waiting for manual upgrade…".to_string(),
|
||||
});
|
||||
conversation.push(make_message(
|
||||
"user",
|
||||
"Automatic execution blocked by sandbox. Await user's manual upgrade before continuing.".to_string(),
|
||||
));
|
||||
app_event_tx.send(AppEvent::TerminalExit {
|
||||
id: terminal_id,
|
||||
exit_code: None,
|
||||
_duration: Duration::from_millis(0),
|
||||
});
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let wrapped = wrap_command(initial_command);
|
||||
if wrapped.is_empty() {
|
||||
app_event_tx.send(AppEvent::TerminalChunk {
|
||||
|
||||
@@ -285,7 +285,7 @@ fn wait_tool_updates_exec_record_via_history_state() {
|
||||
}),
|
||||
order: Some(order_meta(0)),
|
||||
});
|
||||
pump_app_events(&mut chat, &rx);
|
||||
let _ = pump_app_events(&mut chat, &rx);
|
||||
let _ = drain_insert_history(&rx);
|
||||
|
||||
let wait_params = json!({
|
||||
@@ -303,7 +303,7 @@ fn wait_tool_updates_exec_record_via_history_state() {
|
||||
}),
|
||||
order: Some(order_meta(1)),
|
||||
});
|
||||
pump_app_events(&mut chat, &rx);
|
||||
let _ = pump_app_events(&mut chat, &rx);
|
||||
let _ = drain_insert_history(&rx);
|
||||
|
||||
chat.handle_codex_event(Event {
|
||||
@@ -318,7 +318,7 @@ fn wait_tool_updates_exec_record_via_history_state() {
|
||||
}),
|
||||
order: Some(order_meta(2)),
|
||||
});
|
||||
pump_app_events(&mut chat, &rx);
|
||||
let _ = pump_app_events(&mut chat, &rx);
|
||||
let _ = drain_insert_history(&rx);
|
||||
|
||||
let exec_record = chat
|
||||
@@ -369,7 +369,7 @@ fn explore_updates_history_state_via_domain_events() {
|
||||
}),
|
||||
order: Some(order_meta(0)),
|
||||
});
|
||||
pump_app_events(&mut chat, &rx);
|
||||
let _ = pump_app_events(&mut chat, &rx);
|
||||
let _ = drain_insert_history(&rx);
|
||||
|
||||
let explore_id = chat
|
||||
@@ -407,7 +407,7 @@ fn explore_updates_history_state_via_domain_events() {
|
||||
}),
|
||||
order: Some(order_meta(1)),
|
||||
});
|
||||
pump_app_events(&mut chat, &rx);
|
||||
let _ = pump_app_events(&mut chat, &rx);
|
||||
let _ = drain_insert_history(&rx);
|
||||
|
||||
let explore_state = chat
|
||||
@@ -781,14 +781,20 @@ impl ScriptStep {
|
||||
}
|
||||
}
|
||||
|
||||
fn run_script(chat: &mut ChatWidget<'_>, steps: &[ScriptStep], rx: &std::sync::mpsc::Receiver<AppEvent>) {
|
||||
fn run_script(
|
||||
chat: &mut ChatWidget<'_>,
|
||||
steps: &[ScriptStep],
|
||||
rx: &std::sync::mpsc::Receiver<AppEvent>,
|
||||
) -> Vec<AppEvent> {
|
||||
let mut captured = Vec::new();
|
||||
for step in steps {
|
||||
if let ScriptStep::Key(code, modifiers) = step {
|
||||
chat.handle_key_event(KeyEvent::new(*code, *modifiers));
|
||||
pump_app_events(chat, rx);
|
||||
captured.extend(pump_app_events(chat, rx));
|
||||
}
|
||||
}
|
||||
pump_app_events(chat, rx);
|
||||
captured.extend(pump_app_events(chat, rx));
|
||||
captured
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -897,12 +903,12 @@ fn agents_terminal_toggle_via_shortcuts() {
|
||||
assert!(!chat.agents_terminal.active);
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Char('a'), KeyModifiers::CONTROL));
|
||||
pump_app_events(&mut chat, &rx);
|
||||
let _ = pump_app_events(&mut chat, &rx);
|
||||
assert!(chat.agents_terminal.active, "Ctrl+A should open terminal");
|
||||
|
||||
// Esc should exit the terminal view
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE));
|
||||
pump_app_events(&mut chat, &rx);
|
||||
let _ = pump_app_events(&mut chat, &rx);
|
||||
assert!(!chat.agents_terminal.active, "Esc should exit terminal");
|
||||
}
|
||||
|
||||
@@ -932,27 +938,27 @@ fn agents_terminal_focus_and_scroll_controls() {
|
||||
});
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Char('a'), KeyModifiers::CONTROL));
|
||||
pump_app_events(&mut chat, &rx);
|
||||
let _ = pump_app_events(&mut chat, &rx);
|
||||
assert_eq!(chat.agents_terminal.focus(), AgentsTerminalFocus::Sidebar);
|
||||
|
||||
chat.layout.last_history_viewport_height.set(5);
|
||||
chat.layout.last_max_scroll.set(5);
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
pump_app_events(&mut chat, &rx);
|
||||
let _ = pump_app_events(&mut chat, &rx);
|
||||
assert_eq!(chat.agents_terminal.focus(), AgentsTerminalFocus::Detail);
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE));
|
||||
pump_app_events(&mut chat, &rx);
|
||||
let _ = pump_app_events(&mut chat, &rx);
|
||||
assert_eq!(chat.layout.scroll_offset, 1, "Up should scroll output when focused");
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE));
|
||||
pump_app_events(&mut chat, &rx);
|
||||
let _ = pump_app_events(&mut chat, &rx);
|
||||
assert!(chat.agents_terminal.active, "Overlay should remain open after first Esc");
|
||||
assert_eq!(chat.agents_terminal.focus(), AgentsTerminalFocus::Sidebar);
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE));
|
||||
pump_app_events(&mut chat, &rx);
|
||||
let _ = pump_app_events(&mut chat, &rx);
|
||||
assert!(!chat.agents_terminal.active, "Second Esc should close overlay");
|
||||
}
|
||||
|
||||
@@ -982,16 +988,20 @@ fn agents_terminal_esc_closes_from_sidebar() {
|
||||
});
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Char('a'), KeyModifiers::CONTROL));
|
||||
pump_app_events(&mut chat, &rx);
|
||||
let _ = pump_app_events(&mut chat, &rx);
|
||||
assert!(chat.agents_terminal.active, "overlay should open");
|
||||
assert_eq!(chat.agents_terminal.focus(), AgentsTerminalFocus::Sidebar);
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE));
|
||||
pump_app_events(&mut chat, &rx);
|
||||
let _ = pump_app_events(&mut chat, &rx);
|
||||
assert!(!chat.agents_terminal.active, "Esc should close from sidebar focus");
|
||||
}
|
||||
|
||||
fn pump_app_events(chat: &mut ChatWidget<'_>, rx: &std::sync::mpsc::Receiver<AppEvent>) {
|
||||
fn pump_app_events(
|
||||
chat: &mut ChatWidget<'_>,
|
||||
rx: &std::sync::mpsc::Receiver<AppEvent>,
|
||||
) -> Vec<AppEvent> {
|
||||
let mut captured = Vec::new();
|
||||
while let Ok(event) = rx.try_recv() {
|
||||
match event {
|
||||
AppEvent::PrepareAgents => chat.prepare_agents(),
|
||||
@@ -1012,9 +1022,13 @@ fn pump_app_events(chat: &mut ChatWidget<'_>, rx: &std::sync::mpsc::Receiver<App
|
||||
}
|
||||
AppEvent::ShowAgentsOverview => chat.show_agents_overview_ui(),
|
||||
AppEvent::RequestRedraw | AppEvent::Redraw | AppEvent::ScheduleFrameIn(_) => {}
|
||||
AppEvent::OpenTerminal(_) => {
|
||||
captured.push(event);
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
captured
|
||||
}
|
||||
|
||||
fn buffer_to_string(buffer: &ratatui::buffer::Buffer) -> String {
|
||||
@@ -1070,7 +1084,7 @@ fn slash_agents_opens_overview() {
|
||||
ScriptStep::key_char('s'),
|
||||
ScriptStep::enter(),
|
||||
];
|
||||
run_script(&mut chat, &script, &rx);
|
||||
let _ = run_script(&mut chat, &script, &rx);
|
||||
|
||||
let width: u16 = 120;
|
||||
let height = chat.desired_height(width).max(40);
|
||||
@@ -1092,7 +1106,8 @@ fn slash_agents_opens_overview() {
|
||||
|
||||
#[test]
|
||||
fn slash_upgrade_opens_guided_terminal() {
|
||||
let _guard = EnvGuard::set("CODEX_MANAGED_BY_NPM", "1");
|
||||
let _npm_guard = EnvGuard::set("CODEX_MANAGED_BY_NPM", "1");
|
||||
let _upgrade_guard = EnvGuard::set("SHOW_UPGRADE", "1");
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual();
|
||||
chat.latest_upgrade_version = Some("9.9.9".to_string());
|
||||
|
||||
@@ -1107,10 +1122,11 @@ fn slash_upgrade_opens_guided_terminal() {
|
||||
ScriptStep::key_char('e'),
|
||||
ScriptStep::enter(),
|
||||
];
|
||||
run_script(&mut chat, &script, &rx);
|
||||
let mut events = run_script(&mut chat, &script, &rx);
|
||||
events.extend(rx.try_iter());
|
||||
|
||||
let mut saw_open_terminal = false;
|
||||
while let Ok(event) = rx.try_recv() {
|
||||
for event in events {
|
||||
if let AppEvent::OpenTerminal(launch) = event {
|
||||
saw_open_terminal = true;
|
||||
assert_eq!(launch.title, "Upgrade Code");
|
||||
@@ -1137,7 +1153,7 @@ fn slash_undo_shows_no_snapshot_state() {
|
||||
ScriptStep::key_char('o'),
|
||||
ScriptStep::enter(),
|
||||
];
|
||||
run_script(&mut chat, &script, &rx);
|
||||
let _ = run_script(&mut chat, &script, &rx);
|
||||
|
||||
assert!(
|
||||
chat.bottom_pane.has_active_modal_view(),
|
||||
|
||||
@@ -231,7 +231,7 @@ pub async fn auto_upgrade_if_enabled(config: &Config) -> anyhow::Result<AutoUpgr
|
||||
if sudo_requires_manual_intervention(&fallback.stderr, fallback.status)
|
||||
{
|
||||
outcome.user_notice = Some(format!(
|
||||
"Automatic upgrade needs your attention. Run `/upgrade` to finish with `{}`.",
|
||||
"Automatic upgrade needs your attention. Run `/update` to finish with `{}`.",
|
||||
command_display
|
||||
));
|
||||
}
|
||||
@@ -245,7 +245,7 @@ pub async fn auto_upgrade_if_enabled(config: &Config) -> anyhow::Result<AutoUpgr
|
||||
Err(err) => {
|
||||
warn!("auto-upgrade: sudo retry error: {err}");
|
||||
outcome.user_notice = Some(format!(
|
||||
"Automatic upgrade could not escalate permissions. Run `/upgrade` to finish with `{}`.",
|
||||
"Automatic upgrade could not escalate permissions. Run `/update` to finish with `{}`.",
|
||||
command_display
|
||||
));
|
||||
return Ok(outcome);
|
||||
@@ -415,13 +415,24 @@ fn sudo_requires_manual_intervention(stderr: &str, status: Option<i32>) -> bool
|
||||
|
||||
fn truncate_for_log(text: &str) -> String {
|
||||
const LIMIT: usize = 256;
|
||||
const ELLIPSIS_BYTES: usize = '…'.len_utf8();
|
||||
if text.len() <= LIMIT {
|
||||
text.replace('\n', " ")
|
||||
} else {
|
||||
let mut truncated = text[..LIMIT].to_string();
|
||||
truncated.push_str("…");
|
||||
truncated.replace('\n', " ")
|
||||
return text.replace('\n', " ");
|
||||
}
|
||||
|
||||
let slice_limit = LIMIT.saturating_sub(ELLIPSIS_BYTES);
|
||||
let safe_boundary = text
|
||||
.char_indices()
|
||||
.map(|(idx, _)| idx)
|
||||
.chain(std::iter::once(text.len()))
|
||||
.take_while(|idx| *idx <= slice_limit)
|
||||
.last()
|
||||
.unwrap_or(0);
|
||||
|
||||
let safe_slice = text.get(..safe_boundary).unwrap_or("");
|
||||
let mut truncated = safe_slice.to_string();
|
||||
truncated.push('…');
|
||||
truncated.replace('\n', " ")
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -442,6 +453,16 @@ mod unit_tests {
|
||||
assert!(truncated.len() <= 257);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn truncate_for_log_handles_multibyte_characters() {
|
||||
let long = "漢字".repeat(200);
|
||||
let truncated = truncate_for_log(&long);
|
||||
assert!(truncated.ends_with('…'));
|
||||
let prefix = truncated.trim_end_matches('…');
|
||||
assert!(prefix.len() <= 256);
|
||||
assert!(long.starts_with(prefix));
|
||||
}
|
||||
|
||||
#[cfg(any(target_os = "macos", target_os = "linux"))]
|
||||
#[test]
|
||||
fn wrap_with_sudo_injects_dash_dash() {
|
||||
|
||||
Reference in New Issue
Block a user