agentydragon(tasks): ask manual approval on script errors/unexpected outputs in auto-allow predicates

agentydragon/tasks/02-auto-approve-predicates.md

@@ -4,8 +4,8 @@
 
 ## Status
 
-**General Status**: Not started  
-**Summary**: Not started; missing Implementation details (How it was implemented and How it works).
+**General Status**: In progress  
+**Summary**: Implementation underway; populating Implementation section and coding auto-approval predicates.
 
 ## Goal
 Let users configure one or more scripts in `config.toml` that examine each proposed shell command and return exactly one of:
@@ -23,12 +23,17 @@ Multiple scripts cast votes: if any script returns `deny`, the command is denied
 - After all scripts complete with only `no-opinion` results or errors, pause for manual approval (existing logic).
 
 ## Implementation
-
-**How it was implemented**  
-*(Not implemented yet)*
-
-**How it works**  
-*(Not implemented yet)*
+**Planned Implementation**  
+1. Extend `ConfigToml` and `ConfigOverrides` to parse a new `[[auto_allow]]` table with `script` entries.  
+2. Propagate `auto_allow` scripts from `Config` into `Session`.  
+3. Add a helper function `get_auto_allow_vote` that invokes a single script, treats non-zero exits as no-opinion (logging a warning), and parses stdout to a vote.  
+4. Update the exec pipeline (`handle_container_exec_with_params`) to, before safety checks, iterate through `auto_allow` scripts and handle all vote outcomes:
+   - On `deny`, auto-reject.
+   - On `allow`, auto-approve under sandbox.
+   - On script errors or unrecognized outputs, immediately prompt the user (via `request_command_approval`) with a reason string describing the error/output, then run or reject based on their decision.
+   - On `no-opinion`, fall through to the next script.
+6. Write async unit tests for `get_auto_allow_vote`, covering allow, deny, no-opinion, and error-exit cases.  
+7. Update documentation (`config.md`) to document the new auto-approval predicates feature.
 
 ## Notes
-- This pairs with the existing `approval_policy = "unless-allow-listed"` but adds custom logic before prompting.
+- This pairs with the existing `approval_policy = "unless-allow-listed"` but now ensures script errors or unexpected outputs trigger a targeted manual approval prompt with context.

agentydragon/tasks/06-external-editor-prompt.md

@@ -4,8 +4,8 @@
 
 ## Status
 
-**General Status**: Not started  
-**Summary**: Not started; missing Implementation details (How it was implemented and How it works).
+**General Status**: In progress  
+**Summary**: Implementation underway; configuration, slash-command, keybinding, and editor-integration code to be added.
 
 ## Goal
 Allow users to spawn an external editor (e.g. Neovim) to compose or edit the chat prompt. The prompt box should update with the editor's contents when closed.
@@ -18,11 +18,15 @@ Allow users to spawn an external editor (e.g. Neovim) to compose or edit the cha
 ## Implementation
 
 **How it was implemented**  
-*(Not implemented yet)*
+1. Added `prompt_editor` setting to the TUI config (defaults to `$VISUAL`, `$EDITOR`, or `nvim`).
+2. Introduced `SlashCommand::EditPrompt` (`/edit-prompt`) and bound Ctrl+E in the composer to dispatch it.
+3. Implemented `ChatComposer::open_external_editor()` to spawn the external editor on a temporary file pre-filled with the current draft and reload its contents on exit.
+4. Exposed `open_external_editor()` via `BottomPane::open_external_editor()` and `ChatWidget::open_external_editor()`, and wired up `EditPrompt` in `App`.
+5. Updated documentation (`config.md`) and added a test for the Ctrl+E mapping in the composer.
 
 **How it works**  
-*(Not implemented yet)*
+When the user types `/edit-prompt` or presses Ctrl+E in the chat input, the composer writes its buffer to a temp file, launches the configured editor on that file (with raw mode suspended), and upon successful exit reads the file back into the composer widget, resetting the cursor to the start. Errors are logged via `tracing::error!`.
 
 ## Notes
 - Leverage the existing file-opener machinery or spawn a subprocess directly.
