Code Review Bundle (Curated, Single File)

Date: 2025-09-28
Scope: Generic pre‑hooks (core config + exec wiring)

Review Focus

Config correctness: TOML → runtime mapping for [pre_hooks] and steps (cmd/env/cwd/timeout).
Exec integration: CLI flag → override translation; hooks run before agent submission; error semantics.
Runner safety: timeouts, required vs non‑fatal, env/cwd application.
Demos: Makefile/wrapper targets for local validation.

Files To Review (with context)

codex-rs/exec/src/pre_hooks.rs

use std::path::PathBuf;
use std::time::Duration;

use codex_core::config::Config;
use tokio::process::Command;
use tokio::time::timeout;
use tracing::info;

fn describe_cmd(cmd: &[String]) -> String {
    if cmd.is_empty() {
        return "<empty>".to_string();
    }
    cmd.join(" ")
}

pub async fn run_pre_hooks(config: &Config) -> anyhow::Result<()> {
    let Some(pre) = config.pre_hooks.as_ref() else {
        return Ok(());
    };
    if !pre.enable || pre.steps.is_empty() {
        return Ok(());
    }

    info!("Running {} pre-hook step(s)", pre.steps.len());

    for (idx, step) in pre.steps.iter().enumerate() {
        if step.cmd.is_empty() {
            continue;
        }
        let mut cmd = Command::new(&step.cmd[0]);
        if step.cmd.len() > 1 {
            cmd.args(&step.cmd[1..]);
        }

        // Per-step cwd falls back to config.cwd
        let cwd: PathBuf = step.cwd.clone().unwrap_or_else(|| config.cwd.clone());
        cmd.current_dir(&cwd);

        // Merge step envs
        if !step.env.is_empty() {
            cmd.envs(step.env.clone());
        }

        eprintln!(
            "[pre-hooks] ({}/{}) {}",
            idx + 1,
            pre.steps.len(),
            describe_cmd(&step.cmd)
        );

        let run = async {
            let status = cmd.status().await?;
            anyhow::Ok(status.success())
        };

        let success = if let Some(ms) = step.timeout_ms {
            match timeout(Duration::from_millis(ms), run).await {
                Ok(Ok(ok)) => ok,
                Ok(Err(e)) => return Err(e),
                Err(_) => false, // timed out
            }
        } else {
            run.await?
        };

        if !success {
            let is_required = step.required || pre.required;
            if is_required {
                anyhow::bail!("pre-hook failed (required): {}", describe_cmd(&step.cmd));
            } else {
                eprintln!("[pre-hooks] non-fatal failure: {}", describe_cmd(&step.cmd));
            }
        }
    }

    Ok(())
}

#[cfg(test)]
mod tests {
    use super::*;
    use codex_core::config::ConfigOverrides;
    use codex_core::config::ConfigToml;
    use codex_core::config::PreHookStepToml;
    use codex_core::config::PreHooksToml;

    #[tokio::test]
    async fn runs_trivial_true_command() {
        let tmp = tempfile::tempdir().unwrap();
        let mut cfg = ConfigToml::default();
        cfg.pre_hooks = Some(PreHooksToml {
            enable: Some(true),
            required: Some(true),
            steps: vec![PreHookStepToml {
                cmd: vec!["true".to_string()],
                required: None,
                cwd: None,
                env: Default::default(),
                timeout_ms: None,
            }],
        });
        let config = Config::load_from_base_config_with_overrides(
            cfg,
            ConfigOverrides::default(),
            tmp.path().to_path_buf(),
        )
        .unwrap();

        run_pre_hooks(&config).await.unwrap();
    }
}

codex-rs/exec/src/lib.rs — CLI override translation for pre‑hooks

            let quoted: Vec<String> = tokens
                .into_iter()
                .map(|t| format!("\"{}\"", t.replace('\\', "\\\\").replace('"', "\\\"")))
                .collect();
            let cmd_array = format!("[{}]", quoted.join(","));
            tables.push(format!("{{cmd={cmd_array}}}"));
        }
        let array = format!("[{}]", tables.join(","));
        config_overrides
            .raw_overrides
            .push(format!("pre_hooks.steps={array}"));
    }

    // Parse `-c` overrides (including translated pre-hook flags).
    let cli_kv_overrides = match config_overrides.parse_overrides() {
        Ok(v) => v,
        Err(e) => {
            eprintln!("Error parsing -c overrides: {e}");
            std::process::exit(1);
        }
    };

    let config = Config::load_with_cli_overrides(cli_kv_overrides, overrides)?;
    let mut event_processor: Box<dyn EventProcessor> = match (json_mode, experimental_json) {
        (_, true) => Box::new(ExperimentalEventProcessorWithJsonOutput::new(
            last_message_file.clone(),
        )),
        (true, _) => {
            eprintln!(
                "The existing `--json` output format is being deprecated. Please try the new format using `--experimental-json`."
            );

            Box::new(EventProcessorWithJsonOutput::new(last_message_file.clone()))
        }
        _ => Box::new(EventProcessorWithHumanOutput::create_with_ansi(
            stdout_with_ansi,
            &config,
            last_message_file.clone(),
        )),
    };

