Commit Graph

2247 Commits

  • Migrate codex max (#7566)
    - make codex max the default
    - fix: we were doing some async work in sync function which caused tui
    to panic
  • [app-server] make file_path for config optional (#7560)
    When we are writing to config using `config/value/write` or
    `config/batchWrite`, it always require a `config/read` before it right
    now in order to get the correct file path to write to. make this
    optional so we read from the default user config file if this is not
    passed in.
  • 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.
  • Migrate tui to use models manager (#7555)
    - This PR treats the `ModelsManager` like `AuthManager` and propagate it
    into the tui, replacing the `builtin_model_presets`
    - We are also decreasing the visibility of `builtin_model_presets`
    
    based on https://github.com/openai/codex/pull/7552
  • fix: wrap long exec lines in transcript overlay (#7481)
    What
    -----
    - Fix the Ctrl+T transcript overlay so that very long exec output lines
    are soft‑wrapped to the viewport width instead of being rendered as a
    single truncated row.
    - Add a regression test to `TranscriptOverlay` to ensure long exec
    outputs are rendered on multiple lines in the overlay.
    
    Why
    ----
    - Previously, the transcript overlay rendered extremely long single exec
    lines as one on‑screen row and simply cut them off at the right edge,
    with no horizontal scrolling.
    - This made it impossible to inspect the full content of long tool/exec
    outputs in the transcript view, even though the main TUI view already
    wrapped those lines.
    - Fixes #7454.
    
    How
    ----
    - Update `ExecCell::transcript_lines` to wrap exec output lines using
    the existing `RtOptions`/`word_wrap_line` helpers so that transcript
    rendering is width‑aware.
    - Reuse the existing line utilities to expand the wrapped `Line` values
    into the transcript overlay, preserving styling while respecting the
    current viewport width.
    - Add `transcript_overlay_wraps_long_exec_output_lines` test in
    `pager_overlay.rs` that constructs a long single‑line exec output,
    renders the transcript overlay into a small buffer, and asserts that the
    long marker string spans multiple rendered lines.
  • fix: Features should be immutable over the lifetime of a session/thread (#7540)
    I noticed that `features: Features` was defined on `struct
    SessionConfiguration`, which is commonly owned by `SessionState`, which
    is in turn owned by `Session`.
    
    Though I do not believe that `Features` should be allowed to be modified
    over the course of a session (if the feature state is not invariant, it
    makes it harder to reason about), which argues that it should live on
    `Session` rather than `SessionState` or `SessionConfiguration`.
    
    This PR moves `Features` to `Session` and updates all call sites. It
    appears the only place we were mutating `Features` was:
    
    - in tests
    - the sub-agent config for a review task:
    
    
    https://github.com/openai/codex/blob/3ef76ff29d5eed258fb6b8550e0e2b973d0dca21/codex-rs/core/src/tasks/review.rs#L86-L89
    
    Note this change also means it is no longer an `async` call to check the
    state of a feature, eliminating the possibility of a
    [TOCTTOU](https://en.wikipedia.org/wiki/Time-of-check_to_time-of-use)
    error between checking the state of a feature and acting on it:
    
    
    https://github.com/openai/codex/blob/3ef76ff29d5eed258fb6b8550e0e2b973d0dca21/codex-rs/core/src/codex.rs#L1069-L1076
  • feat: Support listing and selecting skills via $ or /skills (#7506)
    List/Select skills with $-mention or /skills
  • [app-server] fix: add thread_id to turn/plan/updated (#7553)
    Realized we're missing this while migrating VSCE.
  • feat(tui): map Ctrl-P/N to arrow navigation in textarea (#7530)
    - Treat Ctrl-P/N (and their C0 fallbacks) the same as Up/Down so cursor
    movement matches popup/history behavior and control bytes never land in
    the buffer
    
    Fixes #7529
    
    Signed-off-by: Aofei Sheng <aofei@aofeisheng.com>
  • fix(tui): limit user shell output by screen lines (#7448)
    What
    - Limit the TUI "user shell" output panel by the number of visible
    screen lines rather than by the number of logical lines.
    - Apply middle truncation after wrapping, so a few extremely long lines
    cannot expand into hundreds of visible lines.
    - Add a regression test to guard this behavior.
    
    Why
    When the `ExecCommandSource::UserShell` tool returns a small number of
    very long logical lines, the TUI wraps those lines into many visual
    lines. The existing truncation logic applied
    `USER_SHELL_TOOL_CALL_MAX_LINES` to the number of logical lines *before*
    wrapping.
    
    As a result, a command like:
    
    - `Ran bash -lc "grep -R --line-number 'maskAssetId' ."`
    
    or a synthetic command that prints a single ~50,000‑character line, can
    produce hundreds of screen lines and effectively flood the viewport. The
    intended middle truncation for user shell output does not take effect in
    this scenario.
    
    How
    - In `codex-rs/tui/src/exec_cell/render.rs`, change the `ExecCell`
    rendering path for `ExecCommandSource::UserShell` so that:
    - Each logical line from `CommandOutput::aggregated_output` is first
    wrapped via `word_wrap_line` into multiple screen lines using the
    appropriate `RtOptions` and width from the `EXEC_DISPLAY_LAYOUT`
    configuration.
    - `truncate_lines_middle` is then applied to the wrapped screen lines,
    with `USER_SHELL_TOOL_CALL_MAX_LINES` as the limit. This means the limit
    is enforced on visible screen lines, not logical lines.
    - The existing layout struct (`ExecDisplayLayout`) continues to provide
    `output_max_lines`, so user shell output is subject to both
    `USER_SHELL_TOOL_CALL_MAX_LINES` and the layout-specific
    `output_max_lines` constraint.
    - Keep using `USER_SHELL_TOOL_CALL_MAX_LINES` as the cap, but interpret
    it as a per‑tool‑call limit on screen lines.
    - Add a regression test `user_shell_output_is_limited_by_screen_lines`
    in `codex-rs/tui/src/exec_cell/render.rs` that:
    - Constructs two extremely long logical lines containing a short marker
    (`"Z"`), so each wrapped screen line still contains the marker.
      - Wraps them at a narrow width to generate many screen lines.
    - Asserts that the unbounded wrapped output would exceed
    `USER_SHELL_TOOL_CALL_MAX_LINES` screen lines.
    - Renders an `ExecCell` for `ExecCommandSource::UserShell` at the same
    width and counts rendered lines containing the marker.
    - Asserts `output_screen_lines <= USER_SHELL_TOOL_CALL_MAX_LINES`,
    guarding against regressions where truncation happens before wrapping.
    
    This change keeps user shell output readable while ensuring it cannot
    flood the TUI, even when the tool emits a few extremely long lines.
    
    Tests
    - `cargo test -p codex-tui`
    
    Issue
    - Fixes #7447
  • Migrate model preset (#7542)
    - Introduce `openai_models` in `/core`
    - Move `PRESETS` under it
    - Move `ModelPreset`, `ModelUpgrade`, `ReasoningEffortPreset`,
    `ReasoningEffortPreset`, and `ReasoningEffortPreset` to `protocol`
    - Introduce `Op::ListModels` and `EventMsg::AvailableModels`
    
    Next steps:
    - migrate `app-server` and `tui` to use the introduced Operation
  • chore: update unified exec sandboxing detection (#7541)
    No integration test for now because it would make them flaky. Tracking
    it in my todos to add some once we have a clock based system for
    integration tests
  • add slash resume (#7302)
    `codex resume` isn't that discoverable. Adding it to the slash commands
    can help
  • chore: conversation_id -> thread_id in app-server feedback/upload (#7538)
    Use `thread_id: Option<String>` instead of `conversation_id:
    Option<ConversationId>` to be consistent with the rest of app-server v2
    APIs.
  • chore: delete unused TodoList item from app-server (#7537)
    This item is sent as a turn notification instead: `turn/plan/updated`,
    similar to Turn diffs (which is `turn/diff/updated`).
    
    We treat these concepts as ephemeral compared to Items which are usually
    persisted.
  • chore: update app-server README (#7510)
    Just keeping the README up to date.
    
    - Reorganize structure a bit to read more naturally
    - Update RPC methods
    - Update events
  • chore: remove bun env var detect (#7534)
    ### Summary
    
    
    [Thread](https://openai.slack.com/archives/C08JZTV654K/p1764780129457519)
    
    We were a bit aggressive on assuming package installer based on env
    variables for BUN. Here we are removing those checks.
  • feat: support list mcp servers in app server (#7505)
    ### Summary
    Added `mcp/servers/list` which is equivalent to `/mcp` slash command in
    CLI for response. This will be used in VSCE MCP settings to show log in
    status, available tools etc.
  • seatbelt: allow openpty() (#7507)
    This allows `openpty(3)` to run in the default sandbox. Also permit
    reading `kern.argmax`, which is the maximum number of arguments to
    exec().
  • feat: codex tool tips (#7440)
    <img width="551" height="316" alt="Screenshot 2025-12-01 at 12 22 26"
    src="https://github.com/user-attachments/assets/6ca3deff-8ef8-4f74-a8e1-e5ea13fd6740"
    />
  • feat: retroactive image placeholder to prevent poisoning (#6774)
    If an image can't be read by the API, it will poison the entire history,
    preventing any new turn on the conversation.
    This detect such cases and replace the image by a placeholder
  • fix(tui) Support image paste from clipboard on native Windows (#7514)
    Closes #3404 
    
    ## Summary
    On windows, ctrl+v does not work for the same reason that cmd+v does not
    work on macos. This PR adds alt/option+v detection, which allows windows
    users to paste images from the clipboard using.
    
    We could swap between just ctrl on mac and just alt on windows, but this
    felt simpler - I don't feel strongly about it.
    
    Note that this will NOT address image pasting in WSL environments, due
    to issues with WSL <> Windows clipboards. I'm planning to address that
    in a separate PR since it will likely warrant some discussion.
    
    ## Testing
    - [x] Tested locally on a Mac and Windows laptop
  • 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.
  • Update device code auth strings. (#7498)
    - [x] Update device code auth strings.
  • fix: inline function marked as dead code (#7508)
    I was debugging something else and noticed we could eliminate an
    instance of `#[allow(dead_code)]` pretty easily.
  • improve resume performance (#7303)
    Reading the tail can be costly if we have a very big rollout item. we
    can just read the file metadata
  • fix: path resolution bug in npx (#7134)
    When running `npx @openai/codex-shell-tool-mcp`, the old code derived
    `__dirname` from `process.argv[1]`, which points to npx’s transient
    wrapper script in
    `~/.npm/_npx/134d0fb7e1a27652/node_modules/.bin/codex-shell-tool-mcp`.
    That made `vendorRoot` resolve to `<npx cache>/vendor`, so the startup
    checks failed with "Required binary missing" because it looked for
    `codex-execve-wrapper` in the wrong place.
    
    By relying on the real module `__dirname` and `path.resolve(__dirname,
    "..", "vendor")`, the package now anchors to its installed location
    under `node_modules/@openai/codex-shell-tool-mcp/`, so the bundled
    binaries are found and npx launches correctly.
  • Ensure duplicate-length paste placeholders stay distinct (#7431)
    Fix issue #7430 
    Generate unique numbered placeholders for multiple large pastes of the
    same length so deleting one no longer removes the others.
    
    Signed-off-by: Joshua <joshua1s@protonmail.com>
  • feat: support --version flag for @openai/codex-shell-tool-mcp (#7504)
    I find it helpful to easily verify which version is running.
    
    Tested:
    
    ```shell
    ~/code/codex3/codex-rs/exec-server$ cargo run --bin codex-exec-mcp-server -- --help
        Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.19s
         Running `/Users/mbolin/code/codex3/codex-rs/target/debug/codex-exec-mcp-server --help`
    Usage: codex-exec-mcp-server [OPTIONS]
    
    Options:
          --execve <EXECVE_WRAPPER>  Executable to delegate execve(2) calls to in Bash
          --bash <BASH_PATH>         Path to Bash that has been patched to support execve() wrapping
      -h, --help                     Print help
      -V, --version                  Print version
    ~/code/codex3/codex-rs/exec-server$ cargo run --bin codex-exec-mcp-server -- --version
        Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.17s
         Running `/Users/mbolin/code/codex3/codex-rs/target/debug/codex-exec-mcp-server --version`
    codex-exec-server 0.0.0
    ```
  • refactor: tui.rs extract several pieces (#7461)
    Pull FrameRequester out of tui.rs into its own module and make a
    FrameScheduler struct. This is effectively an Actor/Handler approach
    (see https://ryhl.io/blog/actors-with-tokio/). Adds tests and docs.
    
    Small refactor of pending_viewport_area logic.
  • 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`.
  • Trim history.jsonl when history.max_bytes is set (#6242)
    This PR honors the `history.max_bytes` configuration parameter by
    trimming `history.jsonl` whenever it grows past the configured limit.
    While appending new entries we retain the newest record, drop the oldest
    lines to stay within the byte budget, and serialize the compacted file
    back to disk under the same lock to keep writers safe.
  • fix: remove serde(flatten) annotation for TurnError (#7499)
    The problem with using `serde(flatten)` on Turn status is that it
    conditionally serializes the `error` field, which is not the pattern we
    want in API v2 where all fields on an object should always be returned.
    
    ```
    #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
    #[serde(rename_all = "camelCase")]
    #[ts(export_to = "v2/")]
    pub struct Turn {
        pub id: String,
        /// Only populated on a `thread/resume` response.
        /// For all other responses and notifications returning a Turn,
        /// the items field will be an empty list.
        pub items: Vec<ThreadItem>,
        #[serde(flatten)]
        pub status: TurnStatus,
    }
    
    #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
    #[serde(tag = "status", rename_all = "camelCase")]
    #[ts(tag = "status", export_to = "v2/")]
    pub enum TurnStatus {
        Completed,
        Interrupted,
        Failed { error: TurnError },
        InProgress,
    }
    ```
    
    serializes to:
    ```
    {
      "id": "turn-123",
      "items": [],
      "status": "completed"
    }
    
    {
      "id": "turn-123",
      "items": [],
      "status": "failed",
      "error": {
        "message": "Tool timeout",
        "codexErrorInfo": null
      }
    }
    ```
    
    Instead we want:
    ```
    {
      "id": "turn-123",
      "items": [],
      "status": "completed",
      "error": null
    }
    
    {
      "id": "turn-123",
      "items": [],
      "status": "failed",
      "error": {
        "message": "Tool timeout",
        "codexErrorInfo": null
      }
    }
    ```
  • persisting credits if new snapshot does not contain credit info (#7490)
    in response to incoming changes to responses headers where the header
    may sometimes not contain credits info (no longer forcing a credit
    check)
  • execpolicy helpers (#7032)
    this PR 
    - adds a helper function to amend `.codexpolicy` files with new prefix
    rules
    - adds a utility to `Policy` allowing prefix rules to be added to
    existing `Policy` structs
    
    both additions will be helpful as we thread codexpolicy into the TUI
    workflow
  • Show token used when context window is unknown (#7497)
    - Show context window usage in tokens instead of percentage when the
    window length is unknown.
  • Fix: track only untracked paths in ghost snapshots (#7470)
    # Ghost snapshot ignores
    
    This PR should close #7067, #7395, #7405.
    
    Prior to this change the ghost snapshot task ran `git status
    --ignored=matching` so the report picked up literally every ignored
    file. When a directory only contained entries matched by patterns such
    as `dozens/*.txt`, `/test123/generated/*.html`, or `/wp-includes/*`, Git
    still enumerated them and the large-untracked-dir detection treated the
    parent directory as “large,” even though everything inside was
    intentionally ignored.
    
    By removing `--ignored=matching` we only capture true untracked paths
    now, so those patterns stay out of the snapshot report and no longer
    trigger the “large untracked directories” warning.
    
    ---------
    
    Signed-off-by: lionelchg <lionel.cheng@hotmail.fr>
    Co-authored-by: lionelchg <lionel.cheng@hotmail.fr>
  • fix: add ts number annotations for app-server v2 types (#7492)
    These will be more ergonomic to work with in Typescript.
  • Add request logging back (#7471)
    Having full requests helps debugging