Commit Graph

115 Commits

  • fix: implement 'Allow this session' for apply_patch approvals (#8451)
    **Summary**
    This PR makes “ApprovalDecision::AcceptForSession / don’t ask again this
    session” actually work for `apply_patch` approvals by caching approvals
    based on absolute file paths in codex-core, properly wiring it through
    app-server v2, and exposing the choice in both TUI and TUI2.
    - This brings `apply_patch` calls to be at feature-parity with general
    shell commands, which also have a "Yes, and don't ask again" option.
    - This also fixes VSCE's "Allow this session" button to actually work.
    
    While we're at it, also split the app-server v2 protocol's
    `ApprovalDecision` enum so execpolicy amendments are only available for
    command execution approvals.
    
    **Key changes**
    - Core: per-session patch approval allowlist keyed by absolute file
    paths
    - Handles multi-file patches and renames/moves by recording both source
    and destination paths for `Update { move_path: Some(...) }`.
    - Extend the `Approvable` trait and `ApplyPatchRuntime` to work with
    multiple keys, because an `apply_patch` tool call can modify multiple
    files. For a request to be auto-approved, we will need to check that all
    file paths have been approved previously.
    - App-server v2: honor AcceptForSession for file changes
    - File-change approval responses now map AcceptForSession to
    ReviewDecision::ApprovedForSession (no longer downgraded to plain
    Approved).
    - Replace `ApprovalDecision` with two enums:
    `CommandExecutionApprovalDecision` and `FileChangeApprovalDecision`
    - TUI / TUI2: expose “don’t ask again for these files this session”
    - Patch approval overlays now include a third option (“Yes, and don’t
    ask again for these files this session (s)”).
        - Snapshot updates for the approval modal.
    
    **Tests added/updated**
    - Core:
    - Integration test that proves ApprovedForSession on a patch skips the
    next patch prompt for the same file
    - App-server:
    - v2 integration test verifying
    FileChangeApprovalDecision::AcceptForSession works properly
    
    **User-visible behavior**
    - When the user approves a patch “for session”, future patches touching
    only those previously approved file(s) will no longer prompt gain during
    that session (both via app-server v2 and TUI/TUI2).
    
    **Manual testing**
    Tested both TUI and TUI2 - see screenshots below.
    
    TUI:
    <img width="1082" height="355" alt="image"
    src="https://github.com/user-attachments/assets/adcf45ad-d428-498d-92fc-1a0a420878d9"
    />
    
    
    TUI2:
    <img width="1089" height="438" alt="image"
    src="https://github.com/user-attachments/assets/dd768b1a-2f5f-4bd6-98fd-e52c1d3abd9e"
    />
  • Merge Modelfamily into modelinfo (#8763)
    - Merge ModelFamily into ModelInfo
    - Remove logic for adding instructions to apply patch
    - Add compaction limit and visible context window to `ModelInfo`
  • chore: unify conversation with thread name (#8830)
    Done and verified by Codex + refactor feature of RustRover
  • add web_search_cached flag (#8795)
    Add `web_search_cached` feature to config. Enables `web_search` tool
    with access only to cached/indexed results (see
    [docs](https://platform.openai.com/docs/guides/tools-web-search#live-internet-access)).
    
    This takes precedence over the existing `web_search_request`, which
    continues to enable `web_search` over live results as it did before.
    
    `web_search_cached` is disabled for review mode, as `web_search_request`
    is.
  • Add ExecPolicyManager (#8349)
    Move exec policy management into services to keep turn context
    immutable.
  • feat(windows) start powershell in utf-8 mode (#7902)
    ## Summary
    Adds a FeatureFlag to enforce UTF8 encoding in powershell, particularly
    Windows Powershell v5. This should help address issues like #7290.
    
    Notably, this PR does not include the ability to parse `apply_patch`
    invocations within UTF8 shell commands (calls to the freeform tool
    should not be impacted). I am leaving this out of scope for now. We
    should address before this feature becomes Stable, but those cases are
    not the default behavior at this time so we're okay for experimentation
    phase. We should continue cleaning up the `apply_patch::invocation`
    logic and then can handle it more cleanly.
    
    ## Testing
    - [x] Adds additional testing
  • Rename OpenAI models to models manager (#8346)
    # External (non-OpenAI) Pull Request Requirements
    
    Before opening this Pull Request, please read the dedicated
    "Contributing" markdown file or your PR may be closed:
    https://github.com/openai/codex/blob/main/docs/contributing.md
    
    If your PR conforms to our contribution guidelines, replace this text
    with a detailed and high quality description of your changes.
    
    Include a link to a bug report or enhancement request.
  • feat: introduce ExternalSandbox policy (#8290)
    ## Description
    
    Introduced `ExternalSandbox` policy to cover use case when sandbox
    defined by outside environment, effectively it translates to
    `SandboxMode#DangerFullAccess` for file system (since sandbox configured
    on container level) and configurable `network_access` (either Restricted
    or Enabled by outside environment).
    
    as example you can configure `ExternalSandbox` policy as part of
    `sendUserTurn` v1 app_server API:
    
    ```
     {
                "conversationId": <id>,
                "cwd": <cwd>,
                "approvalPolicy": "never",
                "sandboxPolicy": {
                      "type": ""external-sandbox",
                      "network_access": "enabled"/"restricted"
                },
                "model": <model>,
                "effort": <effort>,
                ....
            }
    ```
  • chore: migrate from Config::load_from_base_config_with_overrides to ConfigBuilder (#8276)
    https://github.com/openai/codex/pull/8235 introduced `ConfigBuilder` and
    this PR updates all call non-test call sites to use it instead of
    `Config::load_from_base_config_with_overrides()`.
    
    This is important because `load_from_base_config_with_overrides()` uses
    an empty `ConfigRequirements`, which is a reasonable default for testing
    so the tests are not influenced by the settings on the host. This method
    is now guarded by `#[cfg(test)]` so it cannot be used by business logic.
    
    Because `ConfigBuilder::build()` is `async`, many of the test methods
    had to be migrated to be `async`, as well. On the bright side, this made
    it possible to eliminate a bunch of `block_on_future()` stuff.
  • nit: trace span for regular task (#8053)
    Logs are too spammy
    
    ---------
    
    Co-authored-by: Anton Panasenko <apanasenko@openai.com>
  • chore(shell_command) fix freeform timeout output (#7791)
    ## Summary
    Adding an additional integration test for timeout_ms
    
    ## Testing
    - [x] these are tests
  • fix: use PowerShell to parse PowerShell (#7607)
    Previous to this PR, we used a hand-rolled PowerShell parser in
    `windows_safe_commands.rs` to take a `&str` of PowerShell script see if
    it is equivalent to a list of `execvp(3)` invocations, and if so, we
    then test each using `is_safe_powershell_command()` to determine if the
    overall command is safe:
    
    
    https://github.com/openai/codex/blob/6e6338aa876bb4258abe25b02ac6417b8ea9dff0/codex-rs/core/src/command_safety/windows_safe_commands.rs#L89-L98
    
    Unfortunately, our PowerShell parser did not recognize `@(...)` as a
    special construct, so it was treated as an ordinary token. This meant
    that the following would erroneously be considered "safe:"
    
    ```powershell
    ls @(calc.exe)
    ```
    
    The fix introduced in this PR is to do something comparable what we do
    for Bash/Zsh, which is to use a "proper" parser to derive the list of
    `execvp(3)` calls. For Bash/Zsh, we rely on
    https://crates.io/crates/tree-sitter-bash, but there does not appear to
    be a crate of comparable quality for parsing PowerShell statically
    (https://github.com/airbus-cert/tree-sitter-powershell/ is the best
    thing I found).
    
    Instead, in this PR, we use a PowerShell script to parse the input
    PowerShell program to produce the AST.
  • Chore: limit find family visability (#7891)
    a little bit more code quality of life
  • Removed experimental "command risk assessment" feature (#7799)
    This experimental feature received lukewarm reception during internal
    testing. Removing from the code base.
  • refactoring with_escalated_permissions to use SandboxPermissions instead (#7750)
    helpful in the future if we want more granularity for requesting
    escalated permissions:
    e.g when running in readonly sandbox, model can request to escalate to a
    sandbox that allows writes
  • proposing execpolicy amendment when prompting due to sandbox denial (#7653)
    Currently, we only show the “don’t ask again for commands that start
    with…” option when a command is immediately flagged as needing approval.
    However, there is another case where we ask for approval: When a command
    is initially auto-approved to run within sandbox, but it fails to run
    inside sandbox, we would like to attempt to retry running outside of
    sandbox. This will require a prompt to the user.
    
    This PR addresses this latter case
  • feat(core) Add login to shell_command tool (#6846)
    ## Summary
    Adds the `login` parameter to the `shell_command` tool - optional,
    defaults to true.
    
    ## Testing
    - [x] Tested locally
  • Call models endpoint in models manager (#7616)
    - Introduce `with_remote_overrides` and update
    `refresh_available_models`
    - Put `auth_manager` instead of `auth_mode` on `models_manager`
    - Remove `ShellType` and `ReasoningLevel` to use already existing
    structs
  • Inline response recording and remove process_items indirection (#7310)
    - Inline response recording during streaming: `run_turn` now records
    items as they arrive instead of building a `ProcessedResponseItem` list
    and post‑processing via `process_items`.
    - Simplify turn handling: `handle_output_item_done` returns the
    follow‑up signal + optional tool future; `needs_follow_up` is set only
    there, and in‑flight tool futures are drained once at the end (errors
    logged, no extra state writes).
    - Flattened stream loop: removed `process_items` indirection and the
    extra output queue
    - - Tests: relaxed `tool_parallelism::tool_results_grouped` to allow any
    completion order while still requiring matching call/output IDs.
  • Refactor execpolicy fallback evaluation (#7544)
    ## Refactor of the `execpolicy` crate
    
    To illustrate why we need this refactor, consider an agent attempting to
    run `apple | rm -rf ./`. Suppose `apple` is allowed by `execpolicy`.
    Before this PR, `execpolicy` would consider `apple` and `pear` and only
    render one rule match: `Allow`. We would skip any heuristics checks on
    `rm -rf ./` and immediately approve `apple | rm -rf ./` to run.
    
    To fix this, we now thread a `fallback` evaluation function into
    `execpolicy` that runs when no `execpolicy` rules match a given command.
    In our example, we would run `fallback` on `rm -rf ./` and prevent
    `apple | rm -rf ./` from being run without approval.
  • whitelist command prefix integration in core and tui (#7033)
    this PR enables TUI to approve commands and add their prefixes to an
    allowlist:
    <img width="708" height="605" alt="Screenshot 2025-11-21 at 4 18 07 PM"
    src="https://github.com/user-attachments/assets/56a19893-4553-4770-a881-becf79eeda32"
    />
    
    note: we only show the option to whitelist the command when 
    1) command is not multi-part (e.g `git add -A && git commit -m 'hello
    world'`)
    2) command is not already matched by an existing rule
  • Migrate model family to models manager (#7565)
    This PR moves `ModelsFamily` to `openai_models`. It also propagates
    `ModelsManager` to session services and use it to drive model family. We
    also make `derive_default_model_family` private because it's a step
    towards what we want: one place that gives model configuration.
    
    This is a second step at having one source of truth for models
    information and config: `ModelsManager`.
    
    Next steps would be to remove `ModelsFamily` from config. That's massive
    because it's being used in 41 occasions mostly pre launching `codex`.
    Also, we need to make `find_family_for_model` private. It's also big
    because it's being used in 21 occasions ~ all tests.
  • fix(unified_exec): use platform default shell when unified_exec shell… (#7486)
    # Unified Exec Shell Selection on Windows
    
    ## Problem
    
    reference issue #7466
    
    The `unified_exec` handler currently deserializes model-provided tool
    calls into the `ExecCommandArgs` struct:
    
    ```rust
    #[derive(Debug, Deserialize)]
    struct ExecCommandArgs {
        cmd: String,
        #[serde(default)]
        workdir: Option<String>,
        #[serde(default = "default_shell")]
        shell: String,
        #[serde(default = "default_login")]
        login: bool,
        #[serde(default = "default_exec_yield_time_ms")]
        yield_time_ms: u64,
        #[serde(default)]
        max_output_tokens: Option<usize>,
        #[serde(default)]
        with_escalated_permissions: Option<bool>,
        #[serde(default)]
        justification: Option<String>,
    }
    ```
    
    The `shell` field uses a hard-coded default:
    
    ```rust
    fn default_shell() -> String {
        "/bin/bash".to_string()
    }
    ```
    
    When the model returns a tool call JSON that only contains `cmd` (which
    is the common case), Serde fills in `shell` with this default value.
    Later, `get_command` uses that value as if it were a model-provided
    shell path:
    
    ```rust
    fn get_command(args: &ExecCommandArgs) -> Vec<String> {
        let shell = get_shell_by_model_provided_path(&PathBuf::from(args.shell.clone()));
        shell.derive_exec_args(&args.cmd, args.login)
    }
    ```
    
    On Unix, this usually resolves to `/bin/bash` and works as expected.
    However, on Windows this behavior is problematic:
    
    - The hard-coded `"/bin/bash"` is not a valid Windows path.
    - `get_shell_by_model_provided_path` treats this as a model-specified
    shell, and tries to resolve it (e.g. via `which::which("bash")`), which
    may or may not exist and may not behave as intended.
    - In practice, this leads to commands being executed under a non-default
    or non-existent shell on Windows (for example, WSL bash), instead of the
    expected Windows PowerShell or `cmd.exe`.
    
    The core of the issue is that **"model did not specify `shell`" is
    currently interpreted as "the model explicitly requested `/bin/bash`"**,
    which is both Unix-specific and wrong on Windows.
    
    ## Proposed Solution
    
    Instead of hard-coding `"/bin/bash"` into `ExecCommandArgs`, we should
    distinguish between:
    
    1. **The model explicitly specifying a shell**, e.g.:
    
       ```json
       {
         "cmd": "echo hello",
         "shell": "pwsh"
       }
       ```
    
    In this case, we *do* want to respect the model’s choice and use
    `get_shell_by_model_provided_path`.
    
    2. **The model omitting the `shell` field entirely**, e.g.:
    
       ```json
       {
         "cmd": "echo hello"
       }
       ```
    
    In this case, we should *not* assume `/bin/bash`. Instead, we should use
    `default_user_shell()` and let the platform decide.
    
    To express this distinction, we can:
    
    1. Change `shell` to be optional in `ExecCommandArgs`:
    
       ```rust
       #[derive(Debug, Deserialize)]
       struct ExecCommandArgs {
           cmd: String,
           #[serde(default)]
           workdir: Option<String>,
           #[serde(default)]
           shell: Option<String>,
           #[serde(default = "default_login")]
           login: bool,
           #[serde(default = "default_exec_yield_time_ms")]
           yield_time_ms: u64,
           #[serde(default)]
           max_output_tokens: Option<usize>,
           #[serde(default)]
           with_escalated_permissions: Option<bool>,
           #[serde(default)]
           justification: Option<String>,
       }
       ```
    
    Here, the absence of `shell` in the JSON is represented as `shell:
    None`, rather than a hard-coded string value.
  • chore: make create_approval_requirement_for_command an async fn (#7501)
    I think this might help with https://github.com/openai/codex/pull/7033
    because `create_approval_requirement_for_command()` will soon need
    access to `Session.state`, which is a `tokio::sync::Mutex` that needs to
    be accessed via `async`.
  • bypass sandbox for policy approved commands (#7110)
    allowing cmds greenlit by execpolicy to bypass sandbox + minor refactor
    for a world where we have execpolicy rules with specific sandbox
    requirements
  • feat: update process_exec_tool_call() to take a cancellation token (#6972)
    This updates `ExecParams` so that instead of taking `timeout_ms:
    Option<u64>`, it now takes a more general cancellation mechanism,
    `ExecExpiration`, which is an enum that includes a
    `Cancellation(tokio_util::sync::CancellationToken)` variant.
    
    If the cancellation token is fired, then `process_exec_tool_call()`
    returns in the same way as if a timeout was exceeded.
    
    This is necessary so that in #6973, we can manage the timeout logic
    external to the `process_exec_tool_call()` because we want to "suspend"
    the timeout when an elicitation from a human user is pending.
    
    
    
    
    
    
    
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/6972).
    * #7005
    * #6973
    * __->__ #6972
  • Always fallback to real shell (#6953)
    Either cmd.exe or `/bin/sh`.
  • [app-server] feat: v2 apply_patch approval flow (#6760)
    This PR adds the API V2 version of the apply_patch approval flow, which
    centers around `ThreadItem::FileChange`.
    
    This PR wires the new RPC (`item/fileChange/requestApproval`, V2 only)
    and related events (`item/started`, `item/completed` for
    `ThreadItem::FileChange`, which are emitted in both V1 and V2) through
    the app-server
    protocol. The new approval RPC is only sent when the user initiates a
    turn with the new `turn/start` API so we don't break backwards
    compatibility with VSCE.
    
    Similar to https://github.com/openai/codex/pull/6758, the approach I
    took was to make as few changes to the Codex core as possible,
    leveraging existing `EventMsg` core events, and translating those in
    app-server. I did have to add a few additional fields to
    `EventMsg::PatchApplyBegin` and `EventMsg::PatchApplyEnd`, but those
    were fairly lightweight.
    
    However, the `EventMsg`s emitted by core are the following:
    ```
    1) Auto-approved (no request for approval)

    - EventMsg::PatchApplyBegin
    - EventMsg::PatchApplyEnd
    
    2) Approved by user
    - EventMsg::ApplyPatchApprovalRequest
    - EventMsg::PatchApplyBegin
    - EventMsg::PatchApplyEnd
    
    3) Declined by user
    - EventMsg::ApplyPatchApprovalRequest
    - EventMsg::PatchApplyBegin
    - EventMsg::PatchApplyEnd
    ```
    
    For a request triggering an approval, this would result in:
    ```
    item/fileChange/requestApproval
    item/started
    item/completed
    ```
    
    which is different from the `ThreadItem::CommandExecution` flow
    introduced in https://github.com/openai/codex/pull/6758, which does the
    below and is preferable:
    ```
    item/started
    item/commandExecution/requestApproval
    item/completed
    ```
    
    To fix this, we leverage `TurnSummaryStore` on codex_message_processor
    to store a little bit of state, allowing us to fire `item/started` and
    `item/fileChange/requestApproval` whenever we receive the underlying
    `EventMsg::ApplyPatchApprovalRequest`, and no-oping when we receive the
    `EventMsg::PatchApplyBegin` later.
    
    This is much less invasive than modifying the order of EventMsg within
    core (I tried).
    
    The resulting payloads:
    ```
    {
      "method": "item/started",
      "params": {
        "item": {
          "changes": [
            {
              "diff": "Hello from Codex!\n",
              "kind": "add",
              "path": "/Users/owen/repos/codex/codex-rs/APPROVAL_DEMO.txt"
            }
          ],
          "id": "call_Nxnwj7B3YXigfV6Mwh03d686",
          "status": "inProgress",
          "type": "fileChange"
        }
      }
    }
    ```
    
    ```
    {
      "id": 0,
      "method": "item/fileChange/requestApproval",
      "params": {
        "grantRoot": null,
        "itemId": "call_Nxnwj7B3YXigfV6Mwh03d686",
        "reason": null,
        "threadId": "019a9e11-8295-7883-a283-779e06502c6f",
        "turnId": "1"
      }
    }
    ```
    
    ```
    {
      "id": 0,
      "result": {
        "decision": "accept"
      }
    }
    ```
    
    ```
    {
      "method": "item/completed",
      "params": {
        "item": {
          "changes": [
            {
              "diff": "Hello from Codex!\n",
              "kind": "add",
              "path": "/Users/owen/repos/codex/codex-rs/APPROVAL_DEMO.txt"
            }
          ],
          "id": "call_Nxnwj7B3YXigfV6Mwh03d686",
          "status": "completed",
          "type": "fileChange"
        }
      }
    }
    ```
  • execpolicy2 core integration (#6641)
    This PR threads execpolicy2 into codex-core.
    
    activated via feature flag: exec_policy (on by default)
    
    reads and parses all .codexpolicy files in `codex_home/codex`
    
    refactored tool runtime API to integrate execpolicy logic
    
    ---------
    
    Co-authored-by: Michael Bolin <mbolin@openai.com>