openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-23 18:08:45 +00:00
Code Issues Packages Projects Releases Wiki Activity

Address hook browser review feedback

Browse Source
This commit is contained in:
Abhinav Vedmala
2026-05-01 18:48:45 -07:00
parent 1370c133b5
commit 20fdb43399
12 changed files with 125 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/tui/src/bottom_pane/hooks_browser_view.rs
+86 -25
View File
@@ -241,27 +241,46 @@ impl HooksBrowserView {
]
}
fn handler_header_lines(event_name: HookEventName) -> Vec<Line<'static>> {
vec![
format!("{} hooks", event_label(event_name)).bold().into(),
"Review new or changed hooks, then turn them on or off."
.dim()
.into(),
]
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 => {}
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(),
),
}
lines
}
fn review_needed_count(&self, event_name: HookEventName) -> usize {
self.handlers_for_event(event_name)
.filter(|hook| hook_needs_review(hook))
.count()
}
fn event_table_lines(&self) -> Vec<Line<'static>> {
let rows = self.event_rows();
let show_review = rows.iter().any(|row| row.needs_review > 0);
let mut lines = Vec::new();
lines.push(Line::from(vec![
let mut header = vec![
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() {
];
if show_review {
header.push(format!("{:<COUNT_COLUMN_WIDTH$}", "Review").into());
}
header.push("Description".into());
lines.push(Line::from(header));
for (idx, row) in rows.into_iter().enumerate() {
if self.state.selected_idx == Some(idx) {
lines.push(Line::from(vec![
let mut row_line = vec![
Span::from(format!(
"{:<EVENT_COLUMN_WIDTH$}",
event_label(row.event_name)
@@ -274,22 +293,32 @@ 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(),
]));
];
if show_review {
row_line.push(
Span::from(format!("{:<COUNT_COLUMN_WIDTH$}", row.needs_review))
.cyan()
.bold(),
);
}
row_line.push(Span::from(event_description(row.event_name)).cyan().bold());
lines.push(Line::from(row_line));
} else {
lines.push(Line::from(vec![
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(),
Span::from(format!("{:<COUNT_COLUMN_WIDTH$}", row.needs_review)).dim(),
Span::from(event_description(row.event_name)).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));
}
}
lines
@@ -527,11 +556,14 @@ impl Renderable for HooksBrowserView {
HooksBrowserPage::Events => self.event_page_lines().len(),
HooksBrowserPage::Handlers(event_name) => {
let row_count = self.handler_row_lines(event_name, content_width).len();
let header_line_count =
Self::handler_header_lines(event_name, self.review_needed_count(event_name))
.len();
if row_count == 0 {
Self::handler_header_lines(event_name).len() + 2
header_line_count + 2
} else {
let visible_row_count = row_count.min(MAX_POPUP_ROWS);
Self::handler_header_lines(event_name).len()
header_line_count
+ 1
+ visible_row_count
+ 1
@@ -554,7 +586,8 @@ impl Renderable for HooksBrowserView {
let lines = match self.page {
HooksBrowserPage::Events => self.event_page_lines(),
HooksBrowserPage::Handlers(event_name) => {
let mut lines = Self::handler_header_lines(event_name);
let mut lines =
Self::handler_header_lines(event_name, self.review_needed_count(event_name));
let rows = self.handler_row_lines(event_name, width);
if rows.is_empty() {
lines.push(Line::default());
@@ -872,6 +905,34 @@ mod tests {
assert_snapshot!("hooks_browser_events", render_lines(&view, /*width*/ 112));
}
#[test]
fn renders_event_browser_with_review_column_when_needed() {
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.trusted_hash = None;
untrusted_hook.trust_status = HookTrustStatus::Untrusted;
let view = HooksBrowserView::new(
vec![untrusted_hook],
Vec::new(),
Vec::new(),
AppEventSender::new(tx_raw),
);
assert_snapshot!(
"hooks_browser_events_with_review_column",
render_lines(&view, /*width*/ 112)
);
}
#[test]
fn renders_event_browser_with_issues() {
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_capped_command_details.snap
-1
View File
@@ -4,7 +4,6 @@ expression: "render_lines(&view, 44)"
---
PreToolUse hooks
Review new or changed hooks, then turn t
[x] Hook 1
codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_empty_handlers.snap
-1
View File
@@ -4,7 +4,6 @@ expression: "render_lines(&view, 112)"
---
PermissionRequest hooks
Review new or changed hooks, then turn them on or off.
No hooks installed for this event.
codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events.snap
+7 -7
View File
@@ -6,12 +6,12 @@ expression: "render_lines(&view, 112)"
Hooks
Lifecycle hooks from config and enabled plugins.
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
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
Press enter to view hooks; esc to close
codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events_with_issues.snap
+7 -7
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 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
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
Press enter to view hooks; esc to close
codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_events_with_review_column.snap
+17
View File
@@ -0,0 +1,17 @@
---
source: tui/src/bottom_pane/hooks_browser_view.rs
expression: "render_lines(&view, 112)"
---
Hooks
Lifecycle hooks from config and enabled plugins.
Event Installed Active Review Description
PreToolUse 1 0 1 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
codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_handlers.snap
-1
View File
@@ -4,7 +4,6 @@ expression: "render_lines(&view, 112)"
---
PreToolUse hooks
Review new or changed hooks, then turn them on or off.
[x] Hook 1
[ ] Hook 2
codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_managed_handler.snap
-1
View File
@@ -4,7 +4,6 @@ expression: "render_lines(&view, 112)"
---
PermissionRequest hooks
Review new or changed hooks, then turn them on or off.
[x] Hook 1
codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_review_needed_handler.snap
+1 -1
View File
@@ -4,7 +4,7 @@ expression: "render_lines(&view, 112)"
---
PreToolUse hooks
Review new or changed hooks, then turn them on or off.
1 hook needs review before it can run.
[!] Hook 1 · new
codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_scrolled_handlers.snap
-1
View File
@@ -4,7 +4,6 @@ expression: "render_lines(&view, 112)"
---
PreToolUse hooks
Review new or changed hooks, then turn them on or off.
[x] Hook 2
[x] Hook 3
codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_selected_managed_handler.snap
-1
View File
@@ -4,7 +4,6 @@ expression: "render_lines(&view, 112)"
---
PreToolUse hooks
Review new or changed hooks, then turn them on or off.
[x] Hook 1
[x] Hook 2
codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__hooks_popup_shows_list_diagnostics.snap
+7 -7
View File
@@ -9,12 +9,12 @@ expression: popup
⚠ skipped invalid matcher for PreToolUse
■ /tmp/hooks.json: failed to parse hooks config
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
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
Press enter to view hooks; esc to close