codex-rs/exec/src/lib.rs — Invocation before agent submission

                .new_conversation(config.clone())
                .await?
        }
    } else {
        conversation_manager
            .new_conversation(config.clone())
            .await?
    };
    // Print the effective configuration and prompt so users can see what Codex
    // is using.
    event_processor.print_config_summary(&config, &prompt, &session_configured);

    info!("Codex initialized with event: {session_configured:?}");

    // Run generic pre-hooks, if enabled, before sending any input to the agent.
    if let Err(e) = pre_hooks::run_pre_hooks(&config).await {
        eprintln!("Pre-hooks failed: {e}");
        std::process::exit(1);
    }

    let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel::<Event>();
    {
        let conversation = conversation.clone();
        tokio::spawn(async move {
            loop {
                tokio::select! {
                    _ = tokio::signal::ctrl_c() => {
                        tracing::debug!("Keyboard interrupt");
                        // Immediately notify Codex to abort any in‑flight task.
                        conversation.submit(Op::Interrupt).await.ok();

                        // Exit the inner loop and return to the main input prompt. The codex
                        // will emit a `TurnInterrupted` (Error) event which is drained later.
                        break;
                    }
                    res = conversation.next_event() => match res {
                        Ok(event) => {
                            debug!("Received event: {event:?}");

                            let is_shutdown_complete = matches!(event.msg, EventMsg::ShutdownComplete);
                            if let Err(e) = tx.send(event) {

codex-rs/exec/src/cli.rs — Added flags

use clap::Parser;
use clap::ValueEnum;
use codex_common::CliConfigOverrides;
use std::path::PathBuf;

#[derive(Parser, Debug)]
#[command(version)]
pub struct Cli {
    /// Action to perform. If omitted, runs a new non-interactive session.
    #[command(subcommand)]
    pub command: Option<Command>,

    /// Optional image(s) to attach to the initial prompt.
    #[arg(long = "image", short = 'i', value_name = "FILE", value_delimiter = ',', num_args = 1..)]
    pub images: Vec<PathBuf>,

    /// Model the agent should use.
    #[arg(long, short = 'm')]
    pub model: Option<String>,

    #[arg(long = "oss", default_value_t = false)]
    pub oss: bool,

    /// Select the sandbox policy to use when executing model-generated shell
    /// commands.
    #[arg(long = "sandbox", short = 's', value_enum)]
    pub sandbox_mode: Option<codex_common::SandboxModeCliArg>,

    /// Configuration profile from config.toml to specify default options.
    #[arg(long = "profile", short = 'p')]
    pub config_profile: Option<String>,

    /// Convenience alias for low-friction sandboxed automatic execution (-a on-failure, --sandbox workspace-write).
    #[arg(long = "full-auto", default_value_t = false)]
    pub full_auto: bool,

    /// Skip all confirmation prompts and execute commands without sandboxing.
    /// EXTREMELY DANGEROUS. Intended solely for running in environments that are externally sandboxed.
    #[arg(
        long = "dangerously-bypass-approvals-and-sandbox",
        alias = "yolo",
        default_value_t = false,
        conflicts_with = "full_auto"
    )]
    pub dangerously_bypass_approvals_and_sandbox: bool,

    /// Tell the agent to use the specified directory as its working root.
    #[clap(long = "cd", short = 'C', value_name = "DIR")]
    pub cwd: Option<PathBuf>,

    /// Allow running Codex outside a Git repository.
    #[arg(long = "skip-git-repo-check", default_value_t = false)]
    pub skip_git_repo_check: bool,

    /// Path to a JSON Schema file describing the model's final response shape.
    #[arg(long = "output-schema", value_name = "FILE")]
    pub output_schema: Option<PathBuf>,

    /// Enable generic pre-hooks (runs before sending the prompt).
    #[arg(long = "pre-hooks-enable", default_value_t = false)]
    pub pre_hooks_enable: bool,

    /// Treat pre-hook failures as fatal (unless a step marks required=false).
    #[arg(long = "pre-hooks-required", default_value_t = false)]
    pub pre_hooks_required: bool,

    /// Command to run as a pre-hook step (may be repeated).
    #[arg(long = "pre-hook", value_name = "CMD", action = clap::ArgAction::Append)]
    pub pre_hook: Vec<String>,

    #[clap(skip)]
    pub config_overrides: CliConfigOverrides,

    /// Specifies color settings for use in the output.
    #[arg(long = "color", value_enum, default_value_t = Color::Auto)]
    pub color: Color,

    /// Print events to stdout as JSONL.
    #[arg(
        long = "json",
        default_value_t = false,
        conflicts_with = "experimental_json"
    )]
    pub json: bool,

    #[arg(
        long = "experimental-json",
        default_value_t = false,
        conflicts_with = "json"
    )]
    pub experimental_json: bool,

    /// Whether to include the plan tool in the conversation.
    #[arg(long = "include-plan-tool", default_value_t = false)]
    pub include_plan_tool: bool,

    /// Specifies file where the last message from the agent should be written.
    #[arg(long = "output-last-message")]
    pub last_message_file: Option<PathBuf>,

    /// Initial instructions for the agent. If not provided as an argument (or
    /// if `-` is used), instructions are read from stdin.
    #[arg(value_name = "PROMPT")]
    pub prompt: Option<String>,
}

#[derive(Debug, clap::Subcommand)]
pub enum Command {
    /// Resume a previous session by id or pick the most recent with --last.
    Resume(ResumeArgs),
}

#[derive(Parser, Debug)]
pub struct ResumeArgs {
    /// Conversation/session id (UUID). When provided, resumes this session.
    /// If omitted, use --last to pick the most recent recorded session.
    #[arg(value_name = "SESSION_ID")]
    pub session_id: Option<String>,

    /// Resume the most recent recorded session (newest) without specifying an id.
    #[arg(long = "last", default_value_t = false, conflicts_with = "session_id")]
    pub last: bool,

    /// Prompt to send after resuming the session. If `-` is used, read from stdin.
    #[arg(value_name = "PROMPT")]
    pub prompt: Option<String>,
}

#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, ValueEnum)]
#[value(rename_all = "kebab-case")]
pub enum Color {
    Always,
    Never,
    #[default]
    Auto,
}

codex-rs/core/src/config.rs — Types: PreHooksToml/PreHookStepToml

pub struct PreHooksToml {
    /// When true, pre-hooks are enabled.
    #[serde(default)]
    pub enable: Option<bool>,
    /// When true, any failing step is treated as fatal (unless a step overrides required=false).
    #[serde(default)]
    pub required: Option<bool>,
    /// Steps to execute in order; each step is a command and args.
    #[serde(default)]
    pub steps: Vec<PreHookStepToml>,
}

#[derive(Deserialize, Debug, Clone, Default, PartialEq)]
pub struct PreHookStepToml {
    /// Command and arguments to execute.
    #[serde(default)]
    pub cmd: Vec<String>,
    /// If set, overrides the global `required` for this step.
    #[serde(default)]
    pub required: Option<bool>,
    /// Optional working directory for the step.
    pub cwd: Option<PathBuf>,
    /// Optional environment variables for the step.
    #[serde(default)]
    pub env: HashMap<String, String>,
    /// Optional timeout for the step in milliseconds.
    pub timeout_ms: Option<u64>,
}

/// Runtime configuration for generic pre-hooks.
#[derive(Debug, Clone, PartialEq)]
pub struct PreHooksConfig {
    pub enable: bool,
    pub required: bool,
    pub steps: Vec<PreHookStep>,
}

#[derive(Debug, Clone, PartialEq)]
pub struct PreHookStep {
    pub cmd: Vec<String>,
    pub required: bool,
    pub cwd: Option<PathBuf>,
    pub env: HashMap<String, String>,
    pub timeout_ms: Option<u64>,
}

impl From<PreHooksToml> for PreHooksConfig {
    fn from(t: PreHooksToml) -> Self {
        let global_required = t.required.unwrap_or(false);
        let steps = t
            .steps
            .into_iter()
            .map(|s| PreHookStep {
                required: s.required.unwrap_or(global_required),
                cmd: s.cmd,
                cwd: s.cwd,
                env: s.env,
                timeout_ms: s.timeout_ms,
            })
            .collect();
        Self {
            enable: t.enable.unwrap_or(false),
            required: global_required,
            steps,
        }
    }
}

/// Optional overrides for user configuration (e.g., from CLI flags).
#[derive(Default, Debug, Clone)]
pub struct ConfigOverrides {
    pub model: Option<String>,
    pub review_model: Option<String>,
    pub cwd: Option<PathBuf>,
    pub approval_policy: Option<AskForApproval>,
    pub sandbox_mode: Option<SandboxMode>,
    pub model_provider: Option<String>,
    pub config_profile: Option<String>,
    pub codex_linux_sandbox_exe: Option<PathBuf>,
    pub base_instructions: Option<String>,
    pub include_plan_tool: Option<bool>,

codex-rs/core/src/config.rs — Runtime: PreHooksConfig/PreHookStep + From impl

pub struct PreHooksConfig {
    pub enable: bool,
    pub required: bool,
    pub steps: Vec<PreHookStep>,
}

#[derive(Debug, Clone, PartialEq)]
pub struct PreHookStep {
    pub cmd: Vec<String>,
    pub required: bool,
    pub cwd: Option<PathBuf>,
    pub env: HashMap<String, String>,
    pub timeout_ms: Option<u64>,
}

impl From<PreHooksToml> for PreHooksConfig {
    fn from(t: PreHooksToml) -> Self {
        let global_required = t.required.unwrap_or(false);
        let steps = t
            .steps
            .into_iter()
            .map(|s| PreHookStep {
                required: s.required.unwrap_or(global_required),
                cmd: s.cmd,
                cwd: s.cwd,
                env: s.env,
                timeout_ms: s.timeout_ms,
            })
            .collect();
        Self {
            enable: t.enable.unwrap_or(false),
            required: global_required,
            steps,
        }
    }
}

/// Optional overrides for user configuration (e.g., from CLI flags).
#[derive(Default, Debug, Clone)]
pub struct ConfigOverrides {
    pub model: Option<String>,
    pub review_model: Option<String>,
    pub cwd: Option<PathBuf>,
    pub approval_policy: Option<AskForApproval>,
    pub sandbox_mode: Option<SandboxMode>,
    pub model_provider: Option<String>,
    pub config_profile: Option<String>,
    pub codex_linux_sandbox_exe: Option<PathBuf>,
    pub base_instructions: Option<String>,
    pub include_plan_tool: Option<bool>,
    pub include_apply_patch_tool: Option<bool>,
    pub include_view_image_tool: Option<bool>,
    pub show_raw_agent_reasoning: Option<bool>,
    pub tools_web_search_request: Option<bool>,
}

impl Config {
    /// Meant to be used exclusively for tests: `load_with_overrides()` should
    /// be used in all other cases.
    pub fn load_from_base_config_with_overrides(
        cfg: ConfigToml,
        overrides: ConfigOverrides,
        codex_home: PathBuf,
    ) -> std::io::Result<Self> {
        let user_instructions = Self::load_instructions(Some(&codex_home));

        // Destructure ConfigOverrides fully to ensure all overrides are applied.
        let ConfigOverrides {
            model,
            review_model: override_review_model,
            cwd,
            approval_policy,
            sandbox_mode,
            model_provider,
            config_profile: config_profile_key,
            codex_linux_sandbox_exe,
            base_instructions,
            include_plan_tool,
            include_apply_patch_tool,
            include_view_image_tool,
            show_raw_agent_reasoning,

impl From<PreHooksToml> for PreHooksConfig {
    fn from(t: PreHooksToml) -> Self {
        let global_required = t.required.unwrap_or(false);
        let steps = t
            .steps
            .into_iter()
            .map(|s| PreHookStep {
                required: s.required.unwrap_or(global_required),
                cmd: s.cmd,
                cwd: s.cwd,
                env: s.env,
                timeout_ms: s.timeout_ms,
            })
            .collect();
        Self {
            enable: t.enable.unwrap_or(false),
            required: global_required,
            steps,
        }
    }
}

/// Optional overrides for user configuration (e.g., from CLI flags).
#[derive(Default, Debug, Clone)]
pub struct ConfigOverrides {
    pub model: Option<String>,
    pub review_model: Option<String>,
    pub cwd: Option<PathBuf>,
    pub approval_policy: Option<AskForApproval>,
    pub sandbox_mode: Option<SandboxMode>,
    pub model_provider: Option<String>,
    pub config_profile: Option<String>,
    pub codex_linux_sandbox_exe: Option<PathBuf>,
    pub base_instructions: Option<String>,
    pub include_plan_tool: Option<bool>,
    pub include_apply_patch_tool: Option<bool>,
    pub include_view_image_tool: Option<bool>,
    pub show_raw_agent_reasoning: Option<bool>,
    pub tools_web_search_request: Option<bool>,
}

impl Config {
    /// Meant to be used exclusively for tests: `load_with_overrides()` should
    /// be used in all other cases.
    pub fn load_from_base_config_with_overrides(
        cfg: ConfigToml,
        overrides: ConfigOverrides,
        codex_home: PathBuf,
    ) -> std::io::Result<Self> {
        let user_instructions = Self::load_instructions(Some(&codex_home));

        // Destructure ConfigOverrides fully to ensure all overrides are applied.
        let ConfigOverrides {
            model,
            review_model: override_review_model,
            cwd,
            approval_policy,
            sandbox_mode,
            model_provider,
            config_profile: config_profile_key,
            codex_linux_sandbox_exe,
            base_instructions,
            include_plan_tool,
            include_apply_patch_tool,
            include_view_image_tool,
            show_raw_agent_reasoning,
            tools_web_search_request: override_tools_web_search_request,
        } = overrides;

        let active_profile_name = config_profile_key
            .as_ref()
            .or(cfg.profile.as_ref())
            .cloned();
        let config_profile = match active_profile_name.as_ref() {
            Some(key) => cfg
                .profiles
                .get(key)
                .ok_or_else(|| {
                    std::io::Error::new(
                        std::io::ErrorKind::NotFound,
                        format!("config profile `{key}` not found"),

codex-rs/core/src/config.rs — Config fields (Config/ConfigToml)

    /// Optional memory-first pre-turn hook configuration.
    pub memory_first: Option<MemoryFirstConfig>,

    /// Optional generic pre-hooks to run before sending the initial request.
    pub pre_hooks: Option<PreHooksConfig>,
}

impl Config {
    /// Load configuration with *generic* CLI overrides (`-c key=value`) applied
    /// **in between** the values parsed from `config.toml` and the
    /// strongly-typed overrides specified via [`ConfigOverrides`].
    ///
    /// The precedence order is therefore: `config.toml` < `-c` overrides <
    /// `ConfigOverrides`.
    pub fn load_with_cli_overrides(

    #[serde(default)]
    pub memory_first: Option<MemoryFirstToml>,

    /// Optional generic pre-hooks that run prior to submitting the prompt.
    #[serde(default)]
    pub pre_hooks: Option<PreHooksToml>,
}

impl From<ConfigToml> for UserSavedConfig {
    fn from(config_toml: ConfigToml) -> Self {
        let profiles = config_toml
            .profiles
            .into_iter()
            .map(|(k, v)| (k, v.into()))
            .collect();

local/Makefile — Demo targets

prehooks-hello-demo: build
	cd $(CODEX_RS_DIR) \
		&& CH=$$(mktemp -d) \
		&& printf "Writing dev config with pre_hooks to %s\n" "$$CH/config.toml" \
		&& printf "%s\n" \
		   "[pre_hooks]" \
		   "enable = true" \
		   "required = true" \
		   "" \
		   "[[pre_hooks.steps]]" \
		   "cmd = [\"/bin/sh\", \"-lc\", \"echo HELLO PREHOOK\"]" \
		   > "$$CH/config.toml" \
		&& OPENAI_API_KEY=dummy \
		   CODEX_RS_SSE_FIXTURE=$$(pwd)/exec/tests/fixtures/cli_responses_fixture.sse \
		   OPENAI_BASE_URL=http://unused.local \
		   CODEX_HOME=$$CH \
		   target/debug/codex-exec --skip-git-repo-check --color always -C $$PWD "Trigger prehooks" | tee $$CH/out.txt \
		&& printf "\n=== Pre-hooks output (grep) ===\n" \
		&& grep -E "HELLO PREHOOK|\[pre-hooks\]" -n $$CH/out.txt || (echo "Expected HELLO PREHOOK not found" && exit 1)

# Wrapper-based demo: seed two sessions via codex-dev (isolated HOME/CODEX_HOME) and resume last.
codex-dev-seed-and-resume:
	./local/bin/codex-dev exec --skip-git-repo-check -C "$(CODEX_RS_DIR)" "echo MARK1" >/dev/null
	./local/bin/codex-dev exec --skip-git-repo-check -C "/tmp" "echo MARK2" >/dev/null
	./local/bin/codex-dev exec --skip-git-repo-check -C "$(CODEX_RS_DIR)" "echo MARK2" resume --last | sed -n '1,60p'

# Wrapper-based prehooks demo (uses the dev config written by codex-dev).
codex-dev-prehooks:
	./local/bin/codex-dev exec --skip-git-repo-check -C "$(CODEX_RS_DIR)" \
	  -c 'pre_hooks.enable=true' -c 'pre_hooks.required=true' \

codex-dev-seed-and-resume:
	./local/bin/codex-dev exec --skip-git-repo-check -C "$(CODEX_RS_DIR)" "echo MARK1" >/dev/null
	./local/bin/codex-dev exec --skip-git-repo-check -C "/tmp" "echo MARK2" >/dev/null
	./local/bin/codex-dev exec --skip-git-repo-check -C "$(CODEX_RS_DIR)" "echo MARK2" resume --last | sed -n '1,60p'

# Wrapper-based prehooks demo (uses the dev config written by codex-dev).
codex-dev-prehooks:
	./local/bin/codex-dev exec --skip-git-repo-check -C "$(CODEX_RS_DIR)" \
	  -c 'pre_hooks.enable=true' -c 'pre_hooks.required=true' \
	  -c 'pre_hooks.steps=[{cmd=["echo","DEV WRAPPER HOOK"]}]' \
	  "Wrapper run" | tee /tmp/codex-dev-prehooks.txt
	@grep -E "DEV PREHOOK|DEV WRAPPER HOOK" -n /tmp/codex-dev-prehooks.txt >/dev/null || (echo "Expected prehook output not found" && exit 1)

# Fail-closed demo: first step fails and required=true, so the process should exit non-zero.
prehooks-fail-closed-demo: build
	cd $(CODEX_RS_DIR) \
		&& CH=$$(mktemp -d) \
		&& printf "%s\n" \
		   "[pre_hooks]" \
		   "enable = true" \
		   "required = true" \
		   "" \
		   "[[pre_hooks.steps]]" \
		   "cmd = [\"/bin/sh\", \"-lc\", \"false\"]" \
		   > "$$CH/config.toml" \
		&& set +e; OPENAI_API_KEY=dummy \
		   CODEX_RS_SSE_FIXTURE=$$(pwd)/exec/tests/fixtures/cli_responses_fixture.sse \
		   OPENAI_BASE_URL=http://unused.local \
		   CODEX_HOME=$$CH \
		   target/debug/codex-exec --skip-git-repo-check --color always -C $$PWD "Fail-closed" >/dev/null; \

codex-dev-prehooks:
	./local/bin/codex-dev exec --skip-git-repo-check -C "$(CODEX_RS_DIR)" \
	  -c 'pre_hooks.enable=true' -c 'pre_hooks.required=true' \
	  -c 'pre_hooks.steps=[{cmd=["echo","DEV WRAPPER HOOK"]}]' \
	  "Wrapper run" | tee /tmp/codex-dev-prehooks.txt
	@grep -E "DEV PREHOOK|DEV WRAPPER HOOK" -n /tmp/codex-dev-prehooks.txt >/dev/null || (echo "Expected prehook output not found" && exit 1)

# Fail-closed demo: first step fails and required=true, so the process should exit non-zero.
prehooks-fail-closed-demo: build
	cd $(CODEX_RS_DIR) \
		&& CH=$$(mktemp -d) \
		&& printf "%s\n" \
		   "[pre_hooks]" \
		   "enable = true" \
		   "required = true" \
		   "" \
		   "[[pre_hooks.steps]]" \
		   "cmd = [\"/bin/sh\", \"-lc\", \"false\"]" \
		   > "$$CH/config.toml" \
		&& set +e; OPENAI_API_KEY=dummy \
		   CODEX_RS_SSE_FIXTURE=$$(pwd)/exec/tests/fixtures/cli_responses_fixture.sse \
		   OPENAI_BASE_URL=http://unused.local \
		   CODEX_HOME=$$CH \
		   target/debug/codex-exec --skip-git-repo-check --color always -C $$PWD "Fail-closed" >/dev/null; \
		   status=$$?; set -e; \
		   if [ "$$status" -eq 0 ]; then echo "Expected fail-closed non-zero exit" && exit 1; else echo "Fail-closed OK (non-zero as expected)"; fi

# Non-fatal demo: first step fails but required=false, the run continues and prints AFTER FAIL NONFATAL.
prehooks-nonfatal-demo: build
	cd $(CODEX_RS_DIR) \

prehooks-fail-closed-demo: build
	cd $(CODEX_RS_DIR) \
		&& CH=$$(mktemp -d) \
		&& printf "%s\n" \
		   "[pre_hooks]" \
		   "enable = true" \
		   "required = true" \
		   "" \
		   "[[pre_hooks.steps]]" \
		   "cmd = [\"/bin/sh\", \"-lc\", \"false\"]" \
		   > "$$CH/config.toml" \
		&& set +e; OPENAI_API_KEY=dummy \
		   CODEX_RS_SSE_FIXTURE=$$(pwd)/exec/tests/fixtures/cli_responses_fixture.sse \
		   OPENAI_BASE_URL=http://unused.local \
		   CODEX_HOME=$$CH \
		   target/debug/codex-exec --skip-git-repo-check --color always -C $$PWD "Fail-closed" >/dev/null; \
		   status=$$?; set -e; \
		   if [ "$$status" -eq 0 ]; then echo "Expected fail-closed non-zero exit" && exit 1; else echo "Fail-closed OK (non-zero as expected)"; fi

# Non-fatal demo: first step fails but required=false, the run continues and prints AFTER FAIL NONFATAL.
prehooks-nonfatal-demo: build
	cd $(CODEX_RS_DIR) \
		&& CH=$$(mktemp -d) OUT=$$(mktemp) \
		&& printf "%s\n" \
		   "[pre_hooks]" \
		   "enable = true" \
		   "required = false" \
		   "" \
		   "[[pre_hooks.steps]]" \
		   "cmd = [\"/bin/sh\", \"-lc\", \"false\"]" \

prehooks-nonfatal-demo: build
	cd $(CODEX_RS_DIR) \
		&& CH=$$(mktemp -d) OUT=$$(mktemp) \
		&& printf "%s\n" \
		   "[pre_hooks]" \
		   "enable = true" \
		   "required = false" \
		   "" \
		   "[[pre_hooks.steps]]" \
		   "cmd = [\"/bin/sh\", \"-lc\", \"false\"]" \
		   "" \
		   "[[pre_hooks.steps]]" \
		   "cmd = [\"/bin/sh\", \"-lc\", \"echo AFTER FAIL NONFATAL\"]" \
		   > "$$CH/config.toml" \
		&& OPENAI_API_KEY=dummy \
		   CODEX_RS_SSE_FIXTURE=$$(pwd)/exec/tests/fixtures/cli_responses_fixture.sse \
		   OPENAI_BASE_URL=http://unused.local \
		   CODEX_HOME=$$CH \
		   target/debug/codex-exec --skip-git-repo-check --color always -C $$PWD "Nonfatal run" 2>&1 | tee $$OUT \
		&& grep -E "AFTER FAIL NONFATAL" -n $$OUT || (echo "Expected AFTER FAIL NONFATAL not found" && exit 1)

local/bin/codex-dev — Wrapper

#!/usr/bin/env bash
set -euo pipefail

# Resolve repo root
REPO_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"

# Isolated dev state
export CODEX_HOME="${CODEX_HOME:-$REPO_DIR/.codex-dev/CODEX_HOME}"
export HOME="${HOME_OVERRIDE:-$REPO_DIR/.codex-dev/HOME}"
mkdir -p "$CODEX_HOME" "$HOME/.codex"

# Minimal dev config (written once)
if [[ ! -f "$HOME/.codex/config.toml" ]]; then
  cat >"$HOME/.codex/config.toml" <<'EOF'
[pre_hooks]
enable = true
required = true

[[pre_hooks.steps]]
cmd = ["/bin/sh", "-lc", "echo DEV PREHOOK"]
EOF
  cp -f "$HOME/.codex/config.toml" "$CODEX_HOME/config.toml"
fi

# Dummy env + SSE fixture; no real network used
export OPENAI_API_KEY="${OPENAI_API_KEY:-sk-test-1234}"
export OPENAI_BASE_URL="${OPENAI_BASE_URL:-http://unused.local}"
export CODEX_RS_SSE_FIXTURE="${CODEX_RS_SSE_FIXTURE:-$REPO_DIR/codex-rs/exec/tests/fixtures/cli_responses_fixture.sse}"

# Build local binary if missing
if [[ ! -x "$REPO_DIR/codex-rs/target/debug/codex" ]]; then
  ( cd "$REPO_DIR/codex-rs" && cargo build --workspace )
fi

exec "$REPO_DIR/codex-rs/target/debug/codex" "$@"

local/docs/CODEX_TEST_SETUP.md — Generic Pre‑Hooks section

## Generic Pre‑Hooks (local runs)


You can run arbitrary pre‑steps (lint, schema checks, codegen, cache warmups) before the agent request. Enable and configure via either config or flags:
You can run arbitrary pre‑steps (lint, schema checks, codegen, cache warmups) before the agent request. Enable and configure via either config or flags:


- Config (`~/.codex/config.toml` in the dev sandbox set by the wrapper):
- Config (`~/.codex/config.toml` in the dev sandbox set by the wrapper):


```toml
```toml
[pre_hooks]
[pre_hooks]
enable = true
enable = true
required = true  # fail closed if any step fails
required = true  # fail closed if any step fails


[[pre_hooks.steps]]
[[pre_hooks.steps]]
cmd = ["cargo", "fmt", "--", "--check"]
cmd = ["cargo", "fmt", "--", "--check"]


[[pre_hooks.steps]]
[[pre_hooks.steps]]
cmd = ["cargo", "clippy", "-q"]
cmd = ["cargo", "clippy", "-q"]
timeout_ms = 60000
timeout_ms = 60000



- CLI flags (can be repeated):
- CLI flags (can be repeated):


```bash
```bash
./local/bin/codex-dev exec \
./local/bin/codex-dev exec \
  --skip-git-repo-check \
  --skip-git-repo-check \
  --pre-hooks-enable \
  --pre-hooks-enable \
  --pre-hooks-required \
  --pre-hooks-required \
  --pre-hook "cargo fmt -- --check" \
  --pre-hook "cargo fmt -- --check" \
  --pre-hook "cargo clippy -q" \
  --pre-hook "cargo clippy -q" \
  "Run the task after pre‑hooks"
  "Run the task after pre‑hooks"



Notes:
Notes:
- `--pre-hooks-required` treats any failing step as fatal; omit it to continue on failure.
- `--pre-hooks-required` treats any failing step as fatal; omit it to continue on failure.
- Each `--pre-hook` string is shell‑split; if you need quoting, wrap the whole step in quotes.
- Each `--pre-hook` string is shell‑split; if you need quoting, wrap the whole step in quotes.

local/PR3_BODY.md — PR summary

# Title
Generic Pre‑Hooks Framework (opt‑in): run user steps before the LLM

---

## Summary
Problems
- No generic, first‑class mechanism to run pre‑steps (lint, schema checks, codegen, cache warmups) before talking to the LLM.
- Memory‑first exists as a separate opt‑in path; it shouldn’t bias a generic framework.

Fixes
- Add an optional, fail‑closed‑capable generic pre‑hooks pipeline that runs before the first LLM call.
- Users declare ordered steps as commands; per‑step `cwd`, `env`, and `timeout_ms` supported.
- Provide CLI flags and `-c` overrides to enable pre‑hooks ad‑hoc without editing config.

Scope
- Core config additions + Exec runner/wiring only. No protocol/JSON surface changes.
- Pre‑hooks are disabled by default; fully opt‑in. `memory_first` remains an independent opt‑in feature.

Risk
- Medium: executes user commands. Mitigated by existing sandbox policy, per‑step timeouts, and optional fail‑closed mode.

---

## Goals
- Allow running arbitrary pre‑steps before the LLM.
- Support required vs. non‑fatal steps, timeouts, per‑step `cwd`/`env`.
- Keep memory‑first as a separate opt‑in (unchanged here).

---

## User‑Visible Behavior
- When enabled, pre‑hooks run before the first model request and are logged, e.g.: `[pre-hooks] (1/1) cargo clippy -q`.
- If a step is `required=true` and fails or times out → fail‑closed (no LLM call).
- If non‑required step fails → log and continue.

---

## Configuration

TOML (dev sandbox):
```toml
[pre_hooks]
enable = true
required = true          # fail‑closed

[[pre_hooks.steps]]
cmd = ["/bin/sh", "-lc", "echo HELLO PREHOOK"]
timeout_ms = 1000        # optional per‑step

CLI flags (ad‑hoc):

codex exec --skip-git-repo-check \
  --pre-hooks-enable --pre-hooks-required \
  --pre-hook "cargo fmt -- --check" \
  --pre-hook "cargo clippy -q" \
  "Run after hooks"

-c overrides (explicit):

codex-exec --skip-git-repo-check \
  -c 'pre_hooks.enable=true' -c 'pre_hooks.required=true' \
  -c 'pre_hooks.steps=[{cmd=["echo","HELLO"]}]' \
  "Hello"

Implementation Notes

Core

Config gains pre_hooks: Option<PreHooksConfig> with TOML types (PreHooksToml, PreHookStepToml).

Exec

New exec/src/pre_hooks.rs: runner that executes steps with per‑step cwd/env/timeout_ms; respects sandbox.
exec/src/lib.rs: invokes runner before sending any input to the agent; wires CLI flags and translates them to -c overrides.
exec/src/cli.rs: adds --pre-hooks-enable, --pre-hooks-required, and repeatable --pre-hook.

Safety

Respects the turn’s sandbox policy and approval mode; no root escalation.
Tight timeouts; no network beyond policy.
All injections are text‑only; no tool grants.

Compatibility

Protocol/JSON output: unchanged.
Pre‑hooks are a local pre‑processing stage and do not alter the event schema.

Testing Plan

Targets (Makefile)

prehooks-hello-demo: hello‑world pre‑hook (uses SSE fixture).
codex-dev-seed-and-resume: wrapper sandbox; seeds and resumes latest.
codex-dev-prehooks: wrapper + -c pre‑hook override demo.

Crate tests

cargo test -p codex-exec ✅
cargo test -p codex-core — mostly passing; a few unrelated env‑dependent failures (default_client/git_info).

Performance, Security, Privacy

Performance: bounded by per‑step timeouts.
Security: commands run under existing sandbox/approval policy; no escalation.
Privacy: operates locally; no additional telemetry.

Rollout / Backout

Disabled by default; opt‑in via [pre_hooks].
Backout: set enable = false or omit pre_hooks.

Checklist

Optional generic pre‑hooks (before LLM)
User‑defined steps (command vector)
Fail‑closed option (required = true)
CLI flags + -c overrides
Docs + Makefile demos + wrapper
Exec unit/integration tests pass; core mostly passes

Review Bundle (single file)

Public Gist: https://gist.github.com/grahama1970/6c3f68fa6d6e66319fee2c8031fb4c7f


## Questions for Reviewers
- Is the CLI → TOML override translation for `--pre-hook` robust across shells?
- Are timeout and required/non‑fatal semantics clear and sufficient?
- Any concerns with env/cwd propagation or sandbox behavior?
- Do the demos cover the expected usage?

grahama1970/review_curated.md

Select an option

No results found

Select an option

No results found

Code Review Bundle (Curated, Single File)

Review Focus

Files To Review (with context)

codex-rs/exec/src/pre_hooks.rs

codex-rs/exec/src/lib.rs — CLI override translation for pre‑hooks

codex-rs/exec/src/lib.rs — Invocation before agent submission

codex-rs/exec/src/cli.rs — Added flags

codex-rs/core/src/config.rs — Types: PreHooksToml/PreHookStepToml

codex-rs/core/src/config.rs — Runtime: PreHooksConfig/PreHookStep + From impl

codex-rs/core/src/config.rs — Config fields (Config/ConfigToml)

local/Makefile — Demo targets

local/bin/codex-dev — Wrapper

local/docs/CODEX_TEST_SETUP.md — Generic Pre‑Hooks section

local/PR3_BODY.md — PR summary

Implementation Notes

Compatibility

Testing Plan

Performance, Security, Privacy

Rollout / Backout

Checklist

Review Bundle (single file)