openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-09 20:50:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

Improve hooks trust flow in TUI (#21755)

Browse Source 
# Why
Hooks that need trust review were easy to miss, and the existing TUI
flow made users discover `/hooks` manually before they could decide
whether to inspect or trust them.

# What
- add a startup review prompt for new or changed hooks before normal
composer use
- add a top-level `t` shortcut in `/hooks` to trust every review-needed
hook at once
- make pending-review rows and helper copy use warning styling

## TUI

### Startup review interstitial

```text
Hooks need review
2 hooks are new or changed.
Hooks can run outside the sandbox after you trust them.

› 1. Review hooks
  2. Trust all and continue
  3. Continue without trusting (hooks won't run)
```

### Top-level `/hooks` page when review is needed

```text
Hooks
Lifecycle hooks from config and enabled plugins.

⚠ 1 hook needs review before it can run.

Event                 Installed   Active   Review   Description
PreToolUse            1           0        1        Before a tool executes
...

Press t to trust all; enter to review hooks; esc to close
```
This commit is contained in:
Abhinav
2026-05-09 21:17:30 +00:00
committed by GitHub
parent 53468b97f6
commit 6d747db7d8
16 changed files with 846 additions and 209 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/tui/src/app.rs
+6 -3
View File
@@ -40,6 +40,7 @@ use crate::history_cell;
use crate::history_cell::HistoryCell;
#[cfg(not(debug_assertions))]
use crate::history_cell::UpdateAvailableHistoryCell;
use crate::hooks_rpc::HookTrustUpdate;
use crate::key_hint::KeyBindingListExt;
use crate::keymap::RuntimeKeymap;
use crate::legacy_core::config::Config;
@@ -91,8 +92,7 @@ use codex_app_server_protocol::ConfigWriteResponse;
use codex_app_server_protocol::FeedbackUploadParams;
use codex_app_server_protocol::FeedbackUploadResponse;
use codex_app_server_protocol::GetAccountRateLimitsResponse;
use codex_app_server_protocol::HooksListParams;
use codex_app_server_protocol::HooksListResponse;
use codex_app_server_protocol::HooksListEntry;
use codex_app_server_protocol::ListMcpServerStatusParams;
use codex_app_server_protocol::ListMcpServerStatusResponse;
#[cfg(test)]
@@ -618,6 +618,7 @@ impl App {
remote_app_server_auth_token: Option<String>,
state_db: Option<StateDbHandle>,
environment_manager: Arc<EnvironmentManager>,
startup_hooks_browser: Option<HooksListEntry>,
) -> Result<AppExitInfo> {
use tokio_stream::StreamExt;
let (app_event_tx, mut app_event_rx) = unbounded_channel();
@@ -914,6 +915,9 @@ See the Codex keymap documentation for supported actions and examples."
pending_plugin_enabled_writes: HashMap::new(),
pending_hook_enabled_writes: HashMap::new(),
};
if let Some(entry) = startup_hooks_browser {
app.chat_widget.open_hooks_browser(entry);
}
if let Some(started) = initial_started_thread {
let thread_id = started.session.thread_id;
app.enqueue_primary_thread_session(started.session, started.turns)
@@ -957,7 +961,6 @@ See the Codex keymap documentation for supported actions and examples."
tui.frame_requester().schedule_frame();
app.refresh_startup_skills(&app_server);
app.refresh_startup_hooks(&app_server);
// Kick off a non-blocking rate-limit prefetch so the first `/status`
// already has data, without delaying the initial frame render.
if requires_openai_auth && has_chatgpt_account {
codex-rs/tui/src/app/background_requests.rs
+19 -85
View File
@@ -5,7 +5,6 @@
//! the main event loop remains single-threaded.
use super::*;
use codex_app_server_protocol::HookTrustStatus;
use codex_app_server_protocol::MarketplaceAddParams;
use codex_app_server_protocol::MarketplaceAddResponse;
use codex_app_server_protocol::MarketplaceRemoveParams;
@@ -15,6 +14,9 @@ use codex_app_server_protocol::MarketplaceUpgradeResponse;
use codex_app_server_protocol::RequestId;
use crate::hooks_rpc::fetch_hooks_list;
use crate::hooks_rpc::write_hook_trust;
use crate::hooks_rpc::write_hook_trusts;
use codex_utils_absolute_path::AbsolutePathBuf;
impl App {
@@ -89,47 +91,6 @@ impl App {
});
}
/// Emits the initial hook review warning without delaying the first interactive frame.
pub(super) fn refresh_startup_hooks(&mut self, app_server: &AppServerSession) {
let request_handle = app_server.request_handle();
let app_event_tx = self.app_event_tx.clone();
let cwd = self.config.cwd.to_path_buf();
tokio::spawn(async move {
let result = fetch_hooks_list(request_handle, cwd.clone()).await;
let response = match result {
Ok(response) => response,
Err(err) => {
tracing::warn!("failed to load startup hook review state: {err:#}");
return;
}
};
let hooks_needing_review = response
.data
.into_iter()
.find(|entry| entry.cwd.as_path() == cwd.as_path())
.map(|entry| {
entry
.hooks
.into_iter()
.filter(|hook| {
matches!(
hook.trust_status,
HookTrustStatus::Untrusted | HookTrustStatus::Modified
)
})
.count()
})
.unwrap_or_default();
if let Some(message) =
startup_prompts::hooks_needing_review_warning(hooks_needing_review)
{
app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
history_cell::new_warning_event(message),
)));
}
});
}
pub(super) fn fetch_plugins_list(&mut self, app_server: &AppServerSession, cwd: PathBuf) {
let request_handle = app_server.request_handle();
let app_event_tx = self.app_event_tx.clone();
@@ -381,6 +342,22 @@ impl App {
});
}
pub(super) fn trust_hooks(
&mut self,
app_server: &AppServerSession,
updates: Vec<HookTrustUpdate>,
) {
let request_handle = app_server.request_handle();
let app_event_tx = self.app_event_tx.clone();
tokio::spawn(async move {
let result = write_hook_trusts(request_handle, updates)
.await
.map(|_| ())
.map_err(|err| format!("Failed to trust hooks: {err}"));
app_event_tx.send(AppEvent::HookTrusted { result });
});
}
pub(super) fn refresh_plugin_mentions(&mut self) {
let config = self.config.clone();
let app_event_tx = self.app_event_tx.clone();
@@ -674,20 +651,6 @@ pub(super) async fn fetch_plugins_list(
Ok(response)
}
pub(super) async fn fetch_hooks_list(
request_handle: AppServerRequestHandle,
cwd: PathBuf,
) -> Result<HooksListResponse> {
let request_id = RequestId::String(format!("hooks-list-{}", Uuid::new_v4()));
request_handle
.request_typed(ClientRequest::HooksList {
request_id,
params: HooksListParams { cwds: vec![cwd] },
})
.await
.wrap_err("hooks/list failed in TUI")
}
const CLI_HIDDEN_PLUGIN_MARKETPLACES: &[&str] = &["openai-bundled"];
pub(super) fn hide_cli_only_plugin_marketplaces(response: &mut PluginListResponse) {
@@ -864,35 +827,6 @@ pub(super) async fn write_hook_enabled(
.wrap_err("config/batchWrite failed while updating hook enablement in TUI")
}
pub(super) async fn write_hook_trust(
request_handle: AppServerRequestHandle,
key: String,
current_hash: String,
) -> Result<ConfigWriteResponse> {
let request_id = RequestId::String(format!("hooks-config-write-{}", Uuid::new_v4()));
let value = serde_json::json!({
key: {
"trusted_hash": current_hash,
}
});
request_handle
.request_typed(ClientRequest::ConfigBatchWrite {
request_id,
params: ConfigBatchWriteParams {
edits: vec![codex_app_server_protocol::ConfigEdit {
key_path: "hooks.state".to_string(),
value,
merge_strategy: MergeStrategy::Upsert,
}],
file_path: None,
expected_version: None,
reload_user_config: true,
},
})
.await
.wrap_err("config/batchWrite failed while updating hook trust in TUI")
}
pub(super) fn build_feedback_upload_params(
origin_thread_id: Option<ThreadId>,
rollout_path: Option<PathBuf>,
codex-rs/tui/src/app/event_dispatch.rs
+3
View File
@@ -1713,6 +1713,9 @@ impl App {
AppEvent::TrustHook { key, current_hash } => {
self.trust_hook(app_server, key, current_hash);
}
AppEvent::TrustHooks { updates } => {
self.trust_hooks(app_server, updates);
}
AppEvent::HookEnabledSet {
key,
enabled,
codex-rs/tui/src/app/startup_prompts.rs
-10
View File
@@ -77,16 +77,6 @@ pub(super) fn emit_system_bwrap_warning(app_event_tx: &AppEventSender, config: &
)));
}
pub(super) fn hooks_needing_review_warning(count: usize) -> Option<String> {
match count {
0 => None,
1 => Some("1 hook needs review before it can run. Open /hooks to review it.".to_string()),
count => Some(format!(
"{count} hooks need review before they can run. Open /hooks to review them."
)),
}
}
pub(super) fn should_show_model_migration_prompt(
current_model: &str,
target_model: &str,
codex-rs/tui/src/app/tests.rs
-11
View File
@@ -301,17 +301,6 @@ async fn ignore_same_thread_resume_allows_reattaching_displayed_inactive_thread(
assert!(app.transcript_cells.is_empty());
}
#[test]
fn hooks_needing_review_startup_warning_snapshot() {
let message = startup_prompts::hooks_needing_review_warning(/*count*/ 2)
.expect("review-needed hooks should produce a startup warning");
let rendered = lines_to_single_string(
&history_cell::new_warning_event(message).display_lines(/*width*/ 80),
);
assert_app_snapshot!("hooks_needing_review_startup_warning", rendered);
}
#[tokio::test]
async fn enqueue_primary_thread_session_replays_buffered_approval_after_attach() -> Result<()> {
let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await;
codex-rs/tui/src/app_event.rs
+5
View File
@@ -775,6 +775,11 @@ pub(crate) enum AppEvent {
current_hash: String,
},
/// Trust the current definitions for one or more hooks by stable hook key.
TrustHooks {
updates: Vec<crate::hooks_rpc::HookTrustUpdate>,
},
/// Result of persisting hook enabled state.
HookEnabledSet {
key: String,
codex-rs/tui/src/bottom_pane/hooks_browser_view.rs
+285 -78
View File
@@ -1,8 +1,8 @@
use codex_app_server_protocol::HookErrorInfo;
use codex_app_server_protocol::HookEventName;
use codex_app_server_protocol::HookMetadata;
use codex_app_server_protocol::HookSource;
use codex_app_server_protocol::HookTrustStatus;
use codex_app_server_protocol::HooksListEntry;
use crossterm::event::KeyCode;
use crossterm::event::KeyEvent;
use crossterm::event::KeyModifiers;
@@ -26,6 +26,8 @@ use super::scroll_state::ScrollState;
use super::selection_popup_common::render_menu_surface;
use crate::app_event::AppEvent;
use crate::app_event_sender::AppEventSender;
use crate::hooks_rpc::HookTrustUpdate;
use crate::hooks_rpc::hook_needs_review;
use crate::key_hint;
use crate::line_truncation::truncate_line_with_ellipsis_if_overflow;
use crate::render::renderable::Renderable;
@@ -43,9 +45,7 @@ enum HooksBrowserPage {
}
pub(crate) struct HooksBrowserView {
hooks: Vec<HookMetadata>,
warnings: Vec<String>,
errors: Vec<HookErrorInfo>,
entry: HooksListEntry,
page: HooksBrowserPage,
state: ScrollState,
complete: bool,
@@ -53,17 +53,28 @@ pub(crate) struct HooksBrowserView {
}
impl HooksBrowserView {
#[cfg(test)]
pub(crate) fn new(
mut hooks: Vec<HookMetadata>,
hooks: Vec<HookMetadata>,
warnings: Vec<String>,
errors: Vec<HookErrorInfo>,
errors: Vec<codex_app_server_protocol::HookErrorInfo>,
app_event_tx: AppEventSender,
) -> Self {
hooks.sort_by_key(|hook| hook.display_order);
Self::from_entry(
HooksListEntry {
cwd: std::path::PathBuf::new(),
hooks,
warnings,
errors,
},
app_event_tx,
)
}
pub(crate) fn from_entry(mut entry: HooksListEntry, app_event_tx: AppEventSender) -> Self {
entry.hooks.sort_by_key(|hook| hook.display_order);
let mut view = Self {
hooks,
warnings,
errors,
entry,
page: HooksBrowserPage::Events,
state: ScrollState::new(),
complete: false,
@@ -85,16 +96,19 @@ impl HooksBrowserView {
.map(|event_name| {
let event_name: HookEventName = event_name.into();
let installed = self
.entry
.hooks
.iter()
.filter(|hook| hook.event_name == event_name)
.count();
let active = self
.entry
.hooks
.iter()
.filter(|hook| hook.event_name == event_name && hook_is_active(hook))
.count();
let needs_review = self
.entry
.hooks
.iter()
.filter(|hook| hook.event_name == event_name && hook_needs_review(hook))
@@ -110,7 +124,8 @@ impl HooksBrowserView {
}
fn handlers_for_event(&self, event_name: HookEventName) -> impl Iterator<Item = &HookMetadata> {
self.hooks
self.entry
.hooks
.iter()
.filter(move |hook| hook.event_name == event_name)
}
@@ -124,7 +139,8 @@ impl HooksBrowserView {
fn selected_hook_index(&self, event_name: HookEventName) -> Option<usize> {
let selected_visible_idx = self.state.selected_idx?;
self.hooks
self.entry
.hooks
.iter()
.enumerate()
.filter(|(_, hook)| hook.event_name == event_name)
@@ -134,7 +150,7 @@ impl HooksBrowserView {
fn selected_hook(&self, event_name: HookEventName) -> Option<&HookMetadata> {
self.selected_hook_index(event_name)
.and_then(|idx| self.hooks.get(idx))
.and_then(|idx| self.entry.hooks.get(idx))
}
fn move_up(&mut self) {
@@ -175,7 +191,7 @@ impl HooksBrowserView {
let Some(idx) = self.selected_hook_index(event_name) else {
return;
};
let Some(hook) = self.hooks.get_mut(idx) else {
let Some(hook) = self.entry.hooks.get_mut(idx) else {
return;
};
if hook.is_managed {
@@ -196,7 +212,7 @@ impl HooksBrowserView {
let Some(idx) = self.selected_hook_index(event_name) else {
return;
};
let Some(hook) = self.hooks.get_mut(idx) else {
let Some(hook) = self.entry.hooks.get_mut(idx) else {
return;
};
if !hook_needs_review(hook) {
@@ -210,6 +226,24 @@ impl HooksBrowserView {
});
}
fn trust_all_hooks(&mut self) {
let mut updates = Vec::new();
for hook in &mut self.entry.hooks {
if !hook_needs_review(hook) {
continue;
}
hook.trust_status = HookTrustStatus::Trusted;
updates.push(HookTrustUpdate {
key: hook.key.clone(),
current_hash: hook.current_hash.clone(),
});
}
if !updates.is_empty() {
self.app_event_tx.send(AppEvent::TrustHooks { updates });
}
}
fn close(&mut self) {
self.complete = true;
}
@@ -238,23 +272,27 @@ impl HooksBrowserView {
]
}
fn review_needed_total_count(&self) -> usize {
self.entry
.hooks
.iter()
.filter(|hook| hook_needs_review(hook))
.count()
}
#[allow(clippy::disallowed_methods)]
fn handler_header_lines(
event_name: HookEventName,
review_needed_count: usize,
) -> Vec<Line<'static>> {
let mut lines = vec![format!("{} hooks", event_label(event_name)).bold().into()];
match review_needed_count {
0 => lines.push(
match review_needed_message(review_needed_count) {
None => lines.push(
"Turn hooks on or off. Your changes are saved automatically."
.dim()
.into(),
),
1 => lines.push("1 hook needs review before it can run.".dim().into()),
count => lines.push(
format!("{count} hooks need review before they can run.")
.dim()
.into(),
),
Some(message) => lines.push(message.yellow().into()),
}
lines
}
@@ -265,6 +303,7 @@ impl HooksBrowserView {
.count()
}
#[allow(clippy::disallowed_methods)]
fn event_table_lines(&self) -> Vec<Line<'static>> {
let rows = self.event_rows();
let show_review = rows.iter().any(|row| row.needs_review > 0);
@@ -280,59 +319,59 @@ impl HooksBrowserView {
header.push("Description".into());
lines.push(Line::from(header));
for (idx, row) in rows.into_iter().enumerate() {
if self.state.selected_idx == Some(idx) {
let style = accent_style();
let mut row_line = vec![
Span::from(format!(
"{:<EVENT_COLUMN_WIDTH$}",
event_label(row.event_name)
))
.set_style(style),
Span::from(format!("{:<COUNT_COLUMN_WIDTH$}", row.installed)).set_style(style),
Span::from(format!("{:<COUNT_COLUMN_WIDTH$}", row.active)).set_style(style),
];
if show_review {
row_line.push(
Span::from(format!("{:<COUNT_COLUMN_WIDTH$}", row.needs_review))
.set_style(style),
);
}
row_line.push(Span::from(event_description(row.event_name)).set_style(style));
lines.push(Line::from(row_line));
} else {
let mut row_line = vec![
Span::from(format!(
"{:<EVENT_COLUMN_WIDTH$}",
event_label(row.event_name)
)),
Span::from(format!("{:<COUNT_COLUMN_WIDTH$}", row.installed)).dim(),
Span::from(format!("{:<COUNT_COLUMN_WIDTH$}", row.active)).dim(),
];
if show_review {
row_line.push(
Span::from(format!("{:<COUNT_COLUMN_WIDTH$}", row.needs_review)).dim(),
);
}
row_line.push(Span::from(event_description(row.event_name)).dim());
lines.push(Line::from(row_line));
let selected = self.state.selected_idx == Some(idx);
let needs_review = row.needs_review > 0;
let mut row_line = vec![
Span::from(format!(
"{:<EVENT_COLUMN_WIDTH$}",
event_label(row.event_name)
)),
Span::from(format!("{:<COUNT_COLUMN_WIDTH$}", row.installed)),
Span::from(format!("{:<COUNT_COLUMN_WIDTH$}", row.active)),
];
if show_review {
let review_count = Span::from(format!("{:<COUNT_COLUMN_WIDTH$}", row.needs_review));
row_line.push(if needs_review {
review_count.yellow()
} else {
review_count
});
}
row_line.push(Span::from(event_description(row.event_name)));
if selected {
let style = accent_style();
for span in &mut row_line {
*span = span.clone().set_style(style);
}
} else {
row_line[1] = row_line[1].clone().dim();
row_line[2] = row_line[2].clone().dim();
if show_review && !needs_review {
row_line[3] = row_line[3].clone().dim();
}
let description_idx = row_line.len() - 1;
row_line[description_idx] = row_line[description_idx].clone().dim();
}
lines.push(Line::from(row_line));
}
lines
}
fn event_issue_lines(&self) -> Vec<Line<'static>> {
let mut lines = Vec::new();
if self.warnings.is_empty() && self.errors.is_empty() {
if self.entry.warnings.is_empty() && self.entry.errors.is_empty() {
return lines;
}
lines.push("Issues".bold().into());
lines.extend(
self.warnings
self.entry
.warnings
.iter()
.map(|warning| format!("⚠ {warning}").into()),
);
lines.extend(self.errors.iter().map(|error| {
lines.extend(self.entry.errors.iter().map(|error| {
format!("■ {}: {}", error.path.display(), error.message)
.red()
.into()
@@ -340,10 +379,16 @@ impl HooksBrowserView {
lines
}
#[allow(clippy::disallowed_methods)]
fn event_page_lines(&self) -> Vec<Line<'static>> {
let mut lines = Self::event_header_lines();
lines.push(Line::default());
if let Some(message) = review_needed_message(self.review_needed_total_count()) {
lines.push(format!("⚠ {message}").yellow().into());
lines.push(Line::default());
}
let issue_lines = self.event_issue_lines();
if !issue_lines.is_empty() {
lines.extend(issue_lines);
@@ -354,6 +399,7 @@ impl HooksBrowserView {
lines
}
#[allow(clippy::disallowed_methods)]
fn handler_row_lines(&self, event_name: HookEventName, width: usize) -> Vec<Line<'static>> {
self.handlers_for_event(event_name)
.enumerate()
@@ -376,11 +422,17 @@ impl HooksBrowserView {
};
let mut line = Line::from(row);
line = truncate_line_with_ellipsis_if_overflow(line, width);
if hook.is_managed {
line = line.dim();
}
let needs_review = hook_needs_review(hook);
if self.state.selected_idx == Some(idx) {
line = line.patch_style(accent_style());
if needs_review {
line = line.yellow().bold();
} else {
line = line.patch_style(accent_style());
}
} else if needs_review {
line = line.yellow();
} else if hook.is_managed {
line = line.dim();
}
line
})
@@ -423,6 +475,15 @@ impl HooksBrowserView {
height: area.height,
};
let footer = match self.page {
HooksBrowserPage::Events if self.review_needed_total_count() > 0 => Line::from(vec![
"Press ".into(),
key_hint::plain(KeyCode::Char('t')).into(),
" to trust all; ".into(),
key_hint::plain(KeyCode::Enter).into(),
" to review hooks; ".into(),
key_hint::plain(KeyCode::Esc).into(),
" to close".into(),
]),
HooksBrowserPage::Events => Line::from(vec![
"Press ".into(),
key_hint::plain(KeyCode::Enter).into(),
@@ -516,11 +577,10 @@ impl BottomPaneView for HooksBrowserView {
code: KeyCode::Char('t'),
modifiers: KeyModifiers::NONE,
..
} => {
if let HooksBrowserPage::Handlers(event_name) = self.page {
self.trust_selected_hook(event_name);
}
}
} => match self.page {
HooksBrowserPage::Events => self.trust_all_hooks(),
HooksBrowserPage::Handlers(event_name) => self.trust_selected_hook(event_name),
},
KeyEvent {
code: KeyCode::Esc, ..
} => match self.page {
@@ -631,6 +691,14 @@ fn hook_is_active(hook: &HookMetadata) -> bool {
)
}
fn review_needed_message(count: usize) -> Option<String> {
match count {
0 => None,
1 => Some("1 hook needs review before it can run.".to_string()),
count => Some(format!("{count} hooks need review before they can run.")),
}
}
struct EventRow {
event_name: HookEventName,
installed: usize,
@@ -638,13 +706,6 @@ struct EventRow {
needs_review: usize,
}
fn hook_needs_review(hook: &HookMetadata) -> bool {
matches!(
hook.trust_status,
HookTrustStatus::Untrusted | HookTrustStatus::Modified
)
}
fn hook_trust_label(status: HookTrustStatus) -> &'static str {
match status {
HookTrustStatus::Managed => "Managed",
@@ -784,6 +845,7 @@ mod tests {
use crate::test_support::PathBufExt;
use crate::test_support::test_path_buf;
use crate::test_support::test_path_display;
use codex_app_server_protocol::HookErrorInfo;
use codex_app_server_protocol::HookEventName;
use codex_app_server_protocol::HookHandlerType;
use codex_app_server_protocol::HookMetadata;
@@ -794,6 +856,7 @@ mod tests {
use insta::assert_snapshot;
use ratatui::buffer::Buffer;
use ratatui::layout::Rect;
use ratatui::style::Color;
use ratatui::style::Modifier;
use tokio::sync::mpsc::unbounded_channel;
@@ -959,6 +1022,16 @@ mod tests {
"hooks_browser_events_with_review_column",
render_lines(&view, /*width*/ 112)
);
assert_eq!(
view.event_table_lines()[1].spans[3].style.fg,
Some(Color::Cyan)
);
assert!(
view.event_table_lines()[1].spans[3]
.style
.add_modifier
.contains(ratatui::style::Modifier::BOLD)
);
}
#[test]
@@ -1015,6 +1088,62 @@ mod tests {
);
}
#[test]
fn review_needed_handler_rows_use_warning_color() {
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
let mut untrusted_hook = hook(
"path:untrusted",
HookEventName::PreToolUse,
HookSource::User,
/*plugin_id*/ None,
"~/bin/untrusted.sh",
/*enabled*/ false,
/*is_managed*/ false,
/*display_order*/ 1,
);
untrusted_hook.trust_status = HookTrustStatus::Untrusted;
let mut view = HooksBrowserView::new(
vec![
hook(
"path:trusted",
HookEventName::PreToolUse,
HookSource::User,
/*plugin_id*/ None,
"~/bin/trusted.sh",
/*enabled*/ true,
/*is_managed*/ false,
/*display_order*/ 0,
),
untrusted_hook,
],
Vec::new(),
Vec::new(),
AppEventSender::new(tx_raw),
);
view.handle_key_event(KeyEvent::from(KeyCode::Enter));
assert_eq!(
view.handler_row_lines(HookEventName::PreToolUse, /*width*/ 112)[1]
.style
.fg,
Some(Color::Yellow)
);
}
#[test]
fn review_needed_handler_header_uses_warning_color() {
assert_eq!(
HooksBrowserView::handler_header_lines(
HookEventName::PreToolUse,
/*review_needed_count*/ 1,
)[1]
.spans[0]
.style
.fg,
Some(Color::Yellow)
);
}
#[test]
fn renders_managed_handler_without_toggle_hint() {
let mut view = view();
@@ -1373,7 +1502,7 @@ mod tests {
view.handle_key_event(KeyEvent::from(KeyCode::Enter));
view.handle_key_event(KeyEvent::from(KeyCode::Char('t')));
let hook = view.hooks.first().expect("trusted hook");
let hook = view.entry.hooks.first().expect("trusted hook");
assert!(!hook.enabled);
assert_eq!(hook.trust_status, HookTrustStatus::Trusted);
match rx.try_recv().expect("trust event") {
@@ -1388,6 +1517,84 @@ mod tests {
}
}
#[test]
fn trust_key_on_event_page_trusts_all_review_needed_hooks() {
let (tx_raw, mut rx) = unbounded_channel::<AppEvent>();
let mut untrusted_hook = hook(
"path:untrusted",
HookEventName::PreToolUse,
HookSource::User,
/*plugin_id*/ None,
"/tmp/pre-tool-use-check.sh",
/*enabled*/ false,
/*is_managed*/ false,
/*display_order*/ 0,
);
untrusted_hook.trust_status = HookTrustStatus::Untrusted;
let mut modified_hook = hook(
"path:modified",
HookEventName::Stop,
HookSource::User,
/*plugin_id*/ None,
"/tmp/stop-check.sh",
/*enabled*/ false,
/*is_managed*/ false,
/*display_order*/ 1,
);
modified_hook.trust_status = HookTrustStatus::Modified;
let mut view = HooksBrowserView::new(
vec![
untrusted_hook,
modified_hook,
hook(
"path:trusted",
HookEventName::PreToolUse,
HookSource::User,
/*plugin_id*/ None,
"/tmp/trusted.sh",
/*enabled*/ true,
/*is_managed*/ false,
/*display_order*/ 2,
),
],
Vec::new(),
Vec::new(),
AppEventSender::new(tx_raw),
);
view.handle_key_event(KeyEvent::from(KeyCode::Char('t')));
assert_eq!(
view.entry
.hooks
.iter()
.map(|hook| hook.trust_status)
.collect::<Vec<_>>(),
vec![
HookTrustStatus::Trusted,
HookTrustStatus::Trusted,
HookTrustStatus::Trusted,
]
);
match rx.try_recv().expect("trust event") {
AppEvent::TrustHooks { updates } => assert_eq!(
updates,
vec![
HookTrustUpdate {
key: "path:untrusted".to_string(),
current_hash: "sha256:current".to_string(),
},
HookTrustUpdate {
key: "path:modified".to_string(),
current_hash: "sha256:current".to_string(),
},
]
),
other => panic!("expected hook trust event, got {other:?}"),
}
assert!(rx.try_recv().is_err());
}
#[test]
fn escape_returns_to_the_selected_event() {
let mut view = view();
codex-rs/tui/src/bottom_pane/mod.rs
+1 -2
View File
@@ -31,7 +31,7 @@ use crate::render::renderable::Renderable;
use crate::render::renderable::RenderableItem;
use crate::tui::FrameRequester;
pub(crate) use bottom_pane_view::BottomPaneView;
use bottom_pane_view::ViewCompletion;
pub(crate) use bottom_pane_view::ViewCompletion;
use codex_app_server_protocol::ToolRequestUserInputParams;
use codex_core_skills::model::SkillMetadata;
use codex_features::Features;
@@ -104,7 +104,6 @@ pub(crate) use footer::GoalStatusIndicator;
#[cfg(test)]
pub(crate) use footer::goal_status_indicator_line;
pub(crate) use list_selection_view::ColumnWidthMode;
#[cfg(test)]
pub(crate) use list_selection_view::ListSelectionView;
pub(crate) use list_selection_view::SelectionRowDisplay;
pub(crate) use list_selection_view::SelectionToggle;
codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events_with_review_column.snap
+3 -1
View File
@@ -6,6 +6,8 @@ expression: "render_lines(&view, 112)"
Hooks
Lifecycle hooks from config and enabled plugins.
⚠ 1 hook needs review before it can run.
Event Installed Active Review Description
PreToolUse 1 0 1 Before a tool executes
PermissionRequest 0 0 0 When permission is requested
@@ -16,4 +18,4 @@ expression: "render_lines(&view, 112)"
UserPromptSubmit 0 0 0 When the user submits a prompt
Stop 0 0 0 Right before Codex ends its turn
Press enter to view hooks; esc to close
Press t to trust all; enter to review hooks; esc to close
codex-rs/tui/src/chatwidget/hooks.rs
+12 -13
View File
@@ -3,6 +3,8 @@ use std::path::PathBuf;
use super::ChatWidget;
use crate::app_event::AppEvent;
use crate::bottom_pane::HooksBrowserView;
use crate::hooks_rpc::hooks_list_entry_for_cwd;
use codex_app_server_protocol::HooksListEntry;
use codex_app_server_protocol::HooksListResponse;
impl ChatWidget {
@@ -23,21 +25,18 @@ impl ChatWidget {
match result {
Ok(response) => {
let (hooks, warnings, errors) = response
.data
.into_iter()
.find(|entry| entry.cwd.as_path() == cwd.as_path())
.map(|entry| (entry.hooks, entry.warnings, entry.errors))
.unwrap_or_default();
self.bottom_pane.show_view(Box::new(HooksBrowserView::new(
hooks,
warnings,
errors,
self.app_event_tx.clone(),
)));
self.request_redraw();
self.open_hooks_browser(hooks_list_entry_for_cwd(response, &cwd));
}
Err(err) => self.add_error_message(format!("Failed to load hooks: {err}")),
}
}
pub(crate) fn open_hooks_browser(&mut self, entry: HooksListEntry) {
self.bottom_pane
.show_view(Box::new(HooksBrowserView::from_entry(
entry,
self.app_event_tx.clone(),
)));
self.request_redraw();
}
}
codex-rs/tui/src/hooks_rpc.rs
+100
View File
@@ -0,0 +1,100 @@
use codex_app_server_client::AppServerRequestHandle;
use codex_app_server_protocol::ClientRequest;
use codex_app_server_protocol::ConfigBatchWriteParams;
use codex_app_server_protocol::ConfigWriteResponse;
use codex_app_server_protocol::HookMetadata;
use codex_app_server_protocol::HookTrustStatus;
use codex_app_server_protocol::HooksListEntry;
use codex_app_server_protocol::HooksListParams;
use codex_app_server_protocol::HooksListResponse;
use codex_app_server_protocol::MergeStrategy;
use codex_app_server_protocol::RequestId;
use color_eyre::eyre::Result;
use color_eyre::eyre::WrapErr;
use std::path::Path;
use std::path::PathBuf;
use uuid::Uuid;
#[derive(Clone, Debug, Eq, PartialEq)]
pub(crate) struct HookTrustUpdate {
pub(crate) key: String,
pub(crate) current_hash: String,
}
pub(crate) async fn fetch_hooks_list(
request_handle: AppServerRequestHandle,
cwd: PathBuf,
) -> Result<HooksListResponse> {
let request_id = RequestId::String(format!("hooks-list-{}", Uuid::new_v4()));
request_handle
.request_typed(ClientRequest::HooksList {
request_id,
params: HooksListParams { cwds: vec![cwd] },
})
.await
.wrap_err("hooks/list failed in TUI")
}
pub(crate) fn hooks_list_entry_for_cwd(response: HooksListResponse, cwd: &Path) -> HooksListEntry {
response
.data
.into_iter()
.find(|entry| entry.cwd.as_path() == cwd)
.unwrap_or_else(|| HooksListEntry {
cwd: cwd.to_path_buf(),
hooks: Vec::new(),
warnings: Vec::new(),
errors: Vec::new(),
})
}
pub(crate) fn hook_needs_review(hook: &HookMetadata) -> bool {
matches!(
hook.trust_status,
HookTrustStatus::Untrusted | HookTrustStatus::Modified
)
}
pub(crate) async fn write_hook_trusts(
request_handle: AppServerRequestHandle,
trust_updates: Vec<HookTrustUpdate>,
) -> Result<ConfigWriteResponse> {
let request_id = RequestId::String(format!("hooks-config-write-{}", Uuid::new_v4()));
let value = serde_json::Value::Object(
trust_updates
.into_iter()
.map(|update| {
(
update.key,
serde_json::json!({
"trusted_hash": update.current_hash,
}),
)
})
.collect(),
);
request_handle
.request_typed(ClientRequest::ConfigBatchWrite {
request_id,
params: ConfigBatchWriteParams {
edits: vec![codex_app_server_protocol::ConfigEdit {
key_path: "hooks.state".to_string(),
value,
merge_strategy: MergeStrategy::Upsert,
}],
file_path: None,
expected_version: None,
reload_user_config: true,
},
})
.await
.wrap_err("config/batchWrite failed while updating hook trust in TUI")
}
pub(crate) async fn write_hook_trust(
request_handle: AppServerRequestHandle,
key: String,
current_hash: String,
) -> Result<ConfigWriteResponse> {
write_hook_trusts(request_handle, vec![HookTrustUpdate { key, current_hash }]).await
}
codex-rs/tui/src/lib.rs
+12 -1
View File
@@ -126,6 +126,7 @@ mod frames;
mod get_git_diff;
mod goal_display;
mod history_cell;
mod hooks_rpc;
mod ide_context;
pub(crate) mod insert_history;
pub use insert_history::insert_history_lines;
@@ -162,6 +163,7 @@ mod session_state;
mod shimmer;
mod skills_helpers;
mod slash_command;
mod startup_hooks_review;
mod status;
mod status_indicator_widget;
mod streaming;
@@ -256,6 +258,8 @@ pub(crate) mod test_support;
use crate::onboarding::onboarding_screen::OnboardingScreenArgs;
use crate::onboarding::onboarding_screen::run_onboarding_app;
use crate::startup_hooks_review::StartupHooksReviewOutcome;
use crate::startup_hooks_review::maybe_run_startup_hooks_review;
use crate::tui::Tui;
pub use cli::Cli;
use codex_arg0::Arg0DispatchPaths;
@@ -1494,7 +1498,7 @@ async fn run_ratatui_app(
let use_alt_screen = determine_alt_screen_mode(no_alt_screen, config.tui_alternate_screen);
tui.set_alt_screen_enabled(use_alt_screen);
let app_server = match app_server {
let mut app_server = match app_server {
Some(app_server) => app_server,
None => match start_app_server(
&app_server_target,
@@ -1520,6 +1524,12 @@ async fn run_ratatui_app(
},
};
let startup_hooks_browser =
match maybe_run_startup_hooks_review(&mut app_server, &mut tui, &config).await? {
StartupHooksReviewOutcome::Continue => None,
StartupHooksReviewOutcome::OpenHooksBrowser(data) => Some(data),
};
let app_result = App::run(
&mut tui,
app_server,
@@ -1538,6 +1548,7 @@ async fn run_ratatui_app(
remote_auth_token,
state_db,
environment_manager,
startup_hooks_browser,
)
.await;
codex-rs/tui/src/snapshots/codex_tui__app__tests__hooks_needing_review_startup_warning.snap
-5
View File
@@ -1,5 +0,0 @@
---
source: tui/src/app/tests.rs
expression: rendered
---
⚠ 2 hooks need review before they can run. Open /hooks to review them.
codex-rs/tui/src/snapshots/codex_tui__startup_hooks_review__tests__startup_hooks_review_prompt.snap
+14
View File
@@ -0,0 +1,14 @@
---
source: tui/src/startup_hooks_review.rs
expression: "render_lines(&view, 80)"
---
Hooks need review
2 hooks are new or changed.
Hooks can run outside the sandbox after you trust them.
› 1. Review hooks
2. Trust all and continue
3. Continue without trusting (hooks won't run)
Press enter to confirm or esc to go back
codex-rs/tui/src/snapshots/codex_tui__startup_hooks_review__tests__startup_hooks_review_prompt_with_trust_error.snap
+15
View File
@@ -0,0 +1,15 @@
---
source: tui/src/startup_hooks_review.rs
expression: "render_lines(&view, 80)"
---
Hooks need review
2 hooks are new or changed.
Hooks can run outside the sandbox after you trust them.
Failed to trust hooks: disk full
› 1. Review hooks
2. Trust all and continue
3. Continue without trusting (hooks won't run)
Press enter to confirm or esc to go back
codex-rs/tui/src/startup_hooks_review.rs
+371
View File
@@ -0,0 +1,371 @@
use color_eyre::eyre::Result;
use crossterm::event::KeyEventKind;
use ratatui::buffer::Buffer;
use ratatui::layout::Rect;
use ratatui::style::Stylize;
use ratatui::text::Line;
use ratatui::widgets::Clear;
use ratatui::widgets::WidgetRef;
use tokio::sync::mpsc::unbounded_channel;
use tokio_stream::StreamExt;
use crate::app_event::AppEvent;
use crate::app_event_sender::AppEventSender;
use crate::app_server_session::AppServerSession;
use crate::bottom_pane::BottomPaneView;
use crate::bottom_pane::ListSelectionView;
use crate::bottom_pane::SelectionItem;
use crate::bottom_pane::SelectionViewParams;
use crate::bottom_pane::popup_consts::standard_popup_hint_line_for_keymap;
use crate::hooks_rpc::HookTrustUpdate;
use crate::hooks_rpc::fetch_hooks_list;
use crate::hooks_rpc::hook_needs_review;
use crate::hooks_rpc::hooks_list_entry_for_cwd;
use crate::hooks_rpc::write_hook_trusts;
use crate::keymap::RuntimeKeymap;
use crate::legacy_core::config::Config;
use crate::render::renderable::ColumnRenderable;
use crate::render::renderable::Renderable;
use crate::tui::Tui;
use crate::tui::TuiEvent;
use codex_app_server_protocol::HooksListEntry;
pub(crate) enum StartupHooksReviewOutcome {
Continue,
OpenHooksBrowser(HooksListEntry),
}
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
enum StartupHooksReviewSelection {
ReviewHooks,
TrustAllAndContinue,
ContinueWithoutTrusting,
}
pub(crate) async fn maybe_run_startup_hooks_review(
app_server: &mut AppServerSession,
tui: &mut Tui,
config: &Config,
) -> Result<StartupHooksReviewOutcome> {
let cwd = config.cwd.to_path_buf();
let response = match fetch_hooks_list(app_server.request_handle(), cwd.clone()).await {
Ok(response) => response,
Err(err) => {
tracing::warn!("failed to load startup hook review state: {err:#}");
return Ok(StartupHooksReviewOutcome::Continue);
}
};
let entry = hooks_list_entry_for_cwd(response, &cwd);
if review_needed_count(&entry) == 0 {
return Ok(StartupHooksReviewOutcome::Continue);
}
run_startup_hooks_review_app(app_server, tui, config, entry).await
}
async fn run_startup_hooks_review_app(
app_server: &mut AppServerSession,
tui: &mut Tui,
config: &Config,
entry: HooksListEntry,
) -> Result<StartupHooksReviewOutcome> {
let keymap = RuntimeKeymap::from_config(&config.tui_keymap)
.map_err(|err| color_eyre::eyre::eyre!(err))?;
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
let app_event_tx = AppEventSender::new(tx_raw);
let mut trust_all_error = None;
let mut view = selection_view(
&entry,
trust_all_error.as_deref(),
/*trusting_all*/ false,
app_event_tx.clone(),
&keymap,
);
draw_view(tui, &view)?;
let tui_events = tui.event_stream();
tokio::pin!(tui_events);
loop {
let Some(event) = tui_events.next().await else {
return Ok(StartupHooksReviewOutcome::Continue);
};
match event {
TuiEvent::Key(key_event) => {
if matches!(key_event.kind, KeyEventKind::Press | KeyEventKind::Repeat) {
view.handle_key_event(key_event);
}
let Some(selection) = selected_choice(&mut view) else {
draw_view(tui, &view)?;
continue;
};
match selection {
StartupHooksReviewSelection::ReviewHooks => {
return Ok(StartupHooksReviewOutcome::OpenHooksBrowser(entry));
}
StartupHooksReviewSelection::ContinueWithoutTrusting => {
return Ok(StartupHooksReviewOutcome::Continue);
}
StartupHooksReviewSelection::TrustAllAndContinue => {
view = selection_view(
&entry,
trust_all_error.as_deref(),
/*trusting_all*/ true,
app_event_tx.clone(),
&keymap,
);
draw_view(tui, &view)?;
let result = write_hook_trusts(
app_server.request_handle(),
entry
.hooks
.iter()
.filter(|hook| hook_needs_review(hook))
.map(|hook| HookTrustUpdate {
key: hook.key.clone(),
current_hash: hook.current_hash.clone(),
})
.collect(),
)
.await
.map(|_| ())
.map_err(|err| format!("Failed to trust hooks: {err}"));
match result {
Ok(()) => return Ok(StartupHooksReviewOutcome::Continue),
Err(err) => {
trust_all_error = Some(err);
view = selection_view(
&entry,
trust_all_error.as_deref(),
/*trusting_all*/ false,
app_event_tx.clone(),
&keymap,
);
draw_view(tui, &view)?;
}
}
}
}
}
TuiEvent::Paste(_) => {}
TuiEvent::Draw | TuiEvent::Resize => draw_view(tui, &view)?,
}
}
}
fn selected_choice(view: &mut ListSelectionView) -> Option<StartupHooksReviewSelection> {
if !view.is_complete() {
return None;
}
match view.take_last_selected_index() {
Some(0) => Some(StartupHooksReviewSelection::ReviewHooks),
Some(1) => Some(StartupHooksReviewSelection::TrustAllAndContinue),
Some(2) | None => Some(StartupHooksReviewSelection::ContinueWithoutTrusting),
Some(_) => None,
}
}
fn selection_view(
entry: &HooksListEntry,
trust_all_error: Option<&str>,
trusting_all: bool,
app_event_tx: AppEventSender,
keymap: &RuntimeKeymap,
) -> ListSelectionView {
ListSelectionView::new(
selection_view_params(entry, trust_all_error, trusting_all, keymap),
app_event_tx,
keymap.list.clone(),
)
}
#[allow(clippy::disallowed_methods)]
fn selection_view_params(
entry: &HooksListEntry,
trust_all_error: Option<&str>,
trusting_all: bool,
keymap: &RuntimeKeymap,
) -> SelectionViewParams {
let count = review_needed_count(entry);
let count_line = match count {
1 => "1 hook is new or changed.".to_string(),
count => format!("{count} hooks are new or changed."),
};
let mut header = ColumnRenderable::new();
header.push(Line::from("Hooks need review".bold()));
header.push(Line::from(count_line).yellow());
header.push(Line::from(
"Hooks can run outside the sandbox after you trust them.".dim(),
));
if let Some(error) = trust_all_error {
header.push(Line::from(error.to_string()).red());
} else if trusting_all {
header.push(Line::from("Trusting hooks...".dim()));
}
SelectionViewParams {
footer_hint: Some(standard_popup_hint_line_for_keymap(&keymap.list)),
items: vec![
selection_item("Review hooks", trusting_all),
selection_item("Trust all and continue", trusting_all),
selection_item("Continue without trusting (hooks won't run)", trusting_all),
],
header: Box::new(header),
..Default::default()
}
}
fn review_needed_count(entry: &HooksListEntry) -> usize {
entry
.hooks
.iter()
.filter(|hook| hook_needs_review(hook))
.count()
}
fn selection_item(name: &str, is_disabled: bool) -> SelectionItem {
SelectionItem {
name: name.to_string(),
dismiss_on_select: true,
is_disabled,
..Default::default()
}
}
fn draw_view(tui: &mut Tui, view: &ListSelectionView) -> Result<()> {
tui.draw(u16::MAX, |frame| {
let area = frame.area();
frame.render_widget_ref(Clear, area);
let view_area = Rect::new(
area.x,
area.y,
area.width,
view.desired_height(area.width).min(area.height),
);
frame.render_widget_ref(&StandaloneSelectionView { view }, view_area);
})?;
Ok(())
}
struct StandaloneSelectionView<'a> {
view: &'a ListSelectionView,
}
impl WidgetRef for &StandaloneSelectionView<'_> {
fn render_ref(&self, area: Rect, buf: &mut Buffer) {
self.view.render(area, buf);
}
}
#[cfg(test)]
mod tests {
use super::selection_view;
use crate::app_event::AppEvent;
use crate::app_event_sender::AppEventSender;
use crate::keymap::RuntimeKeymap;
use crate::render::renderable::Renderable;
use crate::test_support::PathBufExt;
use crate::test_support::test_path_buf;
use codex_app_server_protocol::HookEventName;
use codex_app_server_protocol::HookHandlerType;
use codex_app_server_protocol::HookMetadata;
use codex_app_server_protocol::HookSource;
use codex_app_server_protocol::HookTrustStatus;
use codex_app_server_protocol::HooksListEntry;
use insta::assert_snapshot;
use ratatui::buffer::Buffer;
use ratatui::layout::Rect;
use tokio::sync::mpsc::unbounded_channel;
fn hook(key: &str, trust_status: HookTrustStatus) -> HookMetadata {
HookMetadata {
key: key.to_string(),
event_name: HookEventName::PreToolUse,
handler_type: HookHandlerType::Command,
is_managed: false,
matcher: Some("Bash".to_string()),
command: Some("/tmp/hook.sh".to_string()),
timeout_sec: 30,
status_message: None,
source_path: test_path_buf("/tmp/hooks.json").abs(),
source: HookSource::User,
plugin_id: None,
display_order: 0,
enabled: false,
current_hash: format!("sha256:{key}"),
trust_status,
}
}
fn entry() -> HooksListEntry {
HooksListEntry {
cwd: test_path_buf("/tmp"),
hooks: vec![
hook("path:new", HookTrustStatus::Untrusted),
hook("path:changed", HookTrustStatus::Modified),
],
warnings: Vec::new(),
errors: Vec::new(),
}
}
fn render_lines(view: &crate::bottom_pane::ListSelectionView, width: u16) -> String {
let height = view.desired_height(width);
let area = Rect::new(0, 0, width, height);
let mut buf = Buffer::empty(area);
view.render(area, &mut buf);
(0..area.height)
.map(|row| {
let rendered = (0..area.width)
.map(|col| {
let symbol = buf[(area.x + col, area.y + row)].symbol();
if symbol.is_empty() {
" ".to_string()
} else {
symbol.to_string()
}
})
.collect::<String>();
format!("{rendered:width$}", width = area.width as usize)
})
.collect::<Vec<_>>()
.join("\n")
}
#[test]
fn renders_prompt() {
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
let keymap = RuntimeKeymap::defaults();
let view = selection_view(
&entry(),
/*trust_all_error*/ None,
/*trusting_all*/ false,
AppEventSender::new(tx_raw),
&keymap,
);
assert_snapshot!(
"startup_hooks_review_prompt",
render_lines(&view, /*width*/ 80)
);
}
#[test]
fn renders_prompt_with_trust_error() {
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
let keymap = RuntimeKeymap::defaults();
let view = selection_view(
&entry(),
Some("Failed to trust hooks: disk full"),
/*trusting_all*/ false,
AppEventSender::new(tx_raw),
&keymap,
);
assert_snapshot!(
"startup_hooks_review_prompt_with_trust_error",
render_lines(&view, /*width*/ 80)
);
}
}