Commit Graph

2066 Commits

  • fix(shell) fallback shells (#6948)
    ## Summary
    Add fallbacks when user_shell_path does not resolve to a known shell
    type
    
    ## Testing
    - [x] Tests still pass
  • chore: refactor exec-server to prepare it for standalone MCP use (#6944)
    This PR reorganizes things slightly so that:
    
    - Instead of a single multitool executable, `codex-exec-server`, we now
    have two executables:
      - `codex-exec-mcp-server` to launch the MCP server
    - `codex-execve-wrapper` is the `execve(2)` wrapper to use with the
    `BASH_EXEC_WRAPPER` environment variable
    - `BASH_EXEC_WRAPPER` must be a single executable: it cannot be a
    command string composed of an executable with args (i.e., it no longer
    adds the `escalate` subcommand, as before)
    - `codex-exec-mcp-server` takes `--bash` and `--execve` as options.
    Though if `--execve` is not specified, the MCP server will check the
    directory containing `std::env::current_exe()` and attempt to use the
    file named `codex-execve-wrapper` within it. In development, this works
    out since these executables are side-by-side in the `target/debug`
    folder.
    
    With respect to testing, this also fixes an important bug in
    `dummy_exec_policy()`, as I was using `ends_with()` as if it applied to
    a `String`, but in this case, it is used with a `&Path`, so the
    semantics are slightly different.
    
    Putting this all together, I was able to test this by running the
    following:
    
    ```
    ~/code/codex/codex-rs$ npx @modelcontextprotocol/inspector \
        ./target/debug/codex-exec-mcp-server --bash ~/code/bash/bash
    ```
    
    If I try to run `git status` in `/Users/mbolin/code/codex` via the
    `shell` tool from the MCP server:
    
    <img width="1589" height="1335" alt="image"
    src="https://github.com/user-attachments/assets/9db6aea8-7fbc-4675-8b1f-ec446685d6c4"
    />
    
    then I get prompted with the following elicitation, as expected:
    
    <img width="1589" height="1335" alt="image"
    src="https://github.com/user-attachments/assets/21b68fe0-494d-4562-9bad-0ddc55fc846d"
    />
    
    Though a current limitation is that the `shell` tool defaults to a
    timeout of 10s, which means I only have 10s to respond to the
    elicitation. Ideally, the time spent waiting for a response from a human
    should not count against the timeout for the command execution. I will
    address this in a subsequent PR.
    
    ---
    
    Note `~/code/bash/bash` was created by doing:
    
    ```
    cd ~/code
    git clone https://github.com/bminor/bash
    cd bash
    git checkout a8a1c2fac029404d3f42cd39f5a20f24b6e4fe4b
    <apply the patch below>
    ./configure
    make
    ```
    
    The patch:
    
    ```
    diff --git a/execute_cmd.c b/execute_cmd.c
    index 070f5119..d20ad2b9 100644
    --- a/execute_cmd.c
    +++ b/execute_cmd.c
    @@ -6129,6 +6129,19 @@ shell_execve (char *command, char **args, char **env)
       char sample[HASH_BANG_BUFSIZ];
       size_t larray;
    
    +  char* exec_wrapper = getenv("BASH_EXEC_WRAPPER");
    +  if (exec_wrapper && *exec_wrapper && !whitespace (*exec_wrapper))
    +    {
    +      char *orig_command = command;
    +
    +      larray = strvec_len (args);
    +
    +      memmove (args + 2, args, (++larray) * sizeof (char *));
    +      args[0] = exec_wrapper;
    +      args[1] = orig_command;
    +      command = exec_wrapper;
    +    }
    +
    ```
  • Fix/correct reasoning display (#6749)
    This closes #6748 by implementing fallback to
    `model_family.default_reasoning_effort` in `reasoning_effort` display of
    `/status` when no `model_reasoning_effort` is set in the configuration.
    
    ## common/src/config_summary.rs
    
    - `create_config_summary_entries` now fills the "reasoning effort" entry
    with the explicit `config.model_reasoning_effort` when present and falls
    back to `config.model_family.default_reasoning_effort` when it is
    `None`, instead of emitting the literal string `none`.
    - This ensures downstream consumers such as `tui/src/status/helpers.rs`
    continue to work unchanged while automatically picking up model-family
    defaults when the user has not selected a reasoning effort.
    
    ## tui/src/status/helpers.rs / core/src/model_family.rs
    
    `ModelFamily::default_reasoning_effort` metadata is set to `medium` for
    both `gpt-5*-codex` and `gpt-5` models following the default behaviour
    of the API and recommendation of the codebase:
    - per https://platform.openai.com/docs/api-reference/responses/create
    `gpt-5` defaults to `medium` reasoning when no preset is passed
    - there is no mention of the preset for `gpt-5.1-codex` in the API docs
    but `medium` is the default setting for `gpt-5.1-codex` as per
    `codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__model_reasoning_selection_popup.snap`
    
    ---------
    
    Signed-off-by: lionelchg <lionel.cheng@hotmail.fr>
    Co-authored-by: Eric Traut <etraut@openai.com>
  • fix(context left after review): review footer context after /review (#5610)
    ## Summary
    - show live review token usage while `/review` runs and restore the main
    session indicator afterward
      - add regression coverage for the footer behavior
    
    ## Testing
      - just fmt
      - cargo test -p codex-tui
    
    Fixes #5604
    
    ---------
    
    Signed-off-by: Fahad <fahad@2doapp.com>
  • stop over-reporting world-writable directories (#6936)
    Fix world-writable audit false positives by expanding generic
    permissions with MapGenericMask and then checking only concrete write
    bits. The earlier check looked for FILE_GENERIC_WRITE/generic masks
    directly, which shares bits with read permissions and could flag an
    Everyone read ACE as writable.
  • fix: prepare ExecPolicy in exec-server for execpolicy2 cutover (#6888)
    This PR introduces an extra layer of abstraction to prepare us for the
    migration to execpolicy2:
    
    - introduces a new trait, `EscalationPolicy`, whose `determine_action()`
    method is responsible for producing the `EscalateAction`
    - the existing `ExecPolicy` typedef is changed to return an intermediate
    `ExecPolicyOutcome` instead of `EscalateAction`
    - the default implementation of `EscalationPolicy`,
    `McpEscalationPolicy`, composes `ExecPolicy`
    - the `ExecPolicyOutcome` includes `codex_execpolicy2::Decision`, which
    has a `Prompt` variant
    - when `McpEscalationPolicy` gets `Decision::Prompt` back from
    `ExecPolicy`, it prompts the user via an MCP elicitation and maps the
    result into an `ElicitationAction`
    - now that the end user can reply to an elicitation with `Decline` or
    `Cancel`, we introduce a new variant, `EscalateAction::Deny`, which the
    client handles by returning exit code `1` without running anything
    
    Note the way the elicitation is created is still not quite right, but I
    will fix that once we have things running end-to-end for real in a
    follow-up PR.
  • [core] add optional status_code to error events (#6865)
    We want to better uncover error status code for clients. Add an optional
    status_code to error events (thread error, error, stream error) so app
    server could uncover the status code from the client side later.
    
    in event log:
    ```
    < {
    <   "method": "codex/event/stream_error",
    <   "params": {
    <     "conversationId": "019a9a32-f576-7292-9711-8e57e8063536",
    <     "id": "0",
    <     "msg": {
    <       "message": "Reconnecting... 5/5",
    <       "status_code": 401,
    <       "type": "stream_error"
    <     }
    <   }
    < }
    < {
    <   "method": "codex/event/error",
    <   "params": {
    <     "conversationId": "019a9a32-f576-7292-9711-8e57e8063536",
    <     "id": "0",
    <     "msg": {
    <       "message": "exceeded retry limit, last status: 401 Unauthorized, request id: 9a0cb03a485067f7-SJC",
    <       "status_code": 401,
    <       "type": "error"
    <     }
    <   }
    < }
    ```
  • fix(app-server) move windows world writable warning (#6916)
    ## Summary
    Move the app-server warning into the process_new_conversation
    
    ## Testing
    - [x] Tested locally
  • storing credits (#6858)
    Expand the rate-limit cache/TUI: store credit snapshots alongside
    primary and secondary windows, render “Credits” when the backend reports
    they exist (unlimited vs rounded integer balances)
  • have world_writable_warning_details accept cwd as a param (#6913)
    this enables app-server to pass in the correct workspace cwd for the
    current conversation
  • feat: arcticfox in the wild (#6906)
    <img width="485" height="600" alt="image"
    src="https://github.com/user-attachments/assets/4341740d-dd58-4a3e-b69a-33a3be0606c5"
    />
    
    ---------
    
    Co-authored-by: jif-oai <jif@openai.com>
  • [app-server] populate thread>turns>items on thread/resume (#6848)
    This PR allows clients to render historical messages when resuming a
    thread via `thread/resume` by reading from the list of `EventMsg`
    payloads loaded from the rollout, and then transforming them into Turns
    and ThreadItems to be returned on the `Thread` object.
    
    This is implemented by leveraging `SessionConfiguredNotification` which
    returns this list of `EventMsg` objects when resuming a conversation,
    and then applying a stateful `ThreadHistoryBuilder` that parses from
    this EventMsg log and transforms it into Turns and ThreadItems.
    
    Note that we only persist a subset of `EventMsg`s in a rollout as
    defined in `policy.rs`, so we lose fidelity whenever we resume a thread
    compared to when we streamed the thread's turns originally. However,
    this behavior is at parity with the legacy API.
  • nit: useless log to debug (#6898)
    When you type too fast in most terminals, it gets interpreted as paste,
    making this log spam
  • fix(core) Support changing /approvals before conversation (#6836)
    ## Summary
    Setting `/approvals` before the start of a conversation was not updating
    the environment_context for a conversation. Not sure exactly when this
    problem was introduced, but this should reduce model confusion
    dramatically.
    
    ## Testing
    - [x] Added unit test to reproduce bug, confirmed fix with update
    - [x] Tested locally
  • chore(app-server) world-writable windows notification (#6880)
    ## Summary
    On app-server startup, detect whether the experimental sandbox is
    enabled, and send a notification .
    
    **Note**
    New conversations will not respect the feature because we [ignore cli
    overrides in
    NewConversation](https://github.com/openai/codex/blob/a75321a64c990275ed4368bf26a5334c9ddfa0a7/codex-rs/app-server/src/codex_message_processor.rs#L1237-L1252).
    However, this should be okay, since we don't actually use config for
    this, we use a [global
    variable](https://github.com/openai/codex/blob/87cce88f4865685a863e143e0fad4cf5ea542e62/codex-rs/core/src/safety.rs#L105-L110).
    We should carefully unwind this setup at some point.
    
    
    ## Testing
    - [ ] In progress: testing locally
    
    ---------
    
    Co-authored-by: jif-oai <jif@openai.com>
  • Move shell to use truncate_text (#6842)
    Move shell to use the configurable `truncate_text`
    
    ---------
    
    Co-authored-by: pakrym-oai <pakrym@openai.com>
  • flaky-unified_exec_formats_large_output_summary (#6884)
    # 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.
  • shell_command returns freeform output (#6860)
    Instead of returning structured out and then re-formatting it into
    freeform, return the freeform output from shell_command tool.
    
    Keep `shell` as the default tool for GPT-5.
  • fix(tui) ghost snapshot notifications (#6881)
    ## Summary
    - avoid surfacing ghost snapshot warnings in the TUI when snapshot
    creation fails, logging the conditions instead
    - continue to capture successful ghost snapshots without changing
    existing behavior
    
    ## Testing
    - `cargo test -p codex-core` *(fails:
    default_client::tests::test_create_client_sets_default_headers,
    default_client::tests::test_get_codex_user_agent,
    exec::tests::kill_child_process_group_kills_grandchildren_on_timeout)*
    
    ------
    [Codex
    Task](https://chatgpt.com/codex/tasks/task_i_691c02238db08322927c47b8c2d72c4c)
  • fix: typos in model picker (#6859)
    # 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: tweak windows sandbox strings (#6875)
    New strings:
    1. Approval mode picker just says "Select Approval Mode"
    1. Updated "Auto" to "Agent"
    1. When you select "Agent", you get "Agent mode on Windows uses an
    experimental sandbox to limit network and filesystem access. [Learn
    more]"
    1. Updated world-writable warning to "The Windows sandbox cannot protect
    writes to folders that are writable by Everyone. Consider removing write
    access for Everyone from the following folders: {folders}"
    
    ---------
    
    Co-authored-by: iceweasel-oai <iceweasel@openai.com>
  • fix: add more fields to ThreadStartResponse and ThreadResumeResponse (#6847)
    This adds the following fields to `ThreadStartResponse` and
    `ThreadResumeResponse`:
    
    ```rust
        pub model: String,
        pub model_provider: String,
        pub cwd: PathBuf,
        pub approval_policy: AskForApproval,
        pub sandbox: SandboxPolicy,
        pub reasoning_effort: Option<ReasoningEffort>,
    ```
    
    This is important because these fields are optional in
    `ThreadStartParams` and `ThreadResumeParams`, so the caller needs to be
    able to determine what values were ultimately used to start/resume the
    conversation. (Though note that any of these could be changed later
    between turns in the conversation.)
    
    Though to get this information reliably, it must be read from the
    internal `SessionConfiguredEvent` that is created in response to the
    start of a conversation. Because `SessionConfiguredEvent` (as defined in
    `codex-rs/protocol/src/protocol.rs`) did not have all of these fields, a
    number of them had to be added as part of this PR.
    
    Because `SessionConfiguredEvent` is referenced in many tests, test
    instances of `SessionConfiguredEvent` had to be updated, as well, which
    is why this PR touches so many files.
  • [app-server] introduce turn/completed v2 event (#6800)
    similar to logic in
    `codex/codex-rs/exec/src/event_processor_with_jsonl_output.rs`.
    translation of v1 -> v2 events:
    `codex/event/task_complete` -> `turn/completed`
    `codex/event/turn_aborted` -> `turn/completed` with `interrupted` status
    `codex/event/error` -> `turn/completed` with `error` status
    
    this PR also makes `items` field in `Turn` optional. For now, we only
    populate it when we resume a thread, and leave it as None for all other
    places until we properly rewrite core to keep track of items.
    
    tested using the codex app server client. example new event:
    ```
    < {
    <   "method": "turn/completed",
    <   "params": {
    <     "turn": {
    <       "id": "0",
    <       "items": [],
    <       "status": "interrupted"
    <     }
    <   }
    < }
    ```
  • tui: add branch to 'codex resume', filter by cwd (#6232)
    By default, show only sessions that shared a cwd with the current cwd.
    `--all` shows all sessions in all cwds. Also, show the branch name from
    the rollout metadata.
    
    <img width="1091" height="638" alt="Screenshot 2025-11-04 at 3 30 47 PM"
    src="https://github.com/user-attachments/assets/aae90308-6115-455f-aff7-22da5f1d9681"
    />
  • windows sandbox: support multiple workspace roots (#6854)
    The Windows sandbox did not previously support multiple workspace roots
    via config. Now it does
  • Fix tests so they don't emit an extraneous config.toml in the source tree (#6853)
    This PR fixes the `release_event_does_not_change_selection` test so it
    doesn't cause an extra `config.toml` to be emitted in the sources when
    running the tests locally. Prior to this fix, I needed to delete this
    file every time I ran the tests to prevent it from showing up as an
    uncommitted source file.
  • Improved runtime of generated_ts_has_no_optional_nullable_fields test (#6851)
    The `generated_ts_has_no_optional_nullable_fields` test was occasionally
    failing on slow CI nodes because of a timeout. This change reduces the
    work done by the test. It adds some "options" for the `generate_ts`
    function so it can skip work that's not needed for the test.
  • Fix typo in config.md for MCP server (#6845)
    # 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.