Commit Graph

5 Commits

  • Route MCP servers through explicit environments (#23583)
    ## Summary
    - route each configured MCP server through an explicit per-server
    `environment_id` instead of a manager-wide remote toggle
    - default omitted `environment_id` to `local`, resolve named ids through
    `EnvironmentManager`, and fail only the affected MCP server when an
    explicit id is unknown
    - keep local stdio on the existing local launcher path for now, while
    named-environment stdio uses the selected environment backend and
    requires an absolute `cwd`
    - allow local HTTP MCP servers to keep using the ambient HTTP client
    when no local `Environment` is configured; named-environment HTTP MCPs
    use that environment's HTTP client
    
    ## Validation
    - devbox Bazel build: `bazel build --bes_backend= --bes_results_url=
    //codex-rs/cli:codex //codex-rs/rmcp-client:test_stdio_server
    //codex-rs/rmcp-client:test_streamable_http_server`
    - devbox app-server config matrix with real `config.toml` /
    `environments.toml` files covering omitted local, explicit local,
    omitted local under remote default, explicit remote stdio, local HTTP
    without local env, explicit remote HTTP, local stdio without local env,
    unknown explicit env, and remote stdio without `cwd`
  • fix(app-server): speed up shutdown (#23578)
    ## Why
    
    Pressing `Ctrl+C` or `Ctrl+D` in the TUI could make Codex pause during
    shutdown when app-server background work still held outbound sender
    clones.
    
    Shutdown tracing against the current `~/.codex` path found three
    relevant holders:
    
    - `SkillsWatcher` kept its event-loop task alive until the shutdown
    timeout path.
    - `AppServerAttestationProvider` retained a strong
    `Arc<OutgoingMessageSender>`, which could keep outbound teardown waiting
    after the processor task had exited.
    - A background `apps/list` task could still own an outbound sender when
    shutdown began, causing the in-process app-server runtime to wait for
    its outbound channel to close.
    
    ## What Changed
    
    - Give `SkillsWatcher` an explicit shutdown `CancellationToken` and
    cancel it from app-server teardown so its event loop drops the outbound
    sender promptly.
    - Change `AppServerAttestationProvider` to keep a
    `Weak<OutgoingMessageSender>` and return immediately when it can no
    longer be upgraded.
    - Give `AppsRequestProcessor` a shutdown `CancellationToken` and cancel
    in-flight background `apps/list` work during teardown.
    
    ## How to Test
    
    1. Start Codex TUI from a real home configuration.
    2. Press `Ctrl+C`.
    3. Confirm Codex exits promptly instead of pausing during shutdown.
    4. Repeat with `Ctrl+D` and confirm the same prompt exit path.
    
    Focused manual trace validation from the investigation:
    
    - Before the full fix, reproduced shutdown traces showed outbound
    teardown waiting on lingering owners, including `attestation.provider=1`
    and later `apps.list.task=1`.
    - After the fix, fresh real-home `Ctrl+D` traces showed
    `app_server.runtime.outbound_state_after_processor_join` with
    `owners=none`, `app_server.runtime.wait_outbound_handle = 0ms`, and
    total TUI app-server shutdown around `18ms`.
    
    Targeted validation:
    
    - `RUST_MIN_STACK=8388608 cargo test -p codex-app-server`
  • [2 of 4] tui: route app and skill enablement through app server (#22914)
    ## Why
    App and skill toggles are user config mutations too. When the TUI is
    attached to a remote app server, writing those toggles into the local
    `config.toml` makes the UI report success without updating the server
    that actually owns the session.
    
    This is **[2 of 4]** in a stacked series that moves TUI-owned config
    mutations onto app-server APIs.
    
    ## What changed
    - Routed app enable/disable persistence through app-server config batch
    writes.
    - Routed skill enable/disable persistence through `skills/config/write`.
    - Avoided refreshing local config from disk after these writes when the
    TUI is connected to a remote app server.
    
    ## Config keys affected
    - `apps.<app_id>.enabled`
    - `apps.<app_id>.disabled_reason`
    - `[[skills.config]]` entries keyed by `path`, with `enabled = false`
    used for persisted disables
    
    ## Suggested manual validation
    - Connect the TUI to a remote app server, disable an app, reconnect, and
    confirm the app remains disabled from remote config rather than local
    disk state.
    - Re-enable the same app and confirm both `apps.<app_id>.enabled` and
    `apps.<app_id>.disabled_reason` are cleared remotely.
    - Disable a skill in the manage-skills UI and confirm a remote
    `[[skills.config]]` disable entry appears.
    - Re-enable that skill and confirm the disable entry is removed and the
    effective enabled state updates without relying on local config reloads.
    
    ## Stack
    1. [#22913](https://github.com/openai/codex/pull/22913) `[1 of 4]`
    primary settings writes
    2. [#22914](https://github.com/openai/codex/pull/22914) `[2 of 4]` app
    and skill enablement
    3. [#22915](https://github.com/openai/codex/pull/22915) `[3 of 4]`
    feature and memory toggles
    4. [#22916](https://github.com/openai/codex/pull/22916) `[4 of 4]`
    startup and onboarding bookkeeping
  • [codex] Use shared app-server JSON-RPC error helpers (#21221)
    ## Why
    
    App-server had repeated hand-built JSON-RPC error objects for standard
    error shapes. Using the shared helpers keeps the common
    `invalid_request`, `invalid_params`, and `internal_error` construction
    in one place and reduces the chance of new call sites drifting from the
    common error payload shape.
    
    ## What changed
    
    - Replaced manual standard JSON-RPC error object creation with
    `internal_error(...)`, `invalid_request(...)`, and `invalid_params(...)`
    across app-server request processors and runtime paths.
    - Removed local duplicate helper definitions from search and review
    request handling.
    - Preserved existing structured `data` payloads by creating the shared
    helper error first and then attaching the existing metadata.
    - Left custom non-standard errors and raw error-code assertions intact.
    
    ## Validation
    
    - `cargo test -p codex-app-server`
  • [codex] Split app-server request processors (#20940)
    ## Why
    
    The app-server request path had grown around a large
    `CodexMessageProcessor` plus separate API wrapper/helper modules. That
    made the dependency graph hard to see and forced unrelated request
    families to share broad processor state.
    
    This PR makes the split mechanical and command-prefix oriented so
    request families own only the dependencies they use.
    
    ## What changed
    
    - Replaced `CodexMessageProcessor` with command-prefix request
    processors under `app-server/src/request_processors/`.
    - Removed the old config, device-key, external-agent-config, and fs API
    wrapper files by moving their API handling into processors.
    - Split apps, plugins, marketplace, catalog, account, MCP, command exec,
    fs, git, feedback, thread, turn, thread goals, and Windows sandbox
    handling into dedicated processors.
    - Kept shared lifecycle, summary conversion, token usage replay, and
    shared error mapping only where multiple processors use them; single-use
    helpers were inlined into their owning processor.
    - Removed the fallback processor path and moved processor tests to
    `_tests` files.
    
    ## Validation
    
    - `cargo test -p codex-app-server`
    - `cargo check -p codex-app-server`
    - `just fix -p codex-app-server`