Add hook trust review flow to TUI

This commit is contained in:
Abhinav Vedmala
2026-04-30 14:10:58 -07:00
parent c9d5b38074
commit b8dbc57d6f
18 changed files with 390 additions and 33 deletions

View File

@@ -926,6 +926,7 @@ 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 {

View File

@@ -5,6 +5,7 @@
//! 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;
@@ -88,6 +89,47 @@ 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::Changed
)
})
.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();
@@ -322,6 +364,24 @@ impl App {
});
}
pub(super) fn trust_hook(
&mut self,
app_server: &AppServerSession,
key: String,
trusted_hash: String,
enable: bool,
) {
let request_handle = app_server.request_handle();
let app_event_tx = self.app_event_tx.clone();
tokio::spawn(async move {
let result = write_hook_trust(request_handle, key, trusted_hash, enable)
.await
.map(|_| ())
.map_err(|err| format!("Failed to trust hook: {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();
@@ -805,6 +865,45 @@ 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,
trusted_hash: String,
enable: bool,
) -> Result<ConfigWriteResponse> {
let request_id = RequestId::String(format!("hooks-config-write-{}", Uuid::new_v4()));
let value = if enable {
serde_json::json!({
key: {
"enabled": true,
"trusted_hash": trusted_hash,
}
})
} else {
serde_json::json!({
key: {
"trusted_hash": trusted_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>,

View File

@@ -1692,6 +1692,13 @@ impl App {
AppEvent::SetHookEnabled { key, enabled } => {
self.set_hook_enabled(app_server, key, enabled);
}
AppEvent::TrustHook {
key,
trusted_hash,
enable,
} => {
self.trust_hook(app_server, key, trusted_hash, enable);
}
AppEvent::HookEnabledSet {
key,
enabled,
@@ -1716,6 +1723,11 @@ impl App {
}
}
}
AppEvent::HookTrusted { result } => {
if let Err(err) = result {
self.chat_widget.add_error_message(err);
}
}
AppEvent::OpenPermissionsPopup => {
self.chat_widget.open_permissions_popup();
}

View File

@@ -77,6 +77,16 @@ 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,

View File

@@ -263,6 +263,16 @@ 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(2)
.expect("review-needed hooks should produce a startup warning");
let rendered =
lines_to_single_string(&history_cell::new_warning_event(message).display_lines(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;

View File

@@ -749,6 +749,13 @@ pub(crate) enum AppEvent {
enabled: bool,
},
/// Trust the current definition for a hook by stable hook key.
TrustHook {
key: String,
trusted_hash: String,
enable: bool,
},
/// Result of persisting hook enabled state.
HookEnabledSet {
key: String,
@@ -756,6 +763,11 @@ pub(crate) enum AppEvent {
result: Result<(), String>,
},
/// Result of persisting hook trust state.
HookTrusted {
result: Result<(), String>,
},
/// Notify that the manage skills popup was closed.
ManageSkillsClosed,

View File

@@ -68,7 +68,12 @@ impl HooksBrowserView {
app_event_tx,
};
if view.page_len() > 0 {
view.state.selected_idx = Some(0);
view.state.selected_idx = Some(
view.event_rows()
.iter()
.position(|row| row.needs_review > 0)
.unwrap_or(0),
);
}
view
}
@@ -87,10 +92,16 @@ impl HooksBrowserView {
.iter()
.filter(|hook| hook.event_name == event_name && hook_is_active(hook))
.count();
let needs_review = self
.hooks
.iter()
.filter(|hook| hook.event_name == event_name && hook_needs_review(hook))
.count();
EventRow {
event_name,
installed,
active,
needs_review,
}
})
.collect()
@@ -168,6 +179,9 @@ impl HooksBrowserView {
if hook.is_managed {
return;
}
if hook_needs_review(hook) {
return;
}
hook.enabled = !hook.enabled;
self.app_event_tx.send(AppEvent::SetHookEnabled {
@@ -176,6 +190,29 @@ impl HooksBrowserView {
});
}
fn trust_selected_hook(&mut self, event_name: HookEventName) {
let Some(idx) = self.selected_hook_index(event_name) else {
return;
};
let Some(hook) = self.hooks.get_mut(idx) else {
return;
};
if !hook_needs_review(hook) {
return;
}
let enable = !hook.enabled;
hook.trust_status = HookTrustStatus::Trusted;
if enable {
hook.enabled = true;
}
self.app_event_tx.send(AppEvent::TrustHook {
key: hook.key.clone(),
current_hash: hook.current_hash.clone(),
enable,
});
}
fn close(&mut self) {
self.complete = true;
}
@@ -207,7 +244,7 @@ impl HooksBrowserView {
fn handler_header_lines(event_name: HookEventName) -> Vec<Line<'static>> {
vec![
format!("{} hooks", event_label(event_name)).bold().into(),
"Turn hooks on or off. Your changes are saved automatically."
"Review new or changed hooks, then turn them on or off."
.dim()
.into(),
]
@@ -219,6 +256,7 @@ impl HooksBrowserView {
format!("{:<EVENT_COLUMN_WIDTH$}", "Event").into(),
format!("{:<COUNT_COLUMN_WIDTH$}", "Installed").into(),
format!("{:<COUNT_COLUMN_WIDTH$}", "Active").into(),
format!("{:<COUNT_COLUMN_WIDTH$}", "Review").into(),
"Description".into(),
]));
for (idx, row) in self.event_rows().into_iter().enumerate() {
@@ -236,6 +274,9 @@ impl HooksBrowserView {
Span::from(format!("{:<COUNT_COLUMN_WIDTH$}", row.active))
.cyan()
.bold(),
Span::from(format!("{:<COUNT_COLUMN_WIDTH$}", row.needs_review))
.cyan()
.bold(),
Span::from(event_description(row.event_name)).cyan().bold(),
]));
} else {
@@ -246,6 +287,7 @@ impl HooksBrowserView {
)),
Span::from(format!("{:<COUNT_COLUMN_WIDTH$}", row.installed)).dim(),
Span::from(format!("{:<COUNT_COLUMN_WIDTH$}", row.active)).dim(),
Span::from(format!("{:<COUNT_COLUMN_WIDTH$}", row.needs_review)).dim(),
Span::from(event_description(row.event_name)).dim(),
]));
}
@@ -291,8 +333,22 @@ impl HooksBrowserView {
self.handlers_for_event(event_name)
.enumerate()
.map(|(idx, hook)| {
let marker = if hook_is_active(hook) { 'x' } else { ' ' };
let row = format!("[{marker}] {}", hook_title(idx));
let marker = if hook_needs_review(hook) {
'!'
} else if hook_is_active(hook) {
'x'
} else {
' '
};
let row = match hook.trust_status {
HookTrustStatus::Modified => {
format!("[{marker}] {} · modified", hook_title(idx))
}
HookTrustStatus::Untrusted => format!("[{marker}] {} · new", hook_title(idx)),
HookTrustStatus::Managed | HookTrustStatus::Trusted => {
format!("[{marker}] {}", hook_title(idx))
}
};
let mut line = Line::from(row);
line = truncate_line_with_ellipsis_if_overflow(line, width);
if hook.is_managed {
@@ -330,6 +386,7 @@ impl HooksBrowserView {
Some(MAX_COMMAND_DETAIL_LINES),
));
lines.push(detail_line("Timeout", &format!("{}s", hook.timeout_sec)));
lines.push(detail_line("Trust", hook_trust_label(hook.trust_status)));
lines
}
@@ -362,6 +419,14 @@ impl HooksBrowserView {
key_hint::plain(KeyCode::Esc).into(),
" to go back".into(),
])
} else if selected_hook.is_some_and(hook_needs_review) {
Line::from(vec![
"Press ".into(),
key_hint::plain(KeyCode::Char('t')).into(),
" to trust; ".into(),
key_hint::plain(KeyCode::Esc).into(),
" to go back".into(),
])
} else {
Line::from(vec![
"Press ".into(),
@@ -422,6 +487,15 @@ impl BottomPaneView for HooksBrowserView {
self.toggle_selected_hook(event_name);
}
}
KeyEvent {
code: KeyCode::Char('t'),
modifiers: KeyModifiers::NONE,
..
} => {
if let HooksBrowserPage::Handlers(event_name) = self.page {
self.trust_selected_hook(event_name);
}
}
KeyEvent {
code: KeyCode::Esc, ..
} => match self.page {
@@ -532,6 +606,23 @@ struct EventRow {
event_name: HookEventName,
installed: usize,
active: usize,
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",
HookTrustStatus::Trusted => "Trusted",
HookTrustStatus::Untrusted => "New hook - review required",
HookTrustStatus::Modified => "Modified since last trusted - review required",
}
}
fn event_label(event_name: HookEventName) -> &'static str {
@@ -987,14 +1078,14 @@ mod tests {
}
#[test]
fn untrusted_enabled_hooks_do_not_count_as_active() {
fn review_needed_hooks_are_not_active() {
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
let mut untrusted_hook = hook(
"path:untrusted",
HookEventName::PreToolUse,
HookSource::User,
/*plugin_id*/ None,
"~/bin/untrusted.sh",
"/tmp/pre-tool-use-check.sh",
/*enabled*/ true,
/*is_managed*/ false,
/*display_order*/ 0,
@@ -1015,6 +1106,62 @@ mod tests {
assert_eq!(pre_tool_use.installed, 1);
assert_eq!(pre_tool_use.active, 0);
assert_eq!(pre_tool_use.needs_review, 1);
}
#[test]
fn review_needed_event_is_selected_by_default() {
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
let mut untrusted_hook = hook(
"path:untrusted",
HookEventName::PermissionRequest,
HookSource::User,
/*plugin_id*/ None,
"/tmp/permission-request-check.sh",
/*enabled*/ false,
/*is_managed*/ false,
/*display_order*/ 0,
);
untrusted_hook.trust_status = HookTrustStatus::Untrusted;
let view = HooksBrowserView::new(
vec![untrusted_hook],
Vec::new(),
Vec::new(),
AppEventSender::new(tx_raw),
);
assert_eq!(
view.selected_event(),
Some(HookEventName::PermissionRequest)
);
}
#[test]
fn renders_review_needed_handler() {
let (tx_raw, _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 view = HooksBrowserView::new(
vec![untrusted_hook],
Vec::new(),
Vec::new(),
AppEventSender::new(tx_raw),
);
view.handle_key_event(KeyEvent::from(KeyCode::Enter));
assert_snapshot!(
"hooks_browser_review_needed_handler",
render_lines(&view, /*width*/ 112)
);
}
fn assert_unmanaged_toggle_key(key_code: KeyCode) {
@@ -1077,6 +1224,44 @@ mod tests {
assert!(rx.try_recv().is_err());
}
#[test]
fn trust_key_trusts_review_needed_handler() {
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 current_hash = untrusted_hook.current_hash.clone();
let mut view = HooksBrowserView::new(
vec![untrusted_hook],
Vec::new(),
Vec::new(),
AppEventSender::new(tx_raw),
);
view.handle_key_event(KeyEvent::from(KeyCode::Enter));
view.handle_key_event(KeyEvent::from(KeyCode::Char('t')));
match rx.try_recv().expect("trust event") {
AppEvent::TrustHook {
key,
current_hash: trusted_hash,
enable,
} => {
assert_eq!(key, "path:untrusted");
assert_eq!(trusted_hash, current_hash);
assert!(enable);
}
other => panic!("expected hook trust event, got {other:?}"),
}
}
#[test]
fn escape_returns_to_the_selected_event() {
let mut view = view();

View File

@@ -4,7 +4,7 @@ expression: "render_lines(&view, 44)"
---
PreToolUse hooks
Turn hooks on or off. Your changes are s
Review new or changed hooks, then turn t
[x] Hook 1
@@ -15,5 +15,6 @@ expression: "render_lines(&view, 44)"
seven eight nine ten eleven
twelve thirteen fourteen…
Timeout 30s
Trust Trusted
Press space or enter to toggle; esc to go

View File

@@ -4,7 +4,7 @@ expression: "render_lines(&view, 112)"
---
PermissionRequest hooks
Turn hooks on or off. Your changes are saved automatically.
Review new or changed hooks, then turn them on or off.
No hooks installed for this event.

View File

@@ -6,12 +6,12 @@ expression: "render_lines(&view, 112)"
Hooks
Lifecycle hooks from config and enabled plugins.
Event Installed Active Description
PreToolUse 2 1 Before a tool executes
PermissionRequest 1 1 When permission is requested
PostToolUse 0 0 After a tool executes
SessionStart 0 0 When a new session starts
UserPromptSubmit 0 0 When the user submits a prompt
Stop 0 0 Right before Codex ends its turn
Event Installed Active Review Description
PreToolUse 2 1 0 Before a tool executes
PermissionRequest 1 1 0 When permission is requested
PostToolUse 0 0 0 After a tool executes
SessionStart 0 0 0 When a new session starts
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

View File

@@ -10,12 +10,12 @@ expression: "render_lines(&view, 112)"
⚠ skipped invalid matcher for PreToolUse
■ /tmp/hooks.json: failed to parse hooks config
Event Installed Active Description
PreToolUse 0 0 Before a tool executes
PermissionRequest 0 0 When permission is requested
PostToolUse 0 0 After a tool executes
SessionStart 0 0 When a new session starts
UserPromptSubmit 0 0 When the user submits a prompt
Stop 0 0 Right before Codex ends its turn
Event Installed Active Review Description
PreToolUse 0 0 0 Before a tool executes
PermissionRequest 0 0 0 When permission is requested
PostToolUse 0 0 0 After a tool executes
SessionStart 0 0 0 When a new session starts
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

View File

@@ -4,7 +4,7 @@ expression: "render_lines(&view, 112)"
---
PreToolUse hooks
Turn hooks on or off. Your changes are saved automatically.
Review new or changed hooks, then turn them on or off.
[x] Hook 1
[ ] Hook 2
@@ -14,5 +14,6 @@ expression: "render_lines(&view, 112)"
Source Plugin - superpowers@openai-curated
Command ${CODEX_PLUGIN_ROOT}/hooks/pre-tool-use-check.sh
Timeout 30s
Trust Trusted
Press space or enter to toggle; esc to go back

View File

@@ -4,7 +4,7 @@ expression: "render_lines(&view, 112)"
---
PermissionRequest hooks
Turn hooks on or off. Your changes are saved automatically.
Review new or changed hooks, then turn them on or off.
[x] Hook 1
@@ -13,5 +13,6 @@ expression: "render_lines(&view, 112)"
Source Admin config
Command /enterprise/hooks/permission-check.sh
Timeout 30s
Trust Managed
Managed hooks are always on; press esc to go back

View File

@@ -0,0 +1,18 @@
---
source: tui/src/bottom_pane/hooks_browser_view.rs
expression: "render_lines(&view, 112)"
---
PreToolUse hooks
Review new or changed hooks, then turn them on or off.
[!] Hook 1 · new
Event PreToolUse
Matcher Bash
Source User config - /tmp/hooks.json
Command /tmp/pre-tool-use-check.sh
Timeout 30s
Trust New hook - review required
Press t to trust; esc to go back

View File

@@ -4,7 +4,7 @@ expression: "render_lines(&view, 112)"
---
PreToolUse hooks
Turn hooks on or off. Your changes are saved automatically.
Review new or changed hooks, then turn them on or off.
[x] Hook 2
[x] Hook 3
@@ -20,5 +20,6 @@ expression: "render_lines(&view, 112)"
Source User config - /tmp/hooks.json
Command /tmp/hook-8.sh
Timeout 30s
Trust Trusted
Press space or enter to toggle; esc to go back

View File

@@ -4,7 +4,7 @@ expression: "render_lines(&view, 112)"
---
PreToolUse hooks
Turn hooks on or off. Your changes are saved automatically.
Review new or changed hooks, then turn them on or off.
[x] Hook 1
[x] Hook 2
@@ -14,5 +14,6 @@ expression: "render_lines(&view, 112)"
Source Admin config
Command /enterprise/hooks/pre-tool-use-2.sh
Timeout 30s
Trust Managed
Managed hooks are always on; press esc to go back

View File

@@ -9,12 +9,12 @@ expression: popup
⚠ skipped invalid matcher for PreToolUse
■ /tmp/hooks.json: failed to parse hooks config
Event Installed Active Description
PreToolUse 0 0 Before a tool executes
PermissionRequest 0 0 When permission is requested
PostToolUse 0 0 After a tool executes
SessionStart 0 0 When a new session starts
UserPromptSubmit 0 0 When the user submits a prompt
Stop 0 0 Right before Codex ends its turn
Event Installed Active Review Description
PreToolUse 0 0 0 Before a tool executes
PermissionRequest 0 0 0 When permission is requested
PostToolUse 0 0 0 After a tool executes
SessionStart 0 0 0 When a new session starts
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

View File

@@ -0,0 +1,5 @@
---
source: tui/src/app/tests.rs
expression: rendered
---
⚠ 2 hooks need review before they can run. Open /hooks to review them.