mirror of
https://github.com/openai/codex.git
synced 2026-11-03 08:22:30 +00:00
tui: retire /approvals and rename /autoreview to /approve (#21034)
## Why The TUI currently exposes overlapping command names for the same permissions flow: `/permissions` and the older `/approvals` alias. It also uses `/autoreview` for the manual retry flow, even though the action users take there is approving one denied auto-review request. This change makes the command surface consistent with the hard rebrand: - `/permissions` is the only command for permission settings. - `/approve` is the command for approving a recent auto-review denial. ## What changed - Removed the legacy `/approvals` slash command and its dispatch path. - Kept `/permissions` as the single permissions command shown and accepted by the TUI. - Renamed the auto-review denial command from `/autoreview` to `/approve`. - Updated nearby comments so they refer to `/permissions` rather than the retired `/approvals` name. ## Verification - Updated the slash-command unit test to assert that `AutoReview` now renders and parses as `approve`.
This commit is contained in:
1 parent
94800ecbbf
commit
5c1ec8f4fd
6 files changed
+12
-20
No files matched your search
@@ -302,8 +302,8 @@ struct AutoReviewMode {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// Enabling the Auto-review experiment in the TUI should also switch the
|
/// Enabling the Auto-review experiment in the TUI should also switch the
|
||||||
/// current `/approvals` settings to the matching Auto-review mode. Users
|
/// current `/permissions` settings to the matching Auto-review mode. Users
|
||||||
/// can still change `/approvals` afterward; this just assumes that opting into
|
/// can still change `/permissions` afterward; this just assumes that opting into
|
||||||
/// the experiment means they want Auto-review enabled immediately.
|
/// the experiment means they want Auto-review enabled immediately.
|
||||||
fn auto_review_mode() -> AutoReviewMode {
|
fn auto_review_mode() -> AutoReviewMode {
|
||||||
AutoReviewMode {
|
AutoReviewMode {
|
||||||
|
|||||||
@@ -203,7 +203,7 @@ impl App {
|
|||||||
let previous_approvals_reviewer = feature_config.approvals_reviewer;
|
let previous_approvals_reviewer = feature_config.approvals_reviewer;
|
||||||
if effective_enabled {
|
if effective_enabled {
|
||||||
// Persist the reviewer setting so future sessions keep the
|
// Persist the reviewer setting so future sessions keep the
|
||||||
// experiment's matching `/approvals` mode until the user
|
// experiment's matching `/permissions` mode until the user
|
||||||
// changes it explicitly.
|
// changes it explicitly.
|
||||||
feature_config.approvals_reviewer = auto_review_preset.approvals_reviewer;
|
feature_config.approvals_reviewer = auto_review_preset.approvals_reviewer;
|
||||||
feature_edits.push(ConfigEdit::SetPath {
|
feature_edits.push(ConfigEdit::SetPath {
|
||||||
@@ -323,7 +323,7 @@ impl App {
|
|||||||
.await;
|
.await;
|
||||||
// This uses `OverrideTurnContext` intentionally: toggling the
|
// This uses `OverrideTurnContext` intentionally: toggling the
|
||||||
// experiment should update the active thread's effective approval
|
// experiment should update the active thread's effective approval
|
||||||
// settings immediately, just like a `/approvals` selection. Without
|
// settings immediately, just like a `/permissions` selection. Without
|
||||||
// this runtime patch, the config edit would only affect future
|
// this runtime patch, the config edit would only affect future
|
||||||
// sessions or turns recreated from disk.
|
// sessions or turns recreated from disk.
|
||||||
let op = AppCommand::override_turn_context(
|
let op = AppCommand::override_turn_context(
|
||||||
|
|||||||
@@ -16,8 +16,7 @@ use crate::slash_command::SlashCommand;
|
|||||||
|
|
||||||
// Hide alias commands in the default popup list so each unique action appears once.
|
// Hide alias commands in the default popup list so each unique action appears once.
|
||||||
// `quit` is an alias of `exit`, so we skip `quit` here.
|
// `quit` is an alias of `exit`, so we skip `quit` here.
|
||||||
// `approvals` is an alias of `permissions`.
|
const ALIAS_COMMANDS: &[SlashCommand] = &[SlashCommand::Quit];
|
||||||
const ALIAS_COMMANDS: &[SlashCommand] = &[SlashCommand::Quit, SlashCommand::Approvals];
|
|
||||||
const COMMAND_COLUMN_WIDTH: ColumnWidthConfig = ColumnWidthConfig::new(
|
const COMMAND_COLUMN_WIDTH: ColumnWidthConfig = ColumnWidthConfig::new(
|
||||||
ColumnWidthMode::AutoAllRows,
|
ColumnWidthMode::AutoAllRows,
|
||||||
/*name_column_width*/ None,
|
/*name_column_width*/ None,
|
||||||
|
|||||||
@@ -8120,7 +8120,7 @@ impl ChatWidget {
|
|||||||
.send(AppEvent::PersistModelSelection { model, effort });
|
.send(AppEvent::PersistModelSelection { model, effort });
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Open the permissions popup (alias for /permissions).
|
/// Open the permissions popup.
|
||||||
pub(crate) fn open_approvals_popup(&mut self) {
|
pub(crate) fn open_approvals_popup(&mut self) {
|
||||||
self.open_permissions_popup();
|
self.open_permissions_popup();
|
||||||
}
|
}
|
||||||
@@ -8693,8 +8693,8 @@ impl ChatWidget {
|
|||||||
// new permission profile, so downstream policy-change hooks don't
|
// new permission profile, so downstream policy-change hooks don't
|
||||||
// re-trigger the warning.
|
// re-trigger the warning.
|
||||||
let mut accept_actions: Vec<SelectionAction> = Vec::new();
|
let mut accept_actions: Vec<SelectionAction> = Vec::new();
|
||||||
// Suppress the immediate re-scan only when a preset will be applied (i.e., via /approvals or
|
// Suppress the immediate re-scan only when a preset will be applied via
|
||||||
// /permissions), to avoid duplicate warnings from the ensuing policy change.
|
// /permissions, to avoid duplicate warnings from the ensuing policy change.
|
||||||
if preset.is_some() {
|
if preset.is_some() {
|
||||||
accept_actions.push(Box::new(|tx| {
|
accept_actions.push(Box::new(|tx| {
|
||||||
tx.send(AppEvent::SkipNextWorldWritableScan);
|
tx.send(AppEvent::SkipNextWorldWritableScan);
|
||||||
|
|||||||
@@ -237,9 +237,6 @@ impl ChatWidget {
|
|||||||
SlashCommand::Agent | SlashCommand::MultiAgents => {
|
SlashCommand::Agent | SlashCommand::MultiAgents => {
|
||||||
self.app_event_tx.send(AppEvent::OpenAgentPicker);
|
self.app_event_tx.send(AppEvent::OpenAgentPicker);
|
||||||
}
|
}
|
||||||
SlashCommand::Approvals => {
|
|
||||||
self.open_permissions_popup();
|
|
||||||
}
|
|
||||||
SlashCommand::Permissions => {
|
SlashCommand::Permissions => {
|
||||||
self.open_permissions_popup();
|
self.open_permissions_popup();
|
||||||
}
|
}
|
||||||
@@ -890,7 +887,6 @@ impl ChatWidget {
|
|||||||
| SlashCommand::Keymap
|
| SlashCommand::Keymap
|
||||||
| SlashCommand::Agent
|
| SlashCommand::Agent
|
||||||
| SlashCommand::MultiAgents
|
| SlashCommand::MultiAgents
|
||||||
| SlashCommand::Approvals
|
|
||||||
| SlashCommand::Permissions
|
| SlashCommand::Permissions
|
||||||
| SlashCommand::ElevateSandbox
|
| SlashCommand::ElevateSandbox
|
||||||
| SlashCommand::SandboxReadRoot
|
| SlashCommand::SandboxReadRoot
|
||||||
|
|||||||
@@ -15,7 +15,6 @@ pub enum SlashCommand {
|
|||||||
Model,
|
Model,
|
||||||
Fast,
|
Fast,
|
||||||
Ide,
|
Ide,
|
||||||
Approvals,
|
|
||||||
Permissions,
|
Permissions,
|
||||||
Keymap,
|
Keymap,
|
||||||
Vim,
|
Vim,
|
||||||
@@ -24,7 +23,7 @@ pub enum SlashCommand {
|
|||||||
#[strum(serialize = "sandbox-add-read-dir")]
|
#[strum(serialize = "sandbox-add-read-dir")]
|
||||||
SandboxReadRoot,
|
SandboxReadRoot,
|
||||||
Experimental,
|
Experimental,
|
||||||
#[strum(to_string = "autoreview")]
|
#[strum(to_string = "approve")]
|
||||||
AutoReview,
|
AutoReview,
|
||||||
Memories,
|
Memories,
|
||||||
Skills,
|
Skills,
|
||||||
@@ -117,7 +116,6 @@ impl SlashCommand {
|
|||||||
SlashCommand::Collab => "change collaboration mode (experimental)",
|
SlashCommand::Collab => "change collaboration mode (experimental)",
|
||||||
SlashCommand::Agent | SlashCommand::MultiAgents => "switch the active agent thread",
|
SlashCommand::Agent | SlashCommand::MultiAgents => "switch the active agent thread",
|
||||||
SlashCommand::Side => "start a side conversation in an ephemeral fork",
|
SlashCommand::Side => "start a side conversation in an ephemeral fork",
|
||||||
SlashCommand::Approvals => "choose what Codex is allowed to do",
|
|
||||||
SlashCommand::Permissions => "choose what Codex is allowed to do",
|
SlashCommand::Permissions => "choose what Codex is allowed to do",
|
||||||
SlashCommand::Keymap => "remap TUI shortcuts",
|
SlashCommand::Keymap => "remap TUI shortcuts",
|
||||||
SlashCommand::Vim => "toggle Vim mode for the composer",
|
SlashCommand::Vim => "toggle Vim mode for the composer",
|
||||||
@@ -184,7 +182,6 @@ impl SlashCommand {
|
|||||||
| SlashCommand::Model
|
| SlashCommand::Model
|
||||||
| SlashCommand::Fast
|
| SlashCommand::Fast
|
||||||
| SlashCommand::Personality
|
| SlashCommand::Personality
|
||||||
| SlashCommand::Approvals
|
|
||||||
| SlashCommand::Permissions
|
| SlashCommand::Permissions
|
||||||
| SlashCommand::Keymap
|
| SlashCommand::Keymap
|
||||||
| SlashCommand::Vim
|
| SlashCommand::Vim
|
||||||
@@ -274,10 +271,10 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn auto_review_command_is_autoreview() {
|
fn auto_review_command_is_approve() {
|
||||||
assert_eq!(SlashCommand::AutoReview.command(), "autoreview");
|
assert_eq!(SlashCommand::AutoReview.command(), "approve");
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
SlashCommand::from_str("autoreview"),
|
SlashCommand::from_str("approve"),
|
||||||
Ok(SlashCommand::AutoReview)
|
Ok(SlashCommand::AutoReview)
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user