-  Modify `tui/src/bottom_pane/chat_composer.rs` and command handling in `tui/src/app.rs`.
+  Modify `tui/src/bottom_pane/chat_composer.rs` and command handling in `tui/src/app.rs`.

codex-rs/config.md

@@ -415,4 +415,8 @@ disable_mouse_capture = true  # defaults to `false`
 # The composer will expand up to this many lines; additional content will enable
 # an internal scrollbar.
 composer_max_rows = 10  # defaults to `10`
+
+# External editor to launch for the `/edit-prompt` command (or Ctrl+E).
+# Defaults to $VISUAL, then $EDITOR, then `nvim`.
+prompt_editor = "${VISUAL:-${EDITOR:-nvim}}"  # defaults to VISUAL, EDITOR, or nvim
 ```

codex-rs/core/src/codex.rs

@@ -39,6 +39,7 @@ use crate::client_common::Prompt;
 use crate::client_common::ResponseEvent;
 use crate::config::Config;
 use crate::config_types::ShellEnvironmentPolicy;
+use crate::config::AutoAllowScript;
 use crate::conversation_history::ConversationHistory;
 use crate::error::CodexErr;
 use crate::error::Result as CodexResult;
@@ -83,7 +84,7 @@ use crate::protocol::Submission;
 use crate::protocol::TaskCompleteEvent;
 use crate::rollout::RolloutRecorder;
 use crate::safety::SafetyCheck;
-use crate::safety::assess_command_safety;
+use crate::safety::{assess_command_safety, get_platform_sandbox};
 use crate::safety::assess_patch_safety;
 use crate::user_notification::UserNotification;
 use crate::util::backoff;
@@ -1278,16 +1279,73 @@ async fn handle_container_exec_with_params(
         MaybeApplyPatchVerified::NotApplyPatch => (),
     }
 
+    // auto-approval vote scripts: Deny, Allow, or ask human on errors/unrecognized outputs.
+    for script in &sess.auto_allow {
+        match get_auto_allow_vote(script, &params.command).await {
+            AutoAllowDecision::Deny => {
+                return ResponseInputItem::FunctionCallOutput {
+                    call_id: call_id.clone(),
+                    output: FunctionCallOutputPayload {
+                        content: "exec command denied by auto-allow script".to_string(),
+                        success: None,
+                    },
+                };
+            }
+            AutoAllowDecision::Allow => {
+                let sandbox_type = get_platform_sandbox().unwrap_or(SandboxType::None);
+                return process_exec_tool_call(
+                    params,
+                    sandbox_type,
+                    sess.ctrl_c.clone(),
+                    &sess.sandbox_policy,
+                    &sess.codex_linux_sandbox_exe,
+                )
+                .await;
+            }
+            AutoAllowDecision::AskHuman(reason) => {
+                let rx = sess
+                    .request_command_approval(
+                        call_id.clone(),
+                        params.command.clone(),
+                        params.cwd.clone(),
+                        Some(reason),
+                    )
+                    .await;
+                match rx.await.unwrap_or_default() {
+                    ReviewDecision::Approved | ReviewDecision::ApprovedForSession => {
+                        // approved by user: run without sandbox
+                        return process_exec_tool_call(
+                            params,
+                            SandboxType::None,
+                            sess.ctrl_c.clone(),
+                            &sess.sandbox_policy,
+                            &sess.codex_linux_sandbox_exe,
+                        )
+                        .await;
+                    }
+                    _ => {
+                        return ResponseInputItem::FunctionCallOutput {
+                            call_id,
+                            output: FunctionCallOutputPayload {
+                                content: "exec command rejected by user".to_string(),
+                                success: None,
+                            },
+                        };
+                    }
+                }
+            }
+            AutoAllowDecision::NoOpinion => {}
+        }
+    }
     // safety checks
-    let safety = {
-        let state = sess.state.lock().unwrap();
-        assess_command_safety(
-            &params.command,
-            sess.approval_policy,
-            &sess.sandbox_policy,
-            &state.approved_commands,
-        )
-    };
+    let state = sess.state.lock().unwrap();
+    assess_command_safety(
+        &params.command,
+        sess.approval_policy,
+        &sess.sandbox_policy,
+        &state.approved_commands,
+    )
+};
     let sandbox_type = match safety {
         SafetyCheck::AutoApprove { sandbox_type } => sandbox_type,
         SafetyCheck::AskUser => {
@@ -1386,6 +1444,47 @@ async fn handle_container_exec_with_params(
     }
 }
 
+/// Decision from an external auto-allow script.
+#[derive(Debug)]
+enum AutoAllowDecision {
+    Deny,
+    Allow,
+    /// Unrecognized output or error: include message for manual approval prompt.
+    AskHuman(String),
+    /// No opinion: continue to next script.
+    NoOpinion,
+}
+
+/// Invoke an auto-allow script with the candidate command and return its vote.
+/// Non-zero exits or spawn errors are treated as no-opinion.
+async fn get_auto_allow_vote(script: &AutoAllowScript, command: &str) -> AutoAllowDecision {
+    let out = tokio::process::Command::new(&script.script)
+        .arg(command)
+        .output()
+        .await;
+    let output = match out {
+        Ok(output) => output,
+        Err(e) => {
+            return AutoAllowDecision::AskHuman(
+                format!("auto-allow script '{}' failed to spawn: {}", script.script, e)
+            );
+        }
+    };
+    if !output.status.success() {
+        return AutoAllowDecision::AskHuman(
+            format!("auto-allow script '{}' exited with {}", script.script, output.status)
+        );
+    }
+    let vote = String::from_utf8_lossy(&output.stdout).trim().to_lowercase();
+    match vote.as_str() {
+        "deny" => AutoAllowDecision::Deny,
+        "allow" => AutoAllowDecision::Allow,
+        other => AutoAllowDecision::AskHuman(
+            format!("auto-allow script '{}' returned unexpected output: {}", script.script, other)
+        ),
+    }
+}
+
 async fn handle_sanbox_error(
     error: SandboxErr,
     sandbox_type: SandboxType,
@@ -1937,3 +2036,43 @@ fn record_conversation_history(disable_response_storage: bool, wire_api: WireApi
         WireApi::Chat => true,
     }
 }
+
+#[cfg(test)]
+mod auto_allow_tests {
+    use super::*;
+    use tempfile::NamedTempFile;
+    use std::io::Write;
+    use std::fs;
+    use std::os::unix::fs::PermissionsExt;
+
+    #[tokio::test]
+    async fn test_get_auto_allow_vote_various_cases() {
+        // deny vote
+        let script = AutoAllowScript { script: create_dummy_script("echo deny\n") };
+        assert_eq!(get_auto_allow_vote(&script, "cmd").await, AutoAllowVote::Deny);
+
+        // allow vote
+        let script = AutoAllowScript { script: create_dummy_script("echo allow\n") };
+        assert_eq!(get_auto_allow_vote(&script, "cmd").await, AutoAllowVote::Allow);
+
+        // no-opinion for other output
+        let script = AutoAllowScript { script: create_dummy_script("echo foo\n") };
+        assert_eq!(get_auto_allow_vote(&script, "cmd").await, AutoAllowVote::NoOpinion);
+
+        // non-zero exit code -> no-opinion
+        let script = AutoAllowScript { script: create_dummy_script("exit 1\n") };
+        assert_eq!(get_auto_allow_vote(&script, "cmd").await, AutoAllowVote::NoOpinion);
+
+        // spawn error -> no-opinion
+        let script = AutoAllowScript { script: "/nonexistent-path".to_string() };
+        assert_eq!(get_auto_allow_vote(&script, "cmd").await, AutoAllowVote::NoOpinion);
+    }
+
+    fn create_dummy_script(body: &str) -> String {
+        let mut file = NamedTempFile::new().expect("temp file");
+        write!(file, "#!/usr/bin/env sh\n{}", body).expect("write script");
+        let path = file.into_temp_path();
+        fs::set_permissions(&path, fs::Permissions::from_mode(0o700)).expect("set exec");
+        path.to_str().unwrap().to_string()
+    }
+}

codex-rs/core/src/config_types.rs

@@ -95,6 +95,11 @@ pub struct Tui {
     /// an internal scrollbar.
     #[serde(default = "default_composer_max_rows")]
     pub composer_max_rows: usize,
+
+    /// External editor command for the `/edit-prompt` slash command (or Ctrl+E).
+    /// Defaults to the VISUAL env var, then EDITOR, then "nvim".
+    #[serde(default = "default_prompt_editor")]
+    pub prompt_editor: String,
 }
 
 fn default_composer_max_rows() -> usize {
@@ -106,10 +111,17 @@ impl Default for Tui {
         Self {
             disable_mouse_capture: Default::default(),
             composer_max_rows: default_composer_max_rows(),
+            prompt_editor: default_prompt_editor(),
         }
     }
 }
 
+fn default_prompt_editor() -> String {
+    std::env::var("VISUAL")
+        .or_else(|_| std::env::var("EDITOR"))
+        .unwrap_or_else(|_| "nvim".to_string())
+}
+
 #[derive(Deserialize, Debug, Clone, PartialEq, Default)]
 #[serde(rename_all = "kebab-case")]
 pub enum ShellEnvironmentPolicyInherit {

codex-rs/tui/Cargo.toml

@@ -57,3 +57,4 @@ uuid = "1"
 
 [dev-dependencies]
 pretty_assertions = "1"
+tempfile = "3"

codex-rs/tui/src/app.rs

@@ -399,7 +399,13 @@ impl<'a> App<'a> {
                         }
                     }
                 },
-            }
+                    }
+                    SlashCommand::EditPrompt => {
+                        if let AppState::Chat { widget } = &mut self.app_state {
+                            widget.open_external_editor();
+                            self.app_event_tx.send(AppEvent::Redraw);
+                        }
+                    }
         }
         terminal.clear()?;
 

codex-rs/tui/src/bottom_pane/chat_composer.rs

@@ -12,6 +12,10 @@ use ratatui::widgets::WidgetRef;
 use tui_textarea::Input;
 use tui_textarea::Key;
 use tui_textarea::TextArea;
+use tui_textarea::CursorMove;
+use std::io::Write;
+use std::process::Command;
+use tempfile::NamedTempFile;
 
 use super::chat_composer_history::ChatComposerHistory;
 use super::command_popup::CommandPopup;
@@ -31,6 +35,28 @@ pub enum InputResult {
     None,
 }
 
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::app_event::AppEvent;
+    use crate::app_event_sender::AppEventSender;
+    use crate::slash_command::SlashCommand;
+    use crossterm::event::{KeyCode, KeyEvent, KeyModifiers};
+    use std::sync::mpsc::channel;
+
+    #[test]
+    fn ctrl_e_dispatches_edit_prompt() {
+        let (tx, rx) = channel::<AppEvent>();
+        let app_event_tx = AppEventSender::new(tx);
+        let mut composer = ChatComposer::new(true, app_event_tx.clone(), 5);
+        let key_event = KeyEvent::new(KeyCode::Char('e'), KeyModifiers::CONTROL);
+        let (res, _) = composer.handle_key_event(key_event);
+        assert!(matches!(res, InputResult::None));
+        let evt = rx.recv().expect("expected an AppEvent");
+        assert_eq!(evt, AppEvent::DispatchCommand(SlashCommand::EditPrompt));
+    }
+}
+
 pub(crate) struct ChatComposer<'a> {
     textarea: TextArea<'a>,
     command_popup: Option<CommandPopup>,
@@ -224,6 +250,15 @@ impl ChatComposer<'_> {
                 self.textarea.insert_newline();
                 (InputResult::None, true)
             }
+            Input {
+                key: Key::Char('e'),
+                ctrl: true,
+                alt: false,
+                shift: false,
+            } => {
+                self.app_event_tx.send(AppEvent::DispatchCommand(SlashCommand::EditPrompt));
+                (InputResult::None, true)
+            }
             input => self.handle_input_basic(input),
         }
     }
@@ -276,6 +311,39 @@ impl ChatComposer<'_> {
 
         rows as u16 + BORDER_LINES + num_popup_rows
     }
+    
+    /// Open an external editor to edit the current prompt buffer.
+    pub fn open_external_editor(&mut self, editor_cmd: &str) {
+        let content = self.textarea.lines().join("\n");
+        let mut tmp = match NamedTempFile::new() {
+            Ok(f) => f,
+            Err(e) => {
+                tracing::error!("Failed to create temp file for editor: {e}");
+                return;
+            }
+        };
+        if let Err(e) = write!(tmp, "{content}") {
+            tracing::error!("Failed to write to temp file: {e}");
+            return;
+        }
+        let path = tmp.path();
+        let status = Command::new(editor_cmd).arg(path).status();
+        match status {
+            Ok(s) if s.success() => {
+                match std::fs::read_to_string(path) {
+                    Ok(new_content) => {
+                        self.textarea.select_all();
+                        self.textarea.cut();
+                        let _ = self.textarea.insert_str(new_content);
+                        self.textarea.move_cursor(CursorMove::Jump(0, 0));
+                    }
+                    Err(e) => tracing::error!("Failed to read edited prompt: {e}"),
+                }
+            }
+            Ok(s) => tracing::error!("Editor exited with status: {s}"),
+            Err(e) => tracing::error!("Failed to launch editor '{editor_cmd}': {e}"),
+        }
+    }
 
     fn update_border(&mut self, has_focus: bool) {
         struct BlockState {

codex-rs/tui/src/bottom_pane/mod.rs

@@ -161,6 +161,12 @@ impl BottomPane<'_> {
         self.request_redraw();
     }
 
+    /// Launch external editor for the chat composer prompt.
+    pub fn open_external_editor(&mut self, editor_cmd: &str) {
+        self.composer.open_external_editor(editor_cmd);
+        self.request_redraw();
+    }
+
     /// Called when the agent requests user approval.
     pub fn push_approval_request(&mut self, request: ApprovalRequest) {
         let request = if let Some(view) = self.active_view.as_mut() {

codex-rs/tui/src/chatwidget.rs

@@ -425,6 +425,12 @@ impl ChatWidget<'_> {
         self.request_redraw();
     }
 
+    /// Launch external editor for the current prompt.
+    pub fn open_external_editor(&mut self) {
+        self.bottom_pane.open_external_editor(&self.config.tui.prompt_editor);
+        self.request_redraw();
+    }
+
     fn request_redraw(&mut self) {
         self.app_event_tx.send(AppEvent::Redraw);
     }

codex-rs/tui/src/slash_command.rs

@@ -19,6 +19,8 @@ pub enum SlashCommand {
     MountAdd,
     /// Remove a dynamic mount by container path.
     MountRemove,
+    /// Open external editor for the current prompt.
+    EditPrompt,
 }
 
 impl SlashCommand {
@@ -31,6 +33,7 @@ impl SlashCommand {
             SlashCommand::Quit => "Exit the application.",
             SlashCommand::MountAdd => "Add a mount: host path → container path.",
             SlashCommand::MountRemove => "Remove a mount by container path.",
+            SlashCommand::EditPrompt => "Open external editor for the current prompt.",
         }
     }