Commit Graph

13 Commits

  • Revert tui code so it does not rely on in-process app server (#14899)
    PR https://github.com/openai/codex/pull/14512 added an in-process app
    server and started to wire up the tui to use it. We were originally
    planning to modify the `tui` code in place, converting it to use the app
    server a bit at a time using a hybrid adapter. We've since decided to
    create an entirely new parallel `tui_app_server` implementation and do
    the conversion all at once but retain the existing `tui` while we work
    the bugs out of the new implementation.
    
    This PR undoes the changes to the `tui` made in the PR #14512 and
    restores the old initialization to its previous state. This allows us to
    modify the `tui_app_server` without the risk of regressing the old `tui`
    code. For example, we can start to remove support for all legacy core
    events, like the ones that PR https://github.com/openai/codex/pull/14892
    needed to ignore.
    
    Testing:
    * I manually verified that the old `tui` starts and shuts down without a
    problem.
  • Apply argument comment lint across codex-rs (#14652)
    ## Why
    
    Once the repo-local lint exists, `codex-rs` needs to follow the
    checked-in convention and CI needs to keep it from drifting. This commit
    applies the fallback `/*param*/` style consistently across existing
    positional literal call sites without changing those APIs.
    
    The longer-term preference is still to avoid APIs that require comments
    by choosing clearer parameter types and call shapes. This PR is
    intentionally the mechanical follow-through for the places where the
    existing signatures stay in place.
    
    After rebasing onto newer `main`, the rollout also had to cover newly
    introduced `tui_app_server` call sites. That made it clear the first cut
    of the CI job was too expensive for the common path: it was spending
    almost as much time installing `cargo-dylint` and re-testing the lint
    crate as a representative test job spends running product tests. The CI
    update keeps the full workspace enforcement but trims that extra
    overhead from ordinary `codex-rs` PRs.
    
    ## What changed
    
    - keep a dedicated `argument_comment_lint` job in `rust-ci`
    - mechanically annotate remaining opaque positional literals across
    `codex-rs` with exact `/*param*/` comments, including the rebased
    `tui_app_server` call sites that now fall under the lint
    - keep the checked-in style aligned with the lint policy by using
    `/*param*/` and leaving string and char literals uncommented
    - cache `cargo-dylint`, `dylint-link`, and the relevant Cargo
    registry/git metadata in the lint job
    - split changed-path detection so the lint crate's own `cargo test` step
    runs only when `tools/argument-comment-lint/*` or `rust-ci.yml` changes
    - continue to run the repo wrapper over the `codex-rs` workspace, so
    product-code enforcement is unchanged
    
    Most of the code changes in this commit are intentionally mechanical
    comment rewrites or insertions driven by the lint itself.
    
    ## Verification
    
    - `./tools/argument-comment-lint/run.sh --workspace`
    - `cargo test -p codex-tui-app-server -p codex-tui`
    - parsed `.github/workflows/rust-ci.yml` locally with PyYAML
    
    ---
    
    * -> #14652
    * #14651
  • Start TUI on embedded app server (#14512)
    This PR is part of the effort to move the TUI on top of the app server.
    In a previous PR, we introduced an in-process app server and moved
    `exec` on top of it.
    
    For the TUI, we want to do the migration in stages. The app server
    doesn't currently expose all of the functionality required by the TUI,
    so we're going to need to support a hybrid approach as we make the
    transition.
    
    This PR changes the TUI initialization to instantiate an in-process app
    server and access its `AuthManager` and `ThreadManager` rather than
    constructing its own copies. It also adds a placeholder TUI event
    handler that will eventually translate app server events into TUI
    events. App server notifications are accepted but ignored for now. It
    also adds proper shutdown of the app server when the TUI terminates.
  • feat(tui) render request_permissions calls (#14004)
    ## Summary
    Adds support for tui rendering of request_permission calls
    
    <img width="724" height="245" alt="Screenshot 2026-03-08 at 9 04 07 PM"
    src="https://github.com/user-attachments/assets/e1997825-a496-4bfb-bbda-43d0006460a5"
    />
    
    
    ## Testing
    - [x] Added snapshot test
  • app-server: include experimental skill metadata in exec approval requests (#13929)
    ## Summary
    
    This change surfaces skill metadata on command approval requests so
    app-server clients can tell when an approval came from a skill script
    and identify the originating `SKILL.md`.
    
    - add `skill_metadata` to exec approval events in the shared protocol
    - thread skill metadata through core shell escalation and delegated
    approval handling for skill-triggered approvals
    - expose the field in app-server v2 as experimental `skillMetadata`
    - regenerate the JSON/TypeScript schemas and cover the new field in
    protocol, transport, core, and TUI tests
    
    ## Why
    
    Skill-triggered approvals already carry skill context inside core, but
    app-server clients could not see which skill caused the prompt. Sending
    the skill metadata with the approval request makes it possible for
    clients to present better approval UX and connect the prompt back to the
    relevant skill definition.
    
    
    ## example event in app-server-v2
    verified that we see this event when experimental api is on:
    ```
    < {
    <   "id": 11,
    <   "method": "item/commandExecution/requestApproval",
    <   "params": {
    <     "additionalPermissions": {
    <       "fileSystem": null,
    <       "macos": {
    <         "accessibility": false,
    <         "automations": {
    <           "bundle_ids": [
    <             "com.apple.Notes"
    <           ]
    <         },
    <         "calendar": false,
    <         "preferences": "read_only"
    <       },
    <       "network": null
    <     },
    <     "approvalId": "25d600ee-5a3c-4746-8d17-e2e61fb4c563",
    <     "availableDecisions": [
    <       "accept",
    <       "acceptForSession",
    <       "cancel"
    <     ],
    <     "command": "/Applications/ChatGPT.app/Contents/Resources/CodexAppServer_CodexAppServerBundledSkills.bundle/Contents/Resources/skills/apple-notes/scripts/notes_info",
    <     "commandActions": [
    <       {
    <         "command": "/Applications/ChatGPT.app/Contents/Resources/CodexAppServer_CodexAppServerBundledSkills.bundle/Contents/Resources/skills/apple-notes/scripts/notes_info",
    <         "type": "unknown"
    <       }
    <     ],
    <     "cwd": "/Applications/ChatGPT.app/Contents/Resources/CodexAppServer_CodexAppServerBundledSkills.bundle/Contents/Resources/skills/apple-notes",
    <     "itemId": "call_jZp3xFpNg4D8iKAD49cvEvZy",
    <     "skillMetadata": {
    <       "pathToSkillsMd": "/Applications/ChatGPT.app/Contents/Resources/CodexAppServer_CodexAppServerBundledSkills.bundle/Contents/Resources/skills/apple-notes/SKILL.md"
    <     },
    <     "threadId": "019ccc10-b7d3-7ff2-84fe-3a75e7681e69",
    <     "turnId": "019ccc10-b848-76f1-81b3-4a1fa225493f"
    <   }
    < }`
    ```
    
    & verified that this is the event when experimental api is off:
    ```
    < {
    <   "id": 13,
    <   "method": "item/commandExecution/requestApproval",
    <   "params": {
    <     "approvalId": "5fbbf776-261b-4cf8-899b-c125b547f2c0",
    <     "availableDecisions": [
    <       "accept",
    <       "acceptForSession",
    <       "cancel"
    <     ],
    <     "command": "/Applications/ChatGPT.app/Contents/Resources/CodexAppServer_CodexAppServerBundledSkills.bundle/Contents/Resources/skills/apple-notes/scripts/notes_info",
    <     "commandActions": [
    <       {
    <         "command": "/Applications/ChatGPT.app/Contents/Resources/CodexAppServer_CodexAppServerBundledSkills.bundle/Contents/Resources/skills/apple-notes/scripts/notes_info",
    <         "type": "unknown"
    <       }
    <     ],
    <     "cwd": "/Users/celia/code/codex/codex-rs",
    <     "itemId": "call_OV2DHzTgYcbYtWaTTBWlocOt",
    <     "threadId": "019ccc16-2a2b-7be1-8500-e00d45b892d4",
    <     "turnId": "019ccc16-2a8e-7961-98ec-649600e7d06a"
    <   }
    < }
    ```
  • [elicitations] Switch to use MCP style elicitation payload for mcp tool approvals. (#13621)
    - [x] Switch to use MCP style elicitation payload for mcp tool
    approvals.
    - [ ] TODO: Update the UI to support the full spec.
  • feat(app-server): support mcp elicitations in v2 api (#13425)
    This adds a first-class server request for MCP server elicitations:
    `mcpServer/elicitation/request`.
    
    Until now, MCP elicitation requests only showed up as a raw
    `codex/event/elicitation_request` event from core. That made it hard for
    v2 clients to handle elicitations using the same request/response flow
    as other server-driven interactions (like shell and `apply_patch`
    tools).
    
    This also updates the underlying MCP elicitation request handling in
    core to pass through the full MCP request (including URL and form data)
    so we can expose it properly in app-server.
    
    ### Why not `item/mcpToolCall/elicitationRequest`?
    This is because MCP elicitations are related to MCP servers first, and
    only optionally to a specific MCP tool call.
    
    In the MCP protocol, elicitation is a server-to-client capability: the
    server sends `elicitation/create`, and the client replies with an
    elicitation result. RMCP models it that way as well.
    
    In practice an elicitation is often triggered by an MCP tool call, but
    not always.
    
    ### What changed
    - add `mcpServer/elicitation/request` to the v2 app-server API
    - translate core `codex/event/elicitation_request` events into the new
    v2 server request
    - map client responses back into `Op::ResolveElicitation` so the MCP
    server can continue
    - update app-server docs and generated protocol schema
    - add an end-to-end app-server test that covers the full round trip
    through a real RMCP elicitation flow
    - The new test exercises a realistic case where an MCP tool call
    triggers an elicitation, the app-server emits
    mcpServer/elicitation/request, the client accepts it, and the tool call
    resumes and completes successfully.
    
    ### app-server API flow
    - Client starts a thread with `thread/start`.
    - Client starts a turn with `turn/start`.
    - App-server sends `item/started` for the `mcpToolCall`.
    - While that tool call is in progress, app-server sends
    `mcpServer/elicitation/request`.
    - Client responds to that request with `{ action: "accept" | "decline" |
    "cancel" }`.
    - App-server sends `serverRequest/resolved`.
    - App-server sends `item/completed` for the mcpToolCall.
    - App-server sends `turn/completed`.
    - If the turn is interrupted while the elicitation is pending,
    app-server still sends `serverRequest/resolved` before the turn
    finishes.
  • feat: include available decisions in command approval requests (#12758)
    Command-approval clients currently infer which choices to show from
    side-channel fields like `networkApprovalContext`,
    `proposedExecpolicyAmendment`, and `additionalPermissions`. That makes
    the request shape harder to evolve, and it forces each client to
    replicate the server's heuristics instead of receiving the exact
    decision list for the prompt.
    
    This PR introduces a mapping between `CommandExecutionApprovalDecision`
    and `codex_protocol::protocol::ReviewDecision`:
    
    ```rust
    impl From<CoreReviewDecision> for CommandExecutionApprovalDecision {
        fn from(value: CoreReviewDecision) -> Self {
            match value {
                CoreReviewDecision::Approved => Self::Accept,
                CoreReviewDecision::ApprovedExecpolicyAmendment {
                    proposed_execpolicy_amendment,
                } => Self::AcceptWithExecpolicyAmendment {
                    execpolicy_amendment: proposed_execpolicy_amendment.into(),
                },
                CoreReviewDecision::ApprovedForSession => Self::AcceptForSession,
                CoreReviewDecision::NetworkPolicyAmendment {
                    network_policy_amendment,
                } => Self::ApplyNetworkPolicyAmendment {
                    network_policy_amendment: network_policy_amendment.into(),
                },
                CoreReviewDecision::Abort => Self::Cancel,
                CoreReviewDecision::Denied => Self::Decline,
            }
        }
    }
    ```
    
    And updates `CommandExecutionRequestApprovalParams` to have a new field:
    
    ```rust
    available_decisions: Option<Vec<CommandExecutionApprovalDecision>>
    ```
    
    when, if specified, should make it easier for clients to display an
    appropriate list of options in the UI.
    
    This makes it possible for `CoreShellActionProvider::prompt()` in
    `unix_escalation.rs` to specify the `Vec<ReviewDecision>` directly,
    adding support for `ApprovedForSession` when approving a skill script,
    which was previously missing in the TUI.
    
    Note this results in a significant change to `exec_options()` in
    `approval_overlay.rs`, as the displayed options are now derived from
    `available_decisions: &[ReviewDecision]`.
    
    ## What Changed
    
    - Add `available_decisions` to
    [`ExecApprovalRequestEvent`](https://github.com/openai/codex/blob/de00e932dd9801de0a4faac0519162099753f331/codex-rs/protocol/src/approvals.rs#L111-L175),
    including helpers to derive the legacy default choices when older
    senders omit the field.
    - Map `codex_protocol::protocol::ReviewDecision` to app-server
    `CommandExecutionApprovalDecision` and expose the ordered list as
    experimental `availableDecisions` in
    [`CommandExecutionRequestApprovalParams`](https://github.com/openai/codex/blob/de00e932dd9801de0a4faac0519162099753f331/codex-rs/app-server-protocol/src/protocol/v2.rs#L3798-L3807).
    - Thread optional `available_decisions` through the core approval path
    so Unix shell escalation can explicitly request `ApprovedForSession` for
    session-scoped approvals instead of relying on client heuristics.
    [`unix_escalation.rs`](https://github.com/openai/codex/blob/de00e932dd9801de0a4faac0519162099753f331/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs#L194-L214)
    - Update the TUI approval overlay to build its buttons from the ordered
    decision list, while preserving the legacy fallback when
    `available_decisions` is missing.
    - Update the app-server README, test client output, and generated schema
    artifacts to document and surface the new field.
    
    ## Testing
    
    - Add `approval_overlay.rs` coverage for explicit decision lists,
    including the generic `ApprovedForSession` path and network approval
    options.
    - Update `chatwidget/tests.rs` and app-server protocol tests to populate
    the new optional field and keep older event shapes working.
    
    ## Developers Docs
    
    - If we document `item/commandExecution/requestApproval` on
    [developers.openai.com/codex](https://developers.openai.com/codex), add
    experimental `availableDecisions` as the preferred source of approval
    choices and note that older servers may omit it.
  • Display pending child-thread approvals in TUI (#12767)
    Summary
    - propagate approval policy from parent to spawned agents and drop the
    Never override so sub-agents respect the caller’s request
    - refresh the pending-approval list whenever events arrive or the active
    thread changes and surface the list above the composer for inactive
    threads
    - add widgets, helpers, and tests covering the new pending-thread
    approval UI state
    
    ![Uploading Screenshot 2026-02-25 at 11.02.18.png…]()
  • fix: chatwidget was not honoring approval_id for an ExecApprovalRequestEvent (#12746)
    ## Why
    
    `ExecApprovalRequestEvent` can carry a distinct `approval_id` for
    subcommand approvals, including the `execve`-intercepted zsh-fork path.
    
    The session registers the pending approval callback under `approval_id`
    when one is present, but `ChatWidget` was stashing `call_id` in the
    approval modal state. When the user approved the command in the TUI, the
    response was sent back with the wrong identifier, so the pending
    approval could not be matched and the approval callback would not
    resolve.
    
    Note `approval_id` was introduced in
    https://github.com/openai/codex/pull/12051.
    
    ## What changed
    
    - In `tui/src/chatwidget.rs`, `ChatWidget` now uses
    `ExecApprovalRequestEvent::effective_approval_id()` when constructing
    `ApprovalRequest::Exec`.
    - That preserves the existing behavior for normal shell and
    `unified_exec` approvals, where `approval_id` is absent and the
    effective id still falls back to `call_id`.
    - For subcommand approvals that provide a distinct `approval_id`, the
    TUI now sends back the same key that
    `Session::request_command_approval()` registered.
    
    ## Verification
    
    - Traced the approval flow end to end to confirm the same effective
    approval id is now used on both sides of the round trip:
    - `Session::request_command_approval()` registers the pending callback
    under `approval_id.unwrap_or(call_id)`.
    - `ChatWidget` now emits `Op::ExecApproval` with that same effective id.
  • feat: run zsh fork shell tool via shell-escalation (#12649)
    ## Why
    
    This PR switches the `shell_command` zsh-fork path over to
    `codex-shell-escalation` so the new shell tool can use the shared
    exec-wrapper/escalation protocol instead of the `zsh_exec_bridge`
    implementation that was introduced in
    https://github.com/openai/codex/pull/12052. `zsh_exec_bridge` relied on
    UNIX domain sockets, which is not as tamper-proof as the FD-based
    approach in `codex-shell-escalation`.
    
    ## What Changed
    
    - Added a Unix zsh-fork runtime adapter in `core`
    (`core/src/tools/runtimes/shell/unix_escalation.rs`) that:
    - runs zsh-fork commands through
    `codex_shell_escalation::run_escalate_server`
      - bridges exec-policy / approval decisions into `ShellActionProvider`
    - executes escalated commands via a `ShellCommandExecutor` that calls
    `process_exec_tool_call`
    - Updated `ShellRuntime` / `ShellCommandHandler` / tool spec wiring to
    select a `shell_command` backend (`classic` vs `zsh-fork`) while leaving
    the generic `shell` tool path unchanged.
    - Removed the `zsh_exec_bridge`-based session service and deleted
    `core/src/zsh_exec_bridge/mod.rs`.
    - Moved exec-wrapper entrypoint dispatch to `arg0` by handling the
    `codex-execve-wrapper` arg0 alias there, and removed the old
    `codex_core::maybe_run_zsh_exec_wrapper_mode()` hooks from `cli` and
    `app-server` mains.
    - Added the needed `codex-shell-escalation` dependencies for `core` and
    `arg0`.
    
    ## Tests
    
    - `cargo test -p codex-core
    shell_zsh_fork_prefers_shell_command_over_unified_exec`
    - `cargo test -p codex-app-server turn_start_shell_zsh_fork --
    --nocapture`
    - verifies zsh-fork command execution and approval flows through the new
    backend
    - includes subcommand approve/decline coverage using the shared zsh
    DotSlash fixture in `app-server/tests/suite/zsh`
    - To test manually, I added the following to `~/.codex/config.toml`:
    
    ```toml
    zsh_path = "/Users/mbolin/code/codex3/codex-rs/app-server/tests/suite/zsh"
    
    [features]
    shell_zsh_fork = true
    ```
    
    Then I ran `just c` to run the dev build of Codex with these changes and
    sent it the message:
    
    ```
    run `echo $0`
    ```
    
    And it replied with:
    
    ```
      echo $0 printed:
    
      /Users/mbolin/code/codex3/codex-rs/app-server/tests/suite/zsh
    
      In this tool context, $0 reflects the script path used to invoke the shell, not just zsh.
    ```
    
    so the tool appears to be wired up correctly.
    
    ## Notes
    
    - The zsh subcommand-decline integration test now uses `rm` under a
    `WorkspaceWrite` sandbox. The previous `/usr/bin/true` scenario is
    auto-allowed by the new `shell-escalation` policy path, which no longer
    produces subcommand approval prompts.
  • fix: replay after /agent (#12663)
    Filter the events after a`/agent` replay to prevent replaying decision
    events