diff --git a/.github/workflows/rust-ci.yml b/.github/workflows/rust-ci.yml index 7918629165..103e8cf6bd 100644 --- a/.github/workflows/rust-ci.yml +++ b/.github/workflows/rust-ci.yml @@ -9,7 +9,7 @@ on: # CI builds in debug (dev) for faster signal. jobs: - # --- Detect what changed (always runs) ------------------------------------- + # --- Detect what changed to detect which tests to run (always runs) ------------------------------------- changed: name: Detect changed areas runs-on: ubuntu-24.04 @@ -84,8 +84,8 @@ jobs: run: cargo shear # --- CI to validate on different os/targets -------------------------------- - lint_build_test: - name: ${{ matrix.runner }} - ${{ matrix.target }}${{ matrix.profile == 'release' && ' (release)' || '' }} + lint_build: + name: Lint/Build — ${{ matrix.runner }} - ${{ matrix.target }}${{ matrix.profile == 'release' && ' (release)' || '' }} runs-on: ${{ matrix.runner }} timeout-minutes: 30 needs: changed @@ -94,6 +94,11 @@ jobs: defaults: run: working-directory: codex-rs + env: + # Speed up repeated builds across CI runs by caching compiled objects. + RUSTC_WRAPPER: sccache + CARGO_INCREMENTAL: "0" + SCCACHE_CACHE_SIZE: 10G strategy: fail-fast: false @@ -159,20 +164,83 @@ jobs: ~/.cargo/registry/index/ ~/.cargo/registry/cache/ ~/.cargo/git/db/ - key: cargo-home-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ hashFiles('**/Cargo.lock') }} + key: cargo-home-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ hashFiles('**/Cargo.lock') }}-${{ hashFiles('codex-rs/rust-toolchain.toml') }} + restore-keys: | + cargo-home-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}- - - name: Restore target cache (except gnu-dev) - id: cache_target_restore - if: ${{ !(matrix.target == 'x86_64-unknown-linux-gnu' && matrix.profile != 'release') }} + # Install and restore sccache cache + - name: Install sccache + uses: taiki-e/install-action@0c5db7f7f897c03b771660e91d065338615679f4 # v2 + with: + tool: sccache + version: 0.7.5 + + - name: Configure sccache backend + shell: bash + run: | + set -euo pipefail + if [[ -n "${ACTIONS_CACHE_URL:-}" && -n "${ACTIONS_RUNTIME_TOKEN:-}" ]]; then + echo "SCCACHE_GHA_ENABLED=true" >> "$GITHUB_ENV" + echo "Using sccache GitHub backend" + else + echo "SCCACHE_GHA_ENABLED=false" >> "$GITHUB_ENV" + echo "SCCACHE_DIR=${{ github.workspace }}/.sccache" >> "$GITHUB_ENV" + echo "Using sccache local disk + actions/cache fallback" + fi + + - name: Restore sccache cache (fallback) + if: ${{ env.SCCACHE_GHA_ENABLED != 'true' }} + id: cache_sccache_restore uses: actions/cache/restore@v4 with: - path: ${{ github.workspace }}/codex-rs/target/ - key: cargo-target-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ hashFiles('**/Cargo.lock') }} + path: ${{ github.workspace }}/.sccache/ + key: sccache-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ hashFiles('**/Cargo.lock') }}-${{ github.run_id }} + restore-keys: | + sccache-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ hashFiles('**/Cargo.lock') }}- + sccache-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}- + + - if: ${{ matrix.target == 'x86_64-unknown-linux-musl' || matrix.target == 'aarch64-unknown-linux-musl'}} + name: Prepare APT cache directories (musl) + shell: bash + run: | + set -euo pipefail + sudo mkdir -p /var/cache/apt/archives /var/lib/apt/lists + sudo chown -R "$USER:$USER" /var/cache/apt /var/lib/apt/lists + + - if: ${{ matrix.target == 'x86_64-unknown-linux-musl' || matrix.target == 'aarch64-unknown-linux-musl'}} + name: Restore APT cache (musl) + id: cache_apt_restore + uses: actions/cache/restore@v4 + with: + path: | + /var/cache/apt + key: apt-${{ matrix.runner }}-${{ matrix.target }}-v1 - if: ${{ matrix.target == 'x86_64-unknown-linux-musl' || matrix.target == 'aarch64-unknown-linux-musl'}} name: Install musl build tools + env: + DEBIAN_FRONTEND: noninteractive + shell: bash run: | - sudo apt install -y musl-tools pkg-config && sudo rm -rf /var/lib/apt/lists/* + set -euo pipefail + sudo apt-get -y update -o Acquire::Retries=3 + sudo apt-get -y install --no-install-recommends musl-tools pkg-config + + - name: Install cargo-chef + if: ${{ matrix.profile == 'release' }} + uses: taiki-e/install-action@0c5db7f7f897c03b771660e91d065338615679f4 # v2 + with: + tool: cargo-chef + version: 0.1.71 + + - name: Pre-warm dependency cache (cargo-chef) + if: ${{ matrix.profile == 'release' }} + shell: bash + run: | + set -euo pipefail + RECIPE="${RUNNER_TEMP}/chef-recipe.json" + cargo chef prepare --recipe-path "$RECIPE" + cargo chef cook --recipe-path "$RECIPE" --target ${{ matrix.target }} --release --all-features - name: cargo clippy id: clippy @@ -191,20 +259,6 @@ jobs: find . -name Cargo.toml -mindepth 2 -maxdepth 2 -print0 \ | xargs -0 -n1 -I{} bash -c 'cd "$(dirname "{}")" && cargo check --profile ${{ matrix.profile }}' - - uses: taiki-e/install-action@0c5db7f7f897c03b771660e91d065338615679f4 # v2 - with: - tool: nextest - version: 0.9.103 - - - name: tests - id: test - # Tests take too long for release builds to run them on every PR. - if: ${{ matrix.profile != 'release' }} - continue-on-error: true - run: cargo nextest run --all-features --no-fail-fast --target ${{ matrix.target }} --cargo-profile ci-test - env: - RUST_BACKTRACE: 1 - # Save caches explicitly; make non-fatal so cache packaging # never fails the overall job. Only save when key wasn't hit. - name: Save cargo home cache @@ -217,33 +271,193 @@ jobs: ~/.cargo/registry/index/ ~/.cargo/registry/cache/ ~/.cargo/git/db/ - key: cargo-home-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ hashFiles('**/Cargo.lock') }} + key: cargo-home-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ hashFiles('**/Cargo.lock') }}-${{ hashFiles('codex-rs/rust-toolchain.toml') }} - - name: Save target cache (except gnu-dev) - if: >- - always() && !cancelled() && - (steps.cache_target_restore.outputs.cache-hit != 'true') && - !(matrix.target == 'x86_64-unknown-linux-gnu' && matrix.profile != 'release') + - name: Save sccache cache (fallback) + if: always() && !cancelled() && env.SCCACHE_GHA_ENABLED != 'true' continue-on-error: true uses: actions/cache/save@v4 with: - path: ${{ github.workspace }}/codex-rs/target/ - key: cargo-target-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ hashFiles('**/Cargo.lock') }} + path: ${{ github.workspace }}/.sccache/ + key: sccache-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ hashFiles('**/Cargo.lock') }}-${{ github.run_id }} + + - name: sccache stats + if: always() + continue-on-error: true + run: sccache --show-stats || true + + - name: sccache summary + if: always() + shell: bash + run: | + { + echo "### sccache stats — ${{ matrix.target }} (${{ matrix.profile }})"; + echo; + echo '```'; + sccache --show-stats || true; + echo '```'; + } >> "$GITHUB_STEP_SUMMARY" + + - name: Save APT cache (musl) + if: always() && !cancelled() && (matrix.target == 'x86_64-unknown-linux-musl' || matrix.target == 'aarch64-unknown-linux-musl') && steps.cache_apt_restore.outputs.cache-hit != 'true' + continue-on-error: true + uses: actions/cache/save@v4 + with: + path: | + /var/cache/apt + key: apt-${{ matrix.runner }}-${{ matrix.target }}-v1 # Fail the job if any of the previous steps failed. - name: verify all steps passed if: | steps.clippy.outcome == 'failure' || - steps.cargo_check_all_crates.outcome == 'failure' || - steps.test.outcome == 'failure' + steps.cargo_check_all_crates.outcome == 'failure' run: | - echo "One or more checks failed (clippy, cargo_check_all_crates, or test). See logs for details." + echo "One or more checks failed (clippy or cargo_check_all_crates). See logs for details." + exit 1 + + tests: + name: Tests — ${{ matrix.runner }} - ${{ matrix.target }} + runs-on: ${{ matrix.runner }} + timeout-minutes: 30 + needs: changed + if: ${{ needs.changed.outputs.codex == 'true' || needs.changed.outputs.workflows == 'true' || github.event_name == 'push' }} + defaults: + run: + working-directory: codex-rs + env: + RUSTC_WRAPPER: sccache + CARGO_INCREMENTAL: "0" + SCCACHE_CACHE_SIZE: 10G + + strategy: + fail-fast: false + matrix: + include: + - runner: macos-14 + target: aarch64-apple-darwin + profile: dev + - runner: ubuntu-24.04 + target: x86_64-unknown-linux-gnu + profile: dev + - runner: ubuntu-24.04-arm + target: aarch64-unknown-linux-gnu + profile: dev + - runner: windows-latest + target: x86_64-pc-windows-msvc + profile: dev + - runner: windows-11-arm + target: aarch64-pc-windows-msvc + profile: dev + + steps: + - uses: actions/checkout@v5 + - uses: dtolnay/rust-toolchain@1.90 + with: + targets: ${{ matrix.target }} + + - name: Restore cargo home cache + id: cache_cargo_home_restore + uses: actions/cache/restore@v4 + with: + path: | + ~/.cargo/bin/ + ~/.cargo/registry/index/ + ~/.cargo/registry/cache/ + ~/.cargo/git/db/ + key: cargo-home-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ hashFiles('**/Cargo.lock') }}-${{ hashFiles('codex-rs/rust-toolchain.toml') }} + restore-keys: | + cargo-home-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}- + + - name: Install sccache + uses: taiki-e/install-action@0c5db7f7f897c03b771660e91d065338615679f4 # v2 + with: + tool: sccache + version: 0.7.5 + + - name: Configure sccache backend + shell: bash + run: | + set -euo pipefail + if [[ -n "${ACTIONS_CACHE_URL:-}" && -n "${ACTIONS_RUNTIME_TOKEN:-}" ]]; then + echo "SCCACHE_GHA_ENABLED=true" >> "$GITHUB_ENV" + echo "Using sccache GitHub backend" + else + echo "SCCACHE_GHA_ENABLED=false" >> "$GITHUB_ENV" + echo "SCCACHE_DIR=${{ github.workspace }}/.sccache" >> "$GITHUB_ENV" + echo "Using sccache local disk + actions/cache fallback" + fi + + - name: Restore sccache cache (fallback) + if: ${{ env.SCCACHE_GHA_ENABLED != 'true' }} + id: cache_sccache_restore + uses: actions/cache/restore@v4 + with: + path: ${{ github.workspace }}/.sccache/ + key: sccache-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ hashFiles('**/Cargo.lock') }}-${{ github.run_id }} + restore-keys: | + sccache-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ hashFiles('**/Cargo.lock') }}- + sccache-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}- + + - uses: taiki-e/install-action@0c5db7f7f897c03b771660e91d065338615679f4 # v2 + with: + tool: nextest + version: 0.9.103 + + - name: tests + id: test + continue-on-error: true + run: cargo nextest run --all-features --no-fail-fast --target ${{ matrix.target }} --cargo-profile ci-test + env: + RUST_BACKTRACE: 1 + + - name: Save cargo home cache + if: always() && !cancelled() && steps.cache_cargo_home_restore.outputs.cache-hit != 'true' + continue-on-error: true + uses: actions/cache/save@v4 + with: + path: | + ~/.cargo/bin/ + ~/.cargo/registry/index/ + ~/.cargo/registry/cache/ + ~/.cargo/git/db/ + key: cargo-home-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ hashFiles('**/Cargo.lock') }}-${{ hashFiles('codex-rs/rust-toolchain.toml') }} + + - name: Save sccache cache (fallback) + if: always() && !cancelled() && env.SCCACHE_GHA_ENABLED != 'true' + continue-on-error: true + uses: actions/cache/save@v4 + with: + path: ${{ github.workspace }}/.sccache/ + key: sccache-${{ matrix.runner }}-${{ matrix.target }}-${{ matrix.profile }}-${{ hashFiles('**/Cargo.lock') }}-${{ github.run_id }} + + - name: sccache stats + if: always() + continue-on-error: true + run: sccache --show-stats || true + + - name: sccache summary + if: always() + shell: bash + run: | + { + echo "### sccache stats — ${{ matrix.target }} (tests)"; + echo; + echo '```'; + sccache --show-stats || true; + echo '```'; + } >> "$GITHUB_STEP_SUMMARY" + + - name: verify tests passed + if: steps.test.outcome == 'failure' + run: | + echo "Tests failed. See logs for details." exit 1 # --- Gatherer job that you mark as the ONLY required status ----------------- results: name: CI results (required) - needs: [changed, general, cargo_shear, lint_build_test] + needs: [changed, general, cargo_shear, lint_build, tests] if: always() runs-on: ubuntu-24.04 steps: @@ -252,7 +466,8 @@ jobs: run: | echo "general: ${{ needs.general.result }}" echo "shear : ${{ needs.cargo_shear.result }}" - echo "matrix : ${{ needs.lint_build_test.result }}" + echo "lint : ${{ needs.lint_build.result }}" + echo "tests : ${{ needs.tests.result }}" # If nothing relevant changed (PR touching only root README, etc.), # declare success regardless of other jobs. @@ -264,4 +479,10 @@ jobs: # Otherwise require the jobs to have succeeded [[ '${{ needs.general.result }}' == 'success' ]] || { echo 'general failed'; exit 1; } [[ '${{ needs.cargo_shear.result }}' == 'success' ]] || { echo 'cargo_shear failed'; exit 1; } - [[ '${{ needs.lint_build_test.result }}' == 'success' ]] || { echo 'matrix failed'; exit 1; } + [[ '${{ needs.lint_build.result }}' == 'success' ]] || { echo 'lint_build failed'; exit 1; } + [[ '${{ needs.tests.result }}' == 'success' ]] || { echo 'tests failed'; exit 1; } + + - name: sccache summary note + if: always() + run: | + echo "Per-job sccache stats are attached to each matrix job's Step Summary." diff --git a/.github/workflows/rust-release.yml b/.github/workflows/rust-release.yml index 5beadd55c4..fa3e22894e 100644 --- a/.github/workflows/rust-release.yml +++ b/.github/workflows/rust-release.yml @@ -53,6 +53,10 @@ jobs: defaults: run: working-directory: codex-rs + env: + RUSTC_WRAPPER: sccache + CARGO_INCREMENTAL: "0" + SCCACHE_CACHE_SIZE: 10G strategy: fail-fast: false @@ -88,14 +92,80 @@ jobs: ~/.cargo/registry/index/ ~/.cargo/registry/cache/ ~/.cargo/git/db/ - ${{ github.workspace }}/codex-rs/target/ - key: cargo-${{ matrix.runner }}-${{ matrix.target }}-release-${{ hashFiles('**/Cargo.lock') }} + key: cargo-${{ matrix.runner }}-${{ matrix.target }}-release-${{ hashFiles('**/Cargo.lock') }}-${{ hashFiles('codex-rs/rust-toolchain.toml') }} + restore-keys: | + cargo-${{ matrix.runner }}-${{ matrix.target }}-release- + + - name: Install sccache + uses: taiki-e/install-action@0c5db7f7f897c03b771660e91d065338615679f4 # v2 + with: + tool: sccache + version: 0.7.5 + + - name: Configure sccache backend + shell: bash + run: | + set -euo pipefail + if [[ -n "${ACTIONS_CACHE_URL:-}" && -n "${ACTIONS_RUNTIME_TOKEN:-}" ]]; then + echo "SCCACHE_GHA_ENABLED=true" >> "$GITHUB_ENV" + echo "Using sccache GitHub backend" + else + echo "SCCACHE_GHA_ENABLED=false" >> "$GITHUB_ENV" + echo "SCCACHE_DIR=${{ github.workspace }}/.sccache" >> "$GITHUB_ENV" + echo "Using sccache local disk + actions/cache fallback" + fi + + - name: Restore sccache cache (fallback) + if: ${{ env.SCCACHE_GHA_ENABLED != 'true' }} + id: cache_sccache_restore + uses: actions/cache/restore@v4 + with: + path: ${{ github.workspace }}/.sccache/ + key: sccache-${{ matrix.runner }}-${{ matrix.target }}-release-${{ hashFiles('**/Cargo.lock') }}-${{ github.run_id }} + restore-keys: | + sccache-${{ matrix.runner }}-${{ matrix.target }}-release-${{ hashFiles('**/Cargo.lock') }}- + sccache-${{ matrix.runner }}-${{ matrix.target }}-release- + + - if: ${{ matrix.target == 'x86_64-unknown-linux-musl' || matrix.target == 'aarch64-unknown-linux-musl'}} + name: Prepare APT cache directories (musl) + shell: bash + run: | + set -euo pipefail + sudo mkdir -p /var/cache/apt/archives /var/lib/apt/lists + sudo chown -R "$USER:$USER" /var/cache/apt /var/lib/apt/lists + + - if: ${{ matrix.target == 'x86_64-unknown-linux-musl' || matrix.target == 'aarch64-unknown-linux-musl'}} + name: Restore APT cache (musl) + id: cache_apt_restore + uses: actions/cache/restore@v4 + with: + path: | + /var/cache/apt + key: apt-${{ matrix.runner }}-${{ matrix.target }}-v1 - if: ${{ matrix.target == 'x86_64-unknown-linux-musl' || matrix.target == 'aarch64-unknown-linux-musl'}} name: Install musl build tools + env: + DEBIAN_FRONTEND: noninteractive + shell: bash run: | - sudo apt-get update - sudo apt-get install -y musl-tools pkg-config + set -euo pipefail + sudo apt-get -y update -o Acquire::Retries=3 + sudo apt-get -y install --no-install-recommends musl-tools pkg-config + + - name: Install cargo-chef + uses: taiki-e/install-action@0c5db7f7f897c03b771660e91d065338615679f4 # v2 + with: + tool: cargo-chef + version: 0.1.71 + + - name: Pre-warm dependency cache (cargo-chef) + shell: bash + run: | + set -euo pipefail + RECIPE="${RUNNER_TEMP}/chef-recipe.json" + cargo chef prepare --recipe-path "$RECIPE" + cargo chef cook --recipe-path "$RECIPE" --target ${{ matrix.target }} --release --all-features - name: Cargo build run: cargo build --target ${{ matrix.target }} --release --bin codex --bin codex-responses-api-proxy @@ -327,6 +397,40 @@ jobs: zstd -T0 -19 --rm "$dest/$base" done + - name: Save APT cache (musl) + if: always() && !cancelled() && (matrix.target == 'x86_64-unknown-linux-musl' || matrix.target == 'aarch64-unknown-linux-musl') && steps.cache_apt_restore.outputs.cache-hit != 'true' + continue-on-error: true + uses: actions/cache/save@v4 + with: + path: | + /var/cache/apt + key: apt-${{ matrix.runner }}-${{ matrix.target }}-v1 + + - name: Save sccache cache (fallback) + if: always() && !cancelled() && env.SCCACHE_GHA_ENABLED != 'true' + continue-on-error: true + uses: actions/cache/save@v4 + with: + path: ${{ github.workspace }}/.sccache/ + key: sccache-${{ matrix.runner }}-${{ matrix.target }}-release-${{ hashFiles('**/Cargo.lock') }}-${{ github.run_id }} + + - name: sccache stats + if: always() + continue-on-error: true + run: sccache --show-stats || true + + - name: sccache summary + if: always() + shell: bash + run: | + { + echo "### sccache stats — ${{ matrix.target }} (release)"; + echo; + echo '```'; + sccache --show-stats || true; + echo '```'; + } >> "$GITHUB_STEP_SUMMARY" + - name: Remove signing keychain if: ${{ always() && matrix.runner == 'macos-15-xlarge' }} shell: bash diff --git a/codex-rs/app-server-protocol/src/protocol.rs b/codex-rs/app-server-protocol/src/protocol.rs index f164ce5601..65a233a376 100644 --- a/codex-rs/app-server-protocol/src/protocol.rs +++ b/codex-rs/app-server-protocol/src/protocol.rs @@ -5,7 +5,7 @@ use crate::JSONRPCNotification; use crate::JSONRPCRequest; use crate::RequestId; use codex_protocol::ConversationId; -use codex_protocol::account::Account; +use codex_protocol::account::PlanType; use codex_protocol::config_types::ForcedLoginMethod; use codex_protocol::config_types::ReasoningEffort; use codex_protocol::config_types::ReasoningSummary; @@ -236,6 +236,22 @@ client_request_definitions! { }, } +#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema, TS)] +#[serde(tag = "type", rename_all = "camelCase")] +#[ts(tag = "type")] +pub enum Account { + #[serde(rename = "apiKey", rename_all = "camelCase")] + #[ts(rename = "apiKey", rename_all = "camelCase")] + ApiKey { api_key: String }, + + #[serde(rename = "chatgpt", rename_all = "camelCase")] + #[ts(rename = "chatgpt", rename_all = "camelCase")] + ChatGpt { + email: Option, + plan_type: PlanType, + }, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] pub struct GetAccountResponse { @@ -1246,6 +1262,35 @@ mod tests { Ok(()) } + #[test] + fn account_serializes_fields_in_camel_case() -> Result<()> { + let api_key = Account::ApiKey { + api_key: "secret".to_string(), + }; + assert_eq!( + json!({ + "type": "apiKey", + "apiKey": "secret", + }), + serde_json::to_value(&api_key)?, + ); + + let chatgpt = Account::ChatGpt { + email: Some("user@example.com".to_string()), + plan_type: PlanType::Plus, + }; + assert_eq!( + json!({ + "type": "chatgpt", + "email": "user@example.com", + "planType": "plus", + }), + serde_json::to_value(&chatgpt)?, + ); + + Ok(()) + } + #[test] fn serialize_list_models() -> Result<()> { let request = ClientRequest::ListModels { diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index fc79d20968..4f8c066f7e 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -323,7 +323,11 @@ impl CodexMessageProcessor { } } - match login_with_api_key(&self.config.codex_home, ¶ms.api_key) { + match login_with_api_key( + &self.config.codex_home, + ¶ms.api_key, + self.config.cli_auth_credentials_store_mode, + ) { Ok(()) => { self.auth_manager.reload(); self.outgoing @@ -367,6 +371,7 @@ impl CodexMessageProcessor { config.codex_home.clone(), CLIENT_ID.to_string(), config.forced_chatgpt_workspace_id.clone(), + config.cli_auth_credentials_store_mode, ) }; diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index a2c192cf2a..7693cc2ff9 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -37,7 +37,11 @@ impl MessageProcessor { feedback: CodexFeedback, ) -> Self { let outgoing = Arc::new(outgoing); - let auth_manager = AuthManager::shared(config.codex_home.clone(), false); + let auth_manager = AuthManager::shared( + config.codex_home.clone(), + false, + config.cli_auth_credentials_store_mode, + ); let conversation_manager = Arc::new(ConversationManager::new( auth_manager.clone(), SessionSource::VSCode, diff --git a/codex-rs/app-server/tests/common/auth_fixtures.rs b/codex-rs/app-server/tests/common/auth_fixtures.rs index 594b621d31..071a920b89 100644 --- a/codex-rs/app-server/tests/common/auth_fixtures.rs +++ b/codex-rs/app-server/tests/common/auth_fixtures.rs @@ -6,6 +6,7 @@ use base64::Engine; use base64::engine::general_purpose::URL_SAFE_NO_PAD; use chrono::DateTime; use chrono::Utc; +use codex_core::auth::AuthCredentialsStoreMode; use codex_core::auth::AuthDotJson; use codex_core::auth::save_auth; use codex_core::token_data::TokenData; @@ -108,7 +109,11 @@ pub fn encode_id_token(claims: &ChatGptIdTokenClaims) -> Result { Ok(format!("{header_b64}.{payload_b64}.{signature_b64}")) } -pub fn write_chatgpt_auth(codex_home: &Path, fixture: ChatGptAuthFixture) -> Result<()> { +pub fn write_chatgpt_auth( + codex_home: &Path, + fixture: ChatGptAuthFixture, + cli_auth_credentials_store_mode: AuthCredentialsStoreMode, +) -> Result<()> { let id_token_raw = encode_id_token(&fixture.claims)?; let id_token = parse_id_token(&id_token_raw).context("parse id token")?; let tokens = TokenData { @@ -126,5 +131,5 @@ pub fn write_chatgpt_auth(codex_home: &Path, fixture: ChatGptAuthFixture) -> Res last_refresh, }; - save_auth(codex_home, &auth).context("write auth.json") + save_auth(codex_home, &auth, cli_auth_credentials_store_mode).context("write auth.json") } diff --git a/codex-rs/app-server/tests/suite/archive_conversation.rs b/codex-rs/app-server/tests/suite/archive_conversation.rs index 6dcfefdbb8..2d8f746a69 100644 --- a/codex-rs/app-server/tests/suite/archive_conversation.rs +++ b/codex-rs/app-server/tests/suite/archive_conversation.rs @@ -1,5 +1,4 @@ -use std::path::Path; - +use anyhow::Result; use app_test_support::McpProcess; use app_test_support::to_response; use codex_app_server_protocol::ArchiveConversationParams; @@ -9,45 +8,37 @@ use codex_app_server_protocol::NewConversationParams; use codex_app_server_protocol::NewConversationResponse; use codex_app_server_protocol::RequestId; use codex_core::ARCHIVED_SESSIONS_SUBDIR; +use std::path::Path; use tempfile::TempDir; use tokio::time::timeout; const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn archive_conversation_moves_rollout_into_archived_directory() { - let codex_home = TempDir::new().expect("create temp dir"); - create_config_toml(codex_home.path()).expect("write config.toml"); +async fn archive_conversation_moves_rollout_into_archived_directory() -> Result<()> { + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path())?; - let mut mcp = McpProcess::new(codex_home.path()) - .await - .expect("spawn mcp process"); - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .expect("initialize timeout") - .expect("initialize request"); + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; let new_request_id = mcp .send_new_conversation_request(NewConversationParams { model: Some("mock-model".to_string()), ..Default::default() }) - .await - .expect("send newConversation"); + .await?; let new_response: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(new_request_id)), ) - .await - .expect("newConversation timeout") - .expect("newConversation response"); + .await??; let NewConversationResponse { conversation_id, rollout_path, .. - } = to_response::(new_response) - .expect("deserialize newConversation response"); + } = to_response::(new_response)?; assert!( rollout_path.exists(), @@ -60,19 +51,15 @@ async fn archive_conversation_moves_rollout_into_archived_directory() { conversation_id, rollout_path: rollout_path.clone(), }) - .await - .expect("send archiveConversation"); + .await?; let archive_response: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(archive_request_id)), ) - .await - .expect("archiveConversation timeout") - .expect("archiveConversation response"); + .await??; let _: ArchiveConversationResponse = - to_response::(archive_response) - .expect("deserialize archiveConversation response"); + to_response::(archive_response)?; let archived_directory = codex_home.path().join(ARCHIVED_SESSIONS_SUBDIR); let archived_rollout_path = @@ -90,6 +77,8 @@ async fn archive_conversation_moves_rollout_into_archived_directory() { "expected archived rollout path {} to exist", archived_rollout_path.display() ); + + Ok(()) } fn create_config_toml(codex_home: &Path) -> std::io::Result<()> { diff --git a/codex-rs/app-server/tests/suite/auth.rs b/codex-rs/app-server/tests/suite/auth.rs index aa6cc1bb7d..72912362fe 100644 --- a/codex-rs/app-server/tests/suite/auth.rs +++ b/codex-rs/app-server/tests/suite/auth.rs @@ -1,5 +1,4 @@ -use std::path::Path; - +use anyhow::Result; use app_test_support::McpProcess; use app_test_support::to_response; use codex_app_server_protocol::AuthMode; @@ -11,6 +10,7 @@ use codex_app_server_protocol::LoginApiKeyParams; use codex_app_server_protocol::LoginApiKeyResponse; use codex_app_server_protocol::RequestId; use pretty_assertions::assert_eq; +use std::path::Path; use tempfile::TempDir; use tokio::time::timeout; @@ -71,125 +71,99 @@ forced_login_method = "{forced_method}" std::fs::write(config_toml, contents) } -async fn login_with_api_key_via_request(mcp: &mut McpProcess, api_key: &str) { +async fn login_with_api_key_via_request(mcp: &mut McpProcess, api_key: &str) -> Result<()> { let request_id = mcp .send_login_api_key_request(LoginApiKeyParams { api_key: api_key.to_string(), }) - .await - .unwrap_or_else(|e| panic!("send loginApiKey: {e}")); + .await?; let resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(request_id)), ) - .await - .unwrap_or_else(|e| panic!("loginApiKey timeout: {e}")) - .unwrap_or_else(|e| panic!("loginApiKey response: {e}")); - let _: LoginApiKeyResponse = - to_response(resp).unwrap_or_else(|e| panic!("deserialize login response: {e}")); + .await??; + let _: LoginApiKeyResponse = to_response(resp)?; + Ok(()) } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn get_auth_status_no_auth() { - let codex_home = TempDir::new().unwrap_or_else(|e| panic!("create tempdir: {e}")); - create_config_toml(codex_home.path()).unwrap_or_else(|err| panic!("write config.toml: {err}")); +async fn get_auth_status_no_auth() -> Result<()> { + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path())?; - let mut mcp = McpProcess::new_with_env(codex_home.path(), &[("OPENAI_API_KEY", None)]) - .await - .expect("spawn mcp process"); - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .expect("init timeout") - .expect("init failed"); + let mut mcp = McpProcess::new_with_env(codex_home.path(), &[("OPENAI_API_KEY", None)]).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; let request_id = mcp .send_get_auth_status_request(GetAuthStatusParams { include_token: Some(true), refresh_token: Some(false), }) - .await - .expect("send getAuthStatus"); + .await?; let resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(request_id)), ) - .await - .expect("getAuthStatus timeout") - .expect("getAuthStatus response"); - let status: GetAuthStatusResponse = to_response(resp).expect("deserialize status"); + .await??; + let status: GetAuthStatusResponse = to_response(resp)?; assert_eq!(status.auth_method, None, "expected no auth method"); assert_eq!(status.auth_token, None, "expected no token"); + Ok(()) } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn get_auth_status_with_api_key() { - let codex_home = TempDir::new().unwrap_or_else(|e| panic!("create tempdir: {e}")); - create_config_toml(codex_home.path()).unwrap_or_else(|err| panic!("write config.toml: {err}")); +async fn get_auth_status_with_api_key() -> Result<()> { + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path())?; - let mut mcp = McpProcess::new(codex_home.path()) - .await - .expect("spawn mcp process"); - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .expect("init timeout") - .expect("init failed"); + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; - login_with_api_key_via_request(&mut mcp, "sk-test-key").await; + login_with_api_key_via_request(&mut mcp, "sk-test-key").await?; let request_id = mcp .send_get_auth_status_request(GetAuthStatusParams { include_token: Some(true), refresh_token: Some(false), }) - .await - .expect("send getAuthStatus"); + .await?; let resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(request_id)), ) - .await - .expect("getAuthStatus timeout") - .expect("getAuthStatus response"); - let status: GetAuthStatusResponse = to_response(resp).expect("deserialize status"); + .await??; + let status: GetAuthStatusResponse = to_response(resp)?; assert_eq!(status.auth_method, Some(AuthMode::ApiKey)); assert_eq!(status.auth_token, Some("sk-test-key".to_string())); + Ok(()) } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn get_auth_status_with_api_key_when_auth_not_required() { - let codex_home = TempDir::new().unwrap_or_else(|e| panic!("create tempdir: {e}")); - create_config_toml_custom_provider(codex_home.path(), false) - .unwrap_or_else(|err| panic!("write config.toml: {err}")); +async fn get_auth_status_with_api_key_when_auth_not_required() -> Result<()> { + let codex_home = TempDir::new()?; + create_config_toml_custom_provider(codex_home.path(), false)?; - let mut mcp = McpProcess::new(codex_home.path()) - .await - .expect("spawn mcp process"); - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .expect("init timeout") - .expect("init failed"); + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; - login_with_api_key_via_request(&mut mcp, "sk-test-key").await; + login_with_api_key_via_request(&mut mcp, "sk-test-key").await?; let request_id = mcp .send_get_auth_status_request(GetAuthStatusParams { include_token: Some(true), refresh_token: Some(false), }) - .await - .expect("send getAuthStatus"); + .await?; let resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(request_id)), ) - .await - .expect("getAuthStatus timeout") - .expect("getAuthStatus response"); - let status: GetAuthStatusResponse = to_response(resp).expect("deserialize status"); + .await??; + let status: GetAuthStatusResponse = to_response(resp)?; assert_eq!(status.auth_method, None, "expected no auth method"); assert_eq!(status.auth_token, None, "expected no token"); assert_eq!( @@ -197,76 +171,60 @@ async fn get_auth_status_with_api_key_when_auth_not_required() { Some(false), "requires_openai_auth should be false", ); + Ok(()) } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn get_auth_status_with_api_key_no_include_token() { - let codex_home = TempDir::new().unwrap_or_else(|e| panic!("create tempdir: {e}")); - create_config_toml(codex_home.path()).unwrap_or_else(|err| panic!("write config.toml: {err}")); +async fn get_auth_status_with_api_key_no_include_token() -> Result<()> { + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path())?; - let mut mcp = McpProcess::new(codex_home.path()) - .await - .expect("spawn mcp process"); - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .expect("init timeout") - .expect("init failed"); + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; - login_with_api_key_via_request(&mut mcp, "sk-test-key").await; + login_with_api_key_via_request(&mut mcp, "sk-test-key").await?; // Build params via struct so None field is omitted in wire JSON. let params = GetAuthStatusParams { include_token: None, refresh_token: Some(false), }; - let request_id = mcp - .send_get_auth_status_request(params) - .await - .expect("send getAuthStatus"); + let request_id = mcp.send_get_auth_status_request(params).await?; let resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(request_id)), ) - .await - .expect("getAuthStatus timeout") - .expect("getAuthStatus response"); - let status: GetAuthStatusResponse = to_response(resp).expect("deserialize status"); + .await??; + let status: GetAuthStatusResponse = to_response(resp)?; assert_eq!(status.auth_method, Some(AuthMode::ApiKey)); assert!(status.auth_token.is_none(), "token must be omitted"); + Ok(()) } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn login_api_key_rejected_when_forced_chatgpt() { - let codex_home = TempDir::new().unwrap_or_else(|e| panic!("create tempdir: {e}")); - create_config_toml_forced_login(codex_home.path(), "chatgpt") - .unwrap_or_else(|err| panic!("write config.toml: {err}")); +async fn login_api_key_rejected_when_forced_chatgpt() -> Result<()> { + let codex_home = TempDir::new()?; + create_config_toml_forced_login(codex_home.path(), "chatgpt")?; - let mut mcp = McpProcess::new(codex_home.path()) - .await - .expect("spawn mcp process"); - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .expect("init timeout") - .expect("init failed"); + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; let request_id = mcp .send_login_api_key_request(LoginApiKeyParams { api_key: "sk-test-key".to_string(), }) - .await - .expect("send loginApiKey"); + .await?; let err: JSONRPCError = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_error_message(RequestId::Integer(request_id)), ) - .await - .expect("loginApiKey error timeout") - .expect("loginApiKey error"); + .await??; assert_eq!( err.error.message, "API key login is disabled. Use ChatGPT login instead." ); + Ok(()) } diff --git a/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs b/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs index 5c010bf034..1feda42841 100644 --- a/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs +++ b/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs @@ -1,5 +1,4 @@ -use std::path::Path; - +use anyhow::Result; use app_test_support::McpProcess; use app_test_support::create_final_assistant_message_sse_response; use app_test_support::create_mock_chat_completions_server; @@ -32,26 +31,27 @@ use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; use pretty_assertions::assert_eq; use std::env; +use std::path::Path; use tempfile::TempDir; use tokio::time::timeout; const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); #[tokio::test(flavor = "multi_thread", worker_threads = 4)] -async fn test_codex_jsonrpc_conversation_flow() { +async fn test_codex_jsonrpc_conversation_flow() -> Result<()> { if env::var(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() { println!( "Skipping test because it cannot execute when network is disabled in a Codex sandbox." ); - return; + return Ok(()); } - let tmp = TempDir::new().expect("tmp dir"); + let tmp = TempDir::new()?; // Temporary Codex home with config pointing at the mock server. let codex_home = tmp.path().join("codex_home"); - std::fs::create_dir(&codex_home).expect("create codex home dir"); + std::fs::create_dir(&codex_home)?; let working_directory = tmp.path().join("workdir"); - std::fs::create_dir(&working_directory).expect("create working directory"); + std::fs::create_dir(&working_directory)?; // Create a mock model server that immediately ends each turn. // Two turns are expected: initial session configure + one user message. @@ -61,20 +61,15 @@ async fn test_codex_jsonrpc_conversation_flow() { Some(&working_directory), Some(5000), "call1234", - ) - .expect("create shell sse response"), - create_final_assistant_message_sse_response("Enjoy your new git repo!") - .expect("create final assistant message"), + )?, + create_final_assistant_message_sse_response("Enjoy your new git repo!")?, ]; let server = create_mock_chat_completions_server(responses).await; - create_config_toml(&codex_home, &server.uri()).expect("write config"); + create_config_toml(&codex_home, &server.uri())?; // Start MCP server and initialize. - let mut mcp = McpProcess::new(&codex_home).await.expect("spawn mcp"); - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .expect("init timeout") - .expect("init error"); + let mut mcp = McpProcess::new(&codex_home).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; // 1) newConversation let new_conv_id = mcp @@ -82,17 +77,13 @@ async fn test_codex_jsonrpc_conversation_flow() { cwd: Some(working_directory.to_string_lossy().into_owned()), ..Default::default() }) - .await - .expect("send newConversation"); + .await?; let new_conv_resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(new_conv_id)), ) - .await - .expect("newConversation timeout") - .expect("newConversation resp"); - let new_conv_resp = to_response::(new_conv_resp) - .expect("deserialize newConversation response"); + .await??; + let new_conv_resp = to_response::(new_conv_resp)?; let NewConversationResponse { conversation_id, model, @@ -107,18 +98,14 @@ async fn test_codex_jsonrpc_conversation_flow() { conversation_id, experimental_raw_events: false, }) - .await - .expect("send addConversationListener"); + .await?; let add_listener_resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(add_listener_id)), ) - .await - .expect("addConversationListener timeout") - .expect("addConversationListener resp"); + .await??; let AddConversationSubscriptionResponse { subscription_id } = - to_response::(add_listener_resp) - .expect("deserialize addConversationListener response"); + to_response::(add_listener_resp)?; // 3) sendUserMessage (should trigger notifications; we only validate an OK response) let send_user_id = mcp @@ -128,17 +115,13 @@ async fn test_codex_jsonrpc_conversation_flow() { text: "text".to_string(), }], }) - .await - .expect("send sendUserMessage"); + .await?; let send_user_resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(send_user_id)), ) - .await - .expect("sendUserMessage timeout") - .expect("sendUserMessage resp"); - let SendUserMessageResponse {} = to_response::(send_user_resp) - .expect("deserialize sendUserMessage response"); + .await??; + let SendUserMessageResponse {} = to_response::(send_user_resp)?; // Verify the task_finished notification is received. // Note this also ensures that the final request to the server was made. @@ -146,9 +129,7 @@ async fn test_codex_jsonrpc_conversation_flow() { DEFAULT_READ_TIMEOUT, mcp.read_stream_until_notification_message("codex/event/task_complete"), ) - .await - .expect("task_finished_notification timeout") - .expect("task_finished_notification resp"); + .await??; let serde_json::Value::Object(map) = task_finished_notification .params .expect("notification should have params") @@ -166,33 +147,31 @@ async fn test_codex_jsonrpc_conversation_flow() { .send_remove_conversation_listener_request(RemoveConversationListenerParams { subscription_id, }) - .await - .expect("send removeConversationListener"); + .await?; let remove_listener_resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(remove_listener_id)), ) - .await - .expect("removeConversationListener timeout") - .expect("removeConversationListener resp"); - let RemoveConversationSubscriptionResponse {} = - to_response(remove_listener_resp).expect("deserialize removeConversationListener response"); + .await??; + let RemoveConversationSubscriptionResponse {} = to_response(remove_listener_resp)?; + + Ok(()) } #[tokio::test(flavor = "multi_thread", worker_threads = 4)] -async fn test_send_user_turn_changes_approval_policy_behavior() { +async fn test_send_user_turn_changes_approval_policy_behavior() -> Result<()> { if env::var(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() { println!( "Skipping test because it cannot execute when network is disabled in a Codex sandbox." ); - return; + return Ok(()); } - let tmp = TempDir::new().expect("tmp dir"); + let tmp = TempDir::new()?; let codex_home = tmp.path().join("codex_home"); - std::fs::create_dir(&codex_home).expect("create codex home dir"); + std::fs::create_dir(&codex_home)?; let working_directory = tmp.path().join("workdir"); - std::fs::create_dir(&working_directory).expect("create working directory"); + std::fs::create_dir(&working_directory)?; // Mock server will request a python shell call for the first and second turn, then finish. let responses = vec![ @@ -205,10 +184,8 @@ async fn test_send_user_turn_changes_approval_policy_behavior() { Some(&working_directory), Some(5000), "call1", - ) - .expect("create first shell sse response"), - create_final_assistant_message_sse_response("done 1") - .expect("create final assistant message 1"), + )?, + create_final_assistant_message_sse_response("done 1")?, create_shell_sse_response( vec![ "python3".to_string(), @@ -218,20 +195,15 @@ async fn test_send_user_turn_changes_approval_policy_behavior() { Some(&working_directory), Some(5000), "call2", - ) - .expect("create second shell sse response"), - create_final_assistant_message_sse_response("done 2") - .expect("create final assistant message 2"), + )?, + create_final_assistant_message_sse_response("done 2")?, ]; let server = create_mock_chat_completions_server(responses).await; - create_config_toml(&codex_home, &server.uri()).expect("write config"); + create_config_toml(&codex_home, &server.uri())?; // Start MCP server and initialize. - let mut mcp = McpProcess::new(&codex_home).await.expect("spawn mcp"); - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .expect("init timeout") - .expect("init error"); + let mut mcp = McpProcess::new(&codex_home).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; // 1) Start conversation with approval_policy=untrusted let new_conv_id = mcp @@ -239,19 +211,15 @@ async fn test_send_user_turn_changes_approval_policy_behavior() { cwd: Some(working_directory.to_string_lossy().into_owned()), ..Default::default() }) - .await - .expect("send newConversation"); + .await?; let new_conv_resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(new_conv_id)), ) - .await - .expect("newConversation timeout") - .expect("newConversation resp"); + .await??; let NewConversationResponse { conversation_id, .. - } = to_response::(new_conv_resp) - .expect("deserialize newConversation response"); + } = to_response::(new_conv_resp)?; // 2) addConversationListener let add_listener_id = mcp @@ -259,19 +227,14 @@ async fn test_send_user_turn_changes_approval_policy_behavior() { conversation_id, experimental_raw_events: false, }) - .await - .expect("send addConversationListener"); - let _: AddConversationSubscriptionResponse = - to_response::( - timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_response_message(RequestId::Integer(add_listener_id)), - ) - .await - .expect("addConversationListener timeout") - .expect("addConversationListener resp"), + .await?; + let _: AddConversationSubscriptionResponse = to_response::( + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(add_listener_id)), ) - .expect("deserialize addConversationListener response"); + .await??, + )?; // 3) sendUserMessage triggers a shell call; approval policy is Untrusted so we should get an elicitation let send_user_id = mcp @@ -281,27 +244,21 @@ async fn test_send_user_turn_changes_approval_policy_behavior() { text: "run python".to_string(), }], }) - .await - .expect("send sendUserMessage"); + .await?; let _send_user_resp: SendUserMessageResponse = to_response::( timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(send_user_id)), ) - .await - .expect("sendUserMessage timeout") - .expect("sendUserMessage resp"), - ) - .expect("deserialize sendUserMessage response"); + .await??, + )?; // Expect an ExecCommandApproval request (elicitation) let request = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_request_message(), ) - .await - .expect("waiting for exec approval request timeout") - .expect("exec approval request"); + .await??; let ServerRequest::ExecCommandApproval { request_id, params } = request else { panic!("expected ExecCommandApproval request, got: {request:?}"); }; @@ -330,17 +287,14 @@ async fn test_send_user_turn_changes_approval_policy_behavior() { request_id, serde_json::json!({ "decision": codex_core::protocol::ReviewDecision::Approved }), ) - .await - .expect("send approval response"); + .await?; // Wait for first TaskComplete let _ = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_notification_message("codex/event/task_complete"), ) - .await - .expect("task_complete 1 timeout") - .expect("task_complete 1 notification"); + .await??; // 4) sendUserTurn with approval_policy=never should run without elicitation let send_turn_id = mcp @@ -356,19 +310,15 @@ async fn test_send_user_turn_changes_approval_policy_behavior() { effort: Some(ReasoningEffort::Medium), summary: ReasoningSummary::Auto, }) - .await - .expect("send sendUserTurn"); + .await?; // Acknowledge sendUserTurn let _send_turn_resp: SendUserTurnResponse = to_response::( timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(send_turn_id)), ) - .await - .expect("sendUserTurn timeout") - .expect("sendUserTurn resp"), - ) - .expect("deserialize sendUserTurn response"); + .await??, + )?; // Ensure we do NOT receive an ExecCommandApproval request before the task completes. // If any Request is seen while waiting for task_complete, the helper will error and the test fails. @@ -376,31 +326,31 @@ async fn test_send_user_turn_changes_approval_policy_behavior() { DEFAULT_READ_TIMEOUT, mcp.read_stream_until_notification_message("codex/event/task_complete"), ) - .await - .expect("task_complete 2 timeout") - .expect("task_complete 2 notification"); + .await??; + + Ok(()) } // Helper: minimal config.toml pointing at mock provider. #[tokio::test(flavor = "multi_thread", worker_threads = 4)] -async fn test_send_user_turn_updates_sandbox_and_cwd_between_turns() { +async fn test_send_user_turn_updates_sandbox_and_cwd_between_turns() -> Result<()> { if env::var(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() { println!( "Skipping test because it cannot execute when network is disabled in a Codex sandbox." ); - return; + return Ok(()); } - let tmp = TempDir::new().expect("tmp dir"); + let tmp = TempDir::new()?; let codex_home = tmp.path().join("codex_home"); - std::fs::create_dir(&codex_home).expect("create codex home dir"); + std::fs::create_dir(&codex_home)?; let workspace_root = tmp.path().join("workspace"); - std::fs::create_dir(&workspace_root).expect("create workspace root"); + std::fs::create_dir(&workspace_root)?; let first_cwd = workspace_root.join("turn1"); let second_cwd = workspace_root.join("turn2"); - std::fs::create_dir(&first_cwd).expect("create first cwd"); - std::fs::create_dir(&second_cwd).expect("create second cwd"); + std::fs::create_dir(&first_cwd)?; + std::fs::create_dir(&second_cwd)?; let responses = vec![ create_shell_sse_response( @@ -412,10 +362,8 @@ async fn test_send_user_turn_updates_sandbox_and_cwd_between_turns() { None, Some(5000), "call-first", - ) - .expect("create first shell response"), - create_final_assistant_message_sse_response("done first") - .expect("create first final assistant message"), + )?, + create_final_assistant_message_sse_response("done first")?, create_shell_sse_response( vec![ "bash".to_string(), @@ -425,21 +373,14 @@ async fn test_send_user_turn_updates_sandbox_and_cwd_between_turns() { None, Some(5000), "call-second", - ) - .expect("create second shell response"), - create_final_assistant_message_sse_response("done second") - .expect("create second final assistant message"), + )?, + create_final_assistant_message_sse_response("done second")?, ]; let server = create_mock_chat_completions_server(responses).await; - create_config_toml(&codex_home, &server.uri()).expect("write config"); + create_config_toml(&codex_home, &server.uri())?; - let mut mcp = McpProcess::new(&codex_home) - .await - .expect("spawn mcp process"); - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .expect("init timeout") - .expect("init failed"); + let mut mcp = McpProcess::new(&codex_home).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; let new_conv_id = mcp .send_new_conversation_request(NewConversationParams { @@ -448,36 +389,29 @@ async fn test_send_user_turn_updates_sandbox_and_cwd_between_turns() { sandbox: Some(SandboxMode::WorkspaceWrite), ..Default::default() }) - .await - .expect("send newConversation"); + .await?; let new_conv_resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(new_conv_id)), ) - .await - .expect("newConversation timeout") - .expect("newConversation resp"); + .await??; let NewConversationResponse { conversation_id, model, .. - } = to_response::(new_conv_resp) - .expect("deserialize newConversation response"); + } = to_response::(new_conv_resp)?; let add_listener_id = mcp .send_add_conversation_listener_request(AddConversationListenerParams { conversation_id, experimental_raw_events: false, }) - .await - .expect("send addConversationListener"); + .await?; timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(add_listener_id)), ) - .await - .expect("addConversationListener timeout") - .expect("addConversationListener resp"); + .await??; let first_turn_id = mcp .send_send_user_turn_request(SendUserTurnParams { @@ -497,22 +431,17 @@ async fn test_send_user_turn_updates_sandbox_and_cwd_between_turns() { effort: Some(ReasoningEffort::Medium), summary: ReasoningSummary::Auto, }) - .await - .expect("send first sendUserTurn"); + .await?; timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(first_turn_id)), ) - .await - .expect("sendUserTurn 1 timeout") - .expect("sendUserTurn 1 resp"); + .await??; timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_notification_message("codex/event/task_complete"), ) - .await - .expect("task_complete 1 timeout") - .expect("task_complete 1 notification"); + .await??; let second_turn_id = mcp .send_send_user_turn_request(SendUserTurnParams { @@ -527,23 +456,18 @@ async fn test_send_user_turn_updates_sandbox_and_cwd_between_turns() { effort: Some(ReasoningEffort::Medium), summary: ReasoningSummary::Auto, }) - .await - .expect("send second sendUserTurn"); + .await?; timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(second_turn_id)), ) - .await - .expect("sendUserTurn 2 timeout") - .expect("sendUserTurn 2 resp"); + .await??; let exec_begin_notification = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_notification_message("codex/event/exec_command_begin"), ) - .await - .expect("exec_command_begin timeout") - .expect("exec_command_begin notification"); + .await??; let params = exec_begin_notification .params .clone() @@ -571,9 +495,9 @@ async fn test_send_user_turn_updates_sandbox_and_cwd_between_turns() { DEFAULT_READ_TIMEOUT, mcp.read_stream_until_notification_message("codex/event/task_complete"), ) - .await - .expect("task_complete 2 timeout") - .expect("task_complete 2 notification"); + .await??; + + Ok(()) } fn create_config_toml(codex_home: &Path, server_uri: &str) -> std::io::Result<()> { diff --git a/codex-rs/app-server/tests/suite/config.rs b/codex-rs/app-server/tests/suite/config.rs index 2809d68ca4..227d30bbe3 100644 --- a/codex-rs/app-server/tests/suite/config.rs +++ b/codex-rs/app-server/tests/suite/config.rs @@ -1,6 +1,4 @@ -use std::collections::HashMap; -use std::path::Path; - +use anyhow::Result; use app_test_support::McpProcess; use app_test_support::to_response; use codex_app_server_protocol::GetUserSavedConfigResponse; @@ -17,6 +15,8 @@ use codex_protocol::config_types::ReasoningSummary; use codex_protocol::config_types::SandboxMode; use codex_protocol::config_types::Verbosity; use pretty_assertions::assert_eq; +use std::collections::HashMap; +use std::path::Path; use tempfile::TempDir; use tokio::time::timeout; @@ -60,31 +60,21 @@ chatgpt_base_url = "https://api.chatgpt.com" } #[tokio::test(flavor = "multi_thread", worker_threads = 4)] -async fn get_config_toml_parses_all_fields() { - let codex_home = TempDir::new().unwrap_or_else(|e| panic!("create tempdir: {e}")); - create_config_toml(codex_home.path()).expect("write config.toml"); +async fn get_config_toml_parses_all_fields() -> Result<()> { + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path())?; - let mut mcp = McpProcess::new(codex_home.path()) - .await - .expect("spawn mcp process"); - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .expect("init timeout") - .expect("init failed"); + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; - let request_id = mcp - .send_get_user_saved_config_request() - .await - .expect("send getUserSavedConfig"); + let request_id = mcp.send_get_user_saved_config_request().await?; let resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(request_id)), ) - .await - .expect("getUserSavedConfig timeout") - .expect("getUserSavedConfig response"); + .await??; - let config: GetUserSavedConfigResponse = to_response(resp).expect("deserialize config"); + let config: GetUserSavedConfigResponse = to_response(resp)?; let expected = GetUserSavedConfigResponse { config: UserSavedConfig { approval_policy: Some(AskForApproval::OnRequest), @@ -122,33 +112,24 @@ async fn get_config_toml_parses_all_fields() { }; assert_eq!(config, expected); + Ok(()) } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn get_config_toml_empty() { - let codex_home = TempDir::new().unwrap_or_else(|e| panic!("create tempdir: {e}")); +async fn get_config_toml_empty() -> Result<()> { + let codex_home = TempDir::new()?; - let mut mcp = McpProcess::new(codex_home.path()) - .await - .expect("spawn mcp process"); - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .expect("init timeout") - .expect("init failed"); + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; - let request_id = mcp - .send_get_user_saved_config_request() - .await - .expect("send getUserSavedConfig"); + let request_id = mcp.send_get_user_saved_config_request().await?; let resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(request_id)), ) - .await - .expect("getUserSavedConfig timeout") - .expect("getUserSavedConfig response"); + .await??; - let config: GetUserSavedConfigResponse = to_response(resp).expect("deserialize config"); + let config: GetUserSavedConfigResponse = to_response(resp)?; let expected = GetUserSavedConfigResponse { config: UserSavedConfig { approval_policy: None, @@ -167,4 +148,5 @@ async fn get_config_toml_empty() { }; assert_eq!(config, expected); + Ok(()) } diff --git a/codex-rs/app-server/tests/suite/create_conversation.rs b/codex-rs/app-server/tests/suite/create_conversation.rs index f507baa30d..7788b8f382 100644 --- a/codex-rs/app-server/tests/suite/create_conversation.rs +++ b/codex-rs/app-server/tests/suite/create_conversation.rs @@ -1,5 +1,4 @@ -use std::path::Path; - +use anyhow::Result; use app_test_support::McpProcess; use app_test_support::create_final_assistant_message_sse_response; use app_test_support::create_mock_chat_completions_server; @@ -15,31 +14,25 @@ use codex_app_server_protocol::SendUserMessageParams; use codex_app_server_protocol::SendUserMessageResponse; use pretty_assertions::assert_eq; use serde_json::json; +use std::path::Path; use tempfile::TempDir; use tokio::time::timeout; const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_conversation_create_and_send_message_ok() { +async fn test_conversation_create_and_send_message_ok() -> Result<()> { // Mock server – we won't strictly rely on it, but provide one to satisfy any model wiring. - let responses = vec![ - create_final_assistant_message_sse_response("Done").expect("build mock assistant message"), - ]; + let responses = vec![create_final_assistant_message_sse_response("Done")?]; let server = create_mock_chat_completions_server(responses).await; // Temporary Codex home with config pointing at the mock server. - let codex_home = TempDir::new().expect("create temp dir"); - create_config_toml(codex_home.path(), &server.uri()).expect("write config.toml"); + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path(), &server.uri())?; // Start MCP server process and initialize. - let mut mcp = McpProcess::new(codex_home.path()) - .await - .expect("spawn mcp process"); - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .expect("init timeout") - .expect("init failed"); + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; // Create a conversation via the new JSON-RPC API. let new_conv_id = mcp @@ -47,22 +40,18 @@ async fn test_conversation_create_and_send_message_ok() { model: Some("o3".to_string()), ..Default::default() }) - .await - .expect("send newConversation"); + .await?; let new_conv_resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(new_conv_id)), ) - .await - .expect("newConversation timeout") - .expect("newConversation resp"); + .await??; let NewConversationResponse { conversation_id, model, reasoning_effort: _, rollout_path: _, - } = to_response::(new_conv_resp) - .expect("deserialize newConversation response"); + } = to_response::(new_conv_resp)?; assert_eq!(model, "o3"); // Add a listener so we receive notifications for this conversation (not strictly required for this test). @@ -71,19 +60,15 @@ async fn test_conversation_create_and_send_message_ok() { conversation_id, experimental_raw_events: false, }) - .await - .expect("send addConversationListener"); + .await?; let _sub: AddConversationSubscriptionResponse = to_response::( timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(add_listener_id)), ) - .await - .expect("addConversationListener timeout") - .expect("addConversationListener resp"), - ) - .expect("deserialize addConversationListener response"); + .await??, + )?; // Now send a user message via the wire API and expect an OK (empty object) result. let send_id = mcp @@ -93,36 +78,32 @@ async fn test_conversation_create_and_send_message_ok() { text: "Hello".to_string(), }], }) - .await - .expect("send sendUserMessage"); + .await?; let send_resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(send_id)), ) - .await - .expect("sendUserMessage timeout") - .expect("sendUserMessage resp"); - let _ok: SendUserMessageResponse = to_response::(send_resp) - .expect("deserialize sendUserMessage response"); + .await??; + let _ok: SendUserMessageResponse = to_response::(send_resp)?; // avoid race condition by waiting for the mock server to receive the chat.completions request let deadline = std::time::Instant::now() + DEFAULT_READ_TIMEOUT; - loop { + let requests = loop { let requests = server.received_requests().await.unwrap_or_default(); if !requests.is_empty() { - break; + break requests; } if std::time::Instant::now() >= deadline { panic!("mock server did not receive the chat.completions request in time"); } tokio::time::sleep(std::time::Duration::from_millis(10)).await; - } + }; // Verify the outbound request body matches expectations for Chat Completions. - let request = &server.received_requests().await.unwrap()[0]; - let body = request - .body_json::() - .expect("parse request body as JSON"); + let request = requests + .first() + .expect("mock server should have received at least one request"); + let body = request.body_json::()?; assert_eq!(body["model"], json!("o3")); assert!(body["stream"].as_bool().unwrap_or(false)); let messages = body["messages"] @@ -133,6 +114,7 @@ async fn test_conversation_create_and_send_message_ok() { assert_eq!(last["content"], json!("Hello")); drop(server); + Ok(()) } // Helper to create a config.toml pointing at the mock model server. diff --git a/codex-rs/app-server/tests/suite/fuzzy_file_search.rs b/codex-rs/app-server/tests/suite/fuzzy_file_search.rs index a2bc974ad0..9c95e3de34 100644 --- a/codex-rs/app-server/tests/suite/fuzzy_file_search.rs +++ b/codex-rs/app-server/tests/suite/fuzzy_file_search.rs @@ -1,5 +1,5 @@ -use anyhow::Context; use anyhow::Result; +use anyhow::anyhow; use app_test_support::McpProcess; use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::RequestId; @@ -13,48 +13,39 @@ const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_fuzzy_file_search_sorts_and_includes_indices() -> Result<()> { // Prepare a temporary Codex home and a separate root with test files. - let codex_home = TempDir::new().context("create temp codex home")?; - let root = TempDir::new().context("create temp search root")?; + let codex_home = TempDir::new()?; + let root = TempDir::new()?; // Create files designed to have deterministic ordering for query "abe". - std::fs::write(root.path().join("abc"), "x").context("write file abc")?; - std::fs::write(root.path().join("abcde"), "x").context("write file abcde")?; - std::fs::write(root.path().join("abexy"), "x").context("write file abexy")?; - std::fs::write(root.path().join("zzz.txt"), "x").context("write file zzz")?; + std::fs::write(root.path().join("abc"), "x")?; + std::fs::write(root.path().join("abcde"), "x")?; + std::fs::write(root.path().join("abexy"), "x")?; + std::fs::write(root.path().join("zzz.txt"), "x")?; let sub_dir = root.path().join("sub"); - std::fs::create_dir_all(&sub_dir).context("create sub dir")?; + std::fs::create_dir_all(&sub_dir)?; let sub_abce_path = sub_dir.join("abce"); - std::fs::write(&sub_abce_path, "x").context("write file sub/abce")?; + std::fs::write(&sub_abce_path, "x")?; let sub_abce_rel = sub_abce_path - .strip_prefix(root.path()) - .context("strip root prefix from sub/abce")? + .strip_prefix(root.path())? .to_string_lossy() .to_string(); // Start MCP server and initialize. - let mut mcp = McpProcess::new(codex_home.path()) - .await - .context("spawn mcp")?; - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .context("init timeout")? - .context("init failed")?; + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; let root_path = root.path().to_string_lossy().to_string(); // Send fuzzyFileSearch request. let request_id = mcp .send_fuzzy_file_search_request("abe", vec![root_path.clone()], None) - .await - .context("send fuzzyFileSearch")?; + .await?; // Read response and verify shape and ordering. let resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(request_id)), ) - .await - .context("fuzzyFileSearch timeout")? - .context("fuzzyFileSearch resp")?; + .await??; let value = resp.result; // The path separator on Windows affects the score. @@ -94,24 +85,18 @@ async fn test_fuzzy_file_search_sorts_and_includes_indices() -> Result<()> { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_fuzzy_file_search_accepts_cancellation_token() -> Result<()> { - let codex_home = TempDir::new().context("create temp codex home")?; - let root = TempDir::new().context("create temp search root")?; + let codex_home = TempDir::new()?; + let root = TempDir::new()?; - std::fs::write(root.path().join("alpha.txt"), "contents").context("write alpha")?; + std::fs::write(root.path().join("alpha.txt"), "contents")?; - let mut mcp = McpProcess::new(codex_home.path()) - .await - .context("spawn mcp")?; - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .context("init timeout")? - .context("init failed")?; + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; let root_path = root.path().to_string_lossy().to_string(); let request_id = mcp .send_fuzzy_file_search_request("alp", vec![root_path.clone()], None) - .await - .context("send fuzzyFileSearch")?; + .await?; let request_id_2 = mcp .send_fuzzy_file_search_request( @@ -119,23 +104,20 @@ async fn test_fuzzy_file_search_accepts_cancellation_token() -> Result<()> { vec![root_path.clone()], Some(request_id.to_string()), ) - .await - .context("send fuzzyFileSearch")?; + .await?; let resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(request_id_2)), ) - .await - .context("fuzzyFileSearch timeout")? - .context("fuzzyFileSearch resp")?; + .await??; let files = resp .result .get("files") - .context("files key missing")? + .ok_or_else(|| anyhow!("files key missing"))? .as_array() - .context("files not array")? + .ok_or_else(|| anyhow!("files not array"))? .clone(); assert_eq!(files.len(), 1); diff --git a/codex-rs/app-server/tests/suite/list_resume.rs b/codex-rs/app-server/tests/suite/list_resume.rs index 85416f60bd..44578e04fa 100644 --- a/codex-rs/app-server/tests/suite/list_resume.rs +++ b/codex-rs/app-server/tests/suite/list_resume.rs @@ -1,6 +1,4 @@ -use std::fs; -use std::path::Path; - +use anyhow::Result; use app_test_support::McpProcess; use app_test_support::to_response; use codex_app_server_protocol::JSONRPCNotification; @@ -15,6 +13,8 @@ use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::SessionConfiguredNotification; use pretty_assertions::assert_eq; use serde_json::json; +use std::fs; +use std::path::Path; use tempfile::TempDir; use tokio::time::timeout; use uuid::Uuid; @@ -22,38 +22,33 @@ use uuid::Uuid; const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_list_and_resume_conversations() { +async fn test_list_and_resume_conversations() -> Result<()> { // Prepare a temporary CODEX_HOME with a few fake rollout files. - let codex_home = TempDir::new().expect("create temp dir"); + let codex_home = TempDir::new()?; create_fake_rollout( codex_home.path(), "2025-01-02T12-00-00", "2025-01-02T12:00:00Z", "Hello A", Some("openai"), - ); + )?; create_fake_rollout( codex_home.path(), "2025-01-01T13-00-00", "2025-01-01T13:00:00Z", "Hello B", Some("openai"), - ); + )?; create_fake_rollout( codex_home.path(), "2025-01-01T12-00-00", "2025-01-01T12:00:00Z", "Hello C", None, - ); + )?; - let mut mcp = McpProcess::new(codex_home.path()) - .await - .expect("spawn mcp process"); - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .expect("init timeout") - .expect("init failed"); + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; // Request first page with size 2 let req_id = mcp @@ -62,17 +57,14 @@ async fn test_list_and_resume_conversations() { cursor: None, model_providers: None, }) - .await - .expect("send listConversations"); + .await?; let resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(req_id)), ) - .await - .expect("listConversations timeout") - .expect("listConversations resp"); + .await??; let ListConversationsResponse { items, next_cursor } = - to_response::(resp).expect("deserialize response"); + to_response::(resp)?; assert_eq!(items.len(), 2); // Newest first; preview text should match @@ -90,20 +82,17 @@ async fn test_list_and_resume_conversations() { cursor: next_cursor, model_providers: None, }) - .await - .expect("send listConversations page 2"); + .await?; let resp2: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(req_id2)), ) - .await - .expect("listConversations page 2 timeout") - .expect("listConversations page 2 resp"); + .await??; let ListConversationsResponse { items: items2, next_cursor: next2, .. - } = to_response::(resp2).expect("deserialize response"); + } = to_response::(resp2)?; assert_eq!(items2.len(), 1); assert_eq!(items2[0].preview, "Hello C"); assert_eq!(items2[0].model_provider, "openai"); @@ -116,7 +105,7 @@ async fn test_list_and_resume_conversations() { "2025-01-01T11:30:00Z", "Hello TP", Some("test-provider"), - ); + )?; // Filtering by model provider should return only matching sessions. let filter_req_id = mcp @@ -125,19 +114,16 @@ async fn test_list_and_resume_conversations() { cursor: None, model_providers: Some(vec!["test-provider".to_string()]), }) - .await - .expect("send listConversations filtered"); + .await?; let filter_resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(filter_req_id)), ) - .await - .expect("listConversations filtered timeout") - .expect("listConversations filtered resp"); + .await??; let ListConversationsResponse { items: filtered_items, next_cursor: filtered_next, - } = to_response::(filter_resp).expect("deserialize filtered"); + } = to_response::(filter_resp)?; assert_eq!(filtered_items.len(), 1); assert_eq!(filtered_next, None); assert_eq!(filtered_items[0].preview, "Hello TP"); @@ -150,20 +136,16 @@ async fn test_list_and_resume_conversations() { cursor: None, model_providers: Some(Vec::new()), }) - .await - .expect("send listConversations unfiltered"); + .await?; let unfiltered_resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(unfiltered_req_id)), ) - .await - .expect("listConversations unfiltered timeout") - .expect("listConversations unfiltered resp"); + .await??; let ListConversationsResponse { items: unfiltered_items, next_cursor: unfiltered_next, - } = to_response::(unfiltered_resp) - .expect("deserialize unfiltered response"); + } = to_response::(unfiltered_resp)?; assert_eq!(unfiltered_items.len(), 4); assert!(unfiltered_next.is_none()); @@ -173,19 +155,16 @@ async fn test_list_and_resume_conversations() { cursor: None, model_providers: Some(vec!["other".to_string()]), }) - .await - .expect("send listConversations filtered empty"); + .await?; let empty_resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(empty_req_id)), ) - .await - .expect("listConversations filtered empty timeout") - .expect("listConversations filtered empty resp"); + .await??; let ListConversationsResponse { items: empty_items, next_cursor: empty_next, - } = to_response::(empty_resp).expect("deserialize filtered empty"); + } = to_response::(empty_resp)?; assert!(empty_items.is_empty()); assert!(empty_next.is_none()); @@ -198,20 +177,15 @@ async fn test_list_and_resume_conversations() { ..Default::default() }), }) - .await - .expect("send resumeConversation"); + .await?; // Expect a codex/event notification with msg.type == sessionConfigured let notification: JSONRPCNotification = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_notification_message("sessionConfigured"), ) - .await - .expect("sessionConfigured notification timeout") - .expect("sessionConfigured notification"); - let session_configured: ServerNotification = notification - .try_into() - .expect("deserialize sessionConfigured notification"); + .await??; + let session_configured: ServerNotification = notification.try_into()?; // Basic shape assertion: ensure event type is sessionConfigured let ServerNotification::SessionConfigured(SessionConfiguredNotification { model, @@ -229,15 +203,14 @@ async fn test_list_and_resume_conversations() { DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(resume_req_id)), ) - .await - .expect("resumeConversation timeout") - .expect("resumeConversation resp"); + .await??; let ResumeConversationResponse { conversation_id, .. - } = to_response::(resume_resp) - .expect("deserialize resumeConversation response"); + } = to_response::(resume_resp)?; // conversation id should be a valid UUID assert!(!conversation_id.to_string().is_empty()); + + Ok(()) } fn create_fake_rollout( @@ -246,14 +219,14 @@ fn create_fake_rollout( meta_rfc3339: &str, preview: &str, model_provider: Option<&str>, -) { +) -> Result<()> { let uuid = Uuid::new_v4(); // sessions/YYYY/MM/DD/ derived from filename_ts (YYYY-MM-DDThh-mm-ss) let year = &filename_ts[0..4]; let month = &filename_ts[5..7]; let day = &filename_ts[8..10]; let dir = codex_home.join("sessions").join(year).join(month).join(day); - fs::create_dir_all(&dir).unwrap_or_else(|e| panic!("create sessions dir: {e}")); + fs::create_dir_all(&dir)?; let file_path = dir.join(format!("rollout-{filename_ts}-{uuid}.jsonl")); let mut lines = Vec::new(); @@ -303,6 +276,6 @@ fn create_fake_rollout( }) .to_string(), ); - fs::write(file_path, lines.join("\n") + "\n") - .unwrap_or_else(|e| panic!("write rollout file: {e}")); + fs::write(file_path, lines.join("\n") + "\n")?; + Ok(()) } diff --git a/codex-rs/app-server/tests/suite/login.rs b/codex-rs/app-server/tests/suite/login.rs index d4d6374bc4..c5470c3ec4 100644 --- a/codex-rs/app-server/tests/suite/login.rs +++ b/codex-rs/app-server/tests/suite/login.rs @@ -1,6 +1,4 @@ -use std::path::Path; -use std::time::Duration; - +use anyhow::Result; use app_test_support::McpProcess; use app_test_support::to_response; use codex_app_server_protocol::CancelLoginChatGptParams; @@ -12,8 +10,11 @@ use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::LoginChatGptResponse; use codex_app_server_protocol::LogoutChatGptResponse; use codex_app_server_protocol::RequestId; +use codex_core::auth::AuthCredentialsStoreMode; use codex_login::login_with_api_key; use serial_test::serial; +use std::path::Path; +use std::time::Duration; use tempfile::TempDir; use tokio::time::timeout; @@ -42,32 +43,26 @@ stream_max_retries = 0 } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn logout_chatgpt_removes_auth() { - let codex_home = TempDir::new().unwrap_or_else(|e| panic!("create tempdir: {e}")); - create_config_toml(codex_home.path()).expect("write config.toml"); - login_with_api_key(codex_home.path(), "sk-test-key").expect("seed api key"); +async fn logout_chatgpt_removes_auth() -> Result<()> { + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path())?; + login_with_api_key( + codex_home.path(), + "sk-test-key", + AuthCredentialsStoreMode::File, + )?; assert!(codex_home.path().join("auth.json").exists()); - let mut mcp = McpProcess::new_with_env(codex_home.path(), &[("OPENAI_API_KEY", None)]) - .await - .expect("spawn mcp process"); - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .expect("init timeout") - .expect("init failed"); + let mut mcp = McpProcess::new_with_env(codex_home.path(), &[("OPENAI_API_KEY", None)]).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; - let id = mcp - .send_logout_chat_gpt_request() - .await - .expect("send logoutChatGpt"); + let id = mcp.send_logout_chat_gpt_request().await?; let resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(id)), ) - .await - .expect("logoutChatGpt timeout") - .expect("logoutChatGpt response"); - let _ok: LogoutChatGptResponse = to_response(resp).expect("deserialize logout response"); + .await??; + let _ok: LogoutChatGptResponse = to_response(resp)?; assert!( !codex_home.path().join("auth.json").exists(), @@ -80,63 +75,47 @@ async fn logout_chatgpt_removes_auth() { include_token: Some(true), refresh_token: Some(false), }) - .await - .expect("send getAuthStatus"); + .await?; let status_resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(status_id)), ) - .await - .expect("getAuthStatus timeout") - .expect("getAuthStatus response"); - let status: GetAuthStatusResponse = to_response(status_resp).expect("deserialize status"); + .await??; + let status: GetAuthStatusResponse = to_response(status_resp)?; assert_eq!(status.auth_method, None); assert_eq!(status.auth_token, None); + Ok(()) } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] // Serialize tests that launch the login server since it binds to a fixed port. #[serial(login_port)] -async fn login_and_cancel_chatgpt() { - let codex_home = TempDir::new().unwrap_or_else(|e| panic!("create tempdir: {e}")); - create_config_toml(codex_home.path()).unwrap_or_else(|err| panic!("write config.toml: {err}")); +async fn login_and_cancel_chatgpt() -> Result<()> { + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path())?; - let mut mcp = McpProcess::new(codex_home.path()) - .await - .expect("spawn mcp process"); - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .expect("init timeout") - .expect("init failed"); + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; - let login_id = mcp - .send_login_chat_gpt_request() - .await - .expect("send loginChatGpt"); + let login_id = mcp.send_login_chat_gpt_request().await?; let login_resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(login_id)), ) - .await - .expect("loginChatGpt timeout") - .expect("loginChatGpt response"); - let login: LoginChatGptResponse = to_response(login_resp).expect("deserialize login resp"); + .await??; + let login: LoginChatGptResponse = to_response(login_resp)?; let cancel_id = mcp .send_cancel_login_chat_gpt_request(CancelLoginChatGptParams { login_id: login.login_id, }) - .await - .expect("send cancelLoginChatGpt"); + .await?; let cancel_resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(cancel_id)), ) - .await - .expect("cancelLoginChatGpt timeout") - .expect("cancelLoginChatGpt response"); - let _ok: CancelLoginChatGptResponse = - to_response(cancel_resp).expect("deserialize cancel response"); + .await??; + let _ok: CancelLoginChatGptResponse = to_response(cancel_resp)?; // Optionally observe the completion notification; do not fail if it races. let maybe_note = timeout( @@ -147,6 +126,7 @@ async fn login_and_cancel_chatgpt() { if maybe_note.is_err() { eprintln!("warning: did not observe login_chat_gpt_complete notification after cancel"); } + Ok(()) } fn create_config_toml_forced_login(codex_home: &Path, forced_method: &str) -> std::io::Result<()> { @@ -179,68 +159,48 @@ forced_chatgpt_workspace_id = "{workspace_id}" } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn login_chatgpt_rejected_when_forced_api() { - let codex_home = TempDir::new().unwrap_or_else(|e| panic!("create tempdir: {e}")); - create_config_toml_forced_login(codex_home.path(), "api") - .unwrap_or_else(|err| panic!("write config.toml: {err}")); +async fn login_chatgpt_rejected_when_forced_api() -> Result<()> { + let codex_home = TempDir::new()?; + create_config_toml_forced_login(codex_home.path(), "api")?; - let mut mcp = McpProcess::new(codex_home.path()) - .await - .expect("spawn mcp process"); - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .expect("init timeout") - .expect("init failed"); + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; - let request_id = mcp - .send_login_chat_gpt_request() - .await - .expect("send loginChatGpt"); + let request_id = mcp.send_login_chat_gpt_request().await?; let err: JSONRPCError = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_error_message(RequestId::Integer(request_id)), ) - .await - .expect("loginChatGpt error timeout") - .expect("loginChatGpt error"); + .await??; assert_eq!( err.error.message, "ChatGPT login is disabled. Use API key login instead." ); + Ok(()) } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] // Serialize tests that launch the login server since it binds to a fixed port. #[serial(login_port)] -async fn login_chatgpt_includes_forced_workspace_query_param() { - let codex_home = TempDir::new().unwrap_or_else(|e| panic!("create tempdir: {e}")); - create_config_toml_forced_workspace(codex_home.path(), "ws-forced") - .unwrap_or_else(|err| panic!("write config.toml: {err}")); +async fn login_chatgpt_includes_forced_workspace_query_param() -> Result<()> { + let codex_home = TempDir::new()?; + create_config_toml_forced_workspace(codex_home.path(), "ws-forced")?; - let mut mcp = McpProcess::new(codex_home.path()) - .await - .expect("spawn mcp process"); - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .expect("init timeout") - .expect("init failed"); + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; - let request_id = mcp - .send_login_chat_gpt_request() - .await - .expect("send loginChatGpt"); + let request_id = mcp.send_login_chat_gpt_request().await?; let resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(request_id)), ) - .await - .expect("loginChatGpt timeout") - .expect("loginChatGpt response"); + .await??; - let login: LoginChatGptResponse = to_response(resp).expect("deserialize login resp"); + let login: LoginChatGptResponse = to_response(resp)?; assert!( login.auth_url.contains("allowed_workspace_id=ws-forced"), "auth URL should include forced workspace" ); + Ok(()) } diff --git a/codex-rs/app-server/tests/suite/rate_limits.rs b/codex-rs/app-server/tests/suite/rate_limits.rs index 302fd60349..16cbf153e9 100644 --- a/codex-rs/app-server/tests/suite/rate_limits.rs +++ b/codex-rs/app-server/tests/suite/rate_limits.rs @@ -1,4 +1,3 @@ -use anyhow::Context; use anyhow::Result; use app_test_support::ChatGptAuthFixture; use app_test_support::McpProcess; @@ -9,6 +8,7 @@ use codex_app_server_protocol::JSONRPCError; use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::LoginApiKeyParams; use codex_app_server_protocol::RequestId; +use codex_core::auth::AuthCredentialsStoreMode; use codex_protocol::protocol::RateLimitSnapshot; use codex_protocol::protocol::RateLimitWindow; use pretty_assertions::assert_eq; @@ -28,28 +28,18 @@ const INVALID_REQUEST_ERROR_CODE: i64 = -32600; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn get_account_rate_limits_requires_auth() -> Result<()> { - let codex_home = TempDir::new().context("create codex home tempdir")?; + let codex_home = TempDir::new()?; - let mut mcp = McpProcess::new_with_env(codex_home.path(), &[("OPENAI_API_KEY", None)]) - .await - .context("spawn mcp process")?; - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .context("initialize timeout")? - .context("initialize request")?; + let mut mcp = McpProcess::new_with_env(codex_home.path(), &[("OPENAI_API_KEY", None)]).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; - let request_id = mcp - .send_get_account_rate_limits_request() - .await - .context("send account/rateLimits/read")?; + let request_id = mcp.send_get_account_rate_limits_request().await?; let error: JSONRPCError = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_error_message(RequestId::Integer(request_id)), ) - .await - .context("account/rateLimits/read timeout")? - .context("account/rateLimits/read error")?; + .await??; assert_eq!(error.id, RequestId::Integer(request_id)); assert_eq!(error.error.code, INVALID_REQUEST_ERROR_CODE); @@ -63,30 +53,20 @@ async fn get_account_rate_limits_requires_auth() -> Result<()> { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn get_account_rate_limits_requires_chatgpt_auth() -> Result<()> { - let codex_home = TempDir::new().context("create codex home tempdir")?; + let codex_home = TempDir::new()?; - let mut mcp = McpProcess::new(codex_home.path()) - .await - .context("spawn mcp process")?; - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .context("initialize timeout")? - .context("initialize request")?; + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; login_with_api_key(&mut mcp, "sk-test-key").await?; - let request_id = mcp - .send_get_account_rate_limits_request() - .await - .context("send account/rateLimits/read")?; + let request_id = mcp.send_get_account_rate_limits_request().await?; let error: JSONRPCError = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_error_message(RequestId::Integer(request_id)), ) - .await - .context("account/rateLimits/read timeout")? - .context("account/rateLimits/read error")?; + .await??; assert_eq!(error.id, RequestId::Integer(request_id)); assert_eq!(error.error.code, INVALID_REQUEST_ERROR_CODE); @@ -100,18 +80,18 @@ async fn get_account_rate_limits_requires_chatgpt_auth() -> Result<()> { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn get_account_rate_limits_returns_snapshot() -> Result<()> { - let codex_home = TempDir::new().context("create codex home tempdir")?; + let codex_home = TempDir::new()?; write_chatgpt_auth( codex_home.path(), ChatGptAuthFixture::new("chatgpt-token") .account_id("account-123") .plan_type("pro"), - ) - .context("write chatgpt auth")?; + AuthCredentialsStoreMode::File, + )?; let server = MockServer::start().await; let server_url = server.uri(); - write_chatgpt_base_url(codex_home.path(), &server_url).context("write chatgpt base url")?; + write_chatgpt_base_url(codex_home.path(), &server_url)?; let primary_reset_timestamp = chrono::DateTime::parse_from_rfc3339("2025-01-01T00:02:00Z") .expect("parse primary reset timestamp") @@ -147,29 +127,18 @@ async fn get_account_rate_limits_returns_snapshot() -> Result<()> { .mount(&server) .await; - let mut mcp = McpProcess::new_with_env(codex_home.path(), &[("OPENAI_API_KEY", None)]) - .await - .context("spawn mcp process")?; - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .context("initialize timeout")? - .context("initialize request")?; + let mut mcp = McpProcess::new_with_env(codex_home.path(), &[("OPENAI_API_KEY", None)]).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; - let request_id = mcp - .send_get_account_rate_limits_request() - .await - .context("send account/rateLimits/read")?; + let request_id = mcp.send_get_account_rate_limits_request().await?; let response: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(request_id)), ) - .await - .context("account/rateLimits/read timeout")? - .context("account/rateLimits/read response")?; + .await??; - let received: GetAccountRateLimitsResponse = - to_response(response).context("deserialize rate limit response")?; + let received: GetAccountRateLimitsResponse = to_response(response)?; let expected = GetAccountRateLimitsResponse { rate_limits: RateLimitSnapshot { @@ -195,16 +164,13 @@ async fn login_with_api_key(mcp: &mut McpProcess, api_key: &str) -> Result<()> { .send_login_api_key_request(LoginApiKeyParams { api_key: api_key.to_string(), }) - .await - .context("send loginApiKey")?; + .await?; timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(request_id)), ) - .await - .context("loginApiKey timeout")? - .context("loginApiKey response")?; + .await??; Ok(()) } diff --git a/codex-rs/app-server/tests/suite/send_message.rs b/codex-rs/app-server/tests/suite/send_message.rs index b3f04c3393..8d4e508c38 100644 --- a/codex-rs/app-server/tests/suite/send_message.rs +++ b/codex-rs/app-server/tests/suite/send_message.rs @@ -1,5 +1,4 @@ -use std::path::Path; - +use anyhow::Result; use app_test_support::McpProcess; use app_test_support::create_final_assistant_message_sse_response; use app_test_support::create_mock_chat_completions_server; @@ -18,49 +17,42 @@ use codex_protocol::ConversationId; use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseItem; use pretty_assertions::assert_eq; +use std::path::Path; use tempfile::TempDir; use tokio::time::timeout; const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); #[tokio::test] -async fn test_send_message_success() { +async fn test_send_message_success() -> Result<()> { // Spin up a mock completions server that immediately ends the Codex turn. // Two Codex turns hit the mock model (session start + send-user-message). Provide two SSE responses. let responses = vec![ - create_final_assistant_message_sse_response("Done").expect("build mock assistant message"), - create_final_assistant_message_sse_response("Done").expect("build mock assistant message"), + create_final_assistant_message_sse_response("Done")?, + create_final_assistant_message_sse_response("Done")?, ]; let server = create_mock_chat_completions_server(responses).await; // Create a temporary Codex home with config pointing at the mock server. - let codex_home = TempDir::new().expect("create temp dir"); - create_config_toml(codex_home.path(), &server.uri()).expect("write config.toml"); + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path(), &server.uri())?; // Start MCP server process and initialize. - let mut mcp = McpProcess::new(codex_home.path()) - .await - .expect("spawn mcp process"); - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .expect("init timed out") - .expect("init failed"); + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; // Start a conversation using the new wire API. let new_conv_id = mcp .send_new_conversation_request(NewConversationParams::default()) - .await - .expect("send newConversation"); + .await?; let new_conv_resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(new_conv_id)), ) - .await - .expect("newConversation timeout") - .expect("newConversation resp"); + .await??; let NewConversationResponse { conversation_id, .. - } = to_response::<_>(new_conv_resp).expect("deserialize newConversation response"); + } = to_response::<_>(new_conv_resp)?; // 2) addConversationListener let add_listener_id = mcp @@ -68,25 +60,27 @@ async fn test_send_message_success() { conversation_id, experimental_raw_events: false, }) - .await - .expect("send addConversationListener"); + .await?; let add_listener_resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(add_listener_id)), ) - .await - .expect("addConversationListener timeout") - .expect("addConversationListener resp"); + .await??; let AddConversationSubscriptionResponse { subscription_id: _ } = - to_response::<_>(add_listener_resp).expect("deserialize addConversationListener response"); + to_response::<_>(add_listener_resp)?; // Now exercise sendUserMessage twice. - send_message("Hello", conversation_id, &mut mcp).await; - send_message("Hello again", conversation_id, &mut mcp).await; + send_message("Hello", conversation_id, &mut mcp).await?; + send_message("Hello again", conversation_id, &mut mcp).await?; + Ok(()) } #[expect(clippy::expect_used)] -async fn send_message(message: &str, conversation_id: ConversationId, mcp: &mut McpProcess) { +async fn send_message( + message: &str, + conversation_id: ConversationId, + mcp: &mut McpProcess, +) -> Result<()> { // Now exercise sendUserMessage. let send_id = mcp .send_send_user_message_request(SendUserMessageParams { @@ -95,19 +89,15 @@ async fn send_message(message: &str, conversation_id: ConversationId, mcp: &mut text: message.to_string(), }], }) - .await - .expect("send sendUserMessage"); + .await?; let response: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(send_id)), ) - .await - .expect("sendUserMessage response timeout") - .expect("sendUserMessage response error"); + .await??; - let _ok: SendUserMessageResponse = to_response::(response) - .expect("deserialize sendUserMessage response"); + let _ok: SendUserMessageResponse = to_response::(response)?; // Verify the task_finished notification is received. // Note this also ensures that the final request to the server was made. @@ -115,9 +105,7 @@ async fn send_message(message: &str, conversation_id: ConversationId, mcp: &mut DEFAULT_READ_TIMEOUT, mcp.read_stream_until_notification_message("codex/event/task_complete"), ) - .await - .expect("task_finished_notification timeout") - .expect("task_finished_notification resp"); + .await??; let serde_json::Value::Object(map) = task_finished_notification .params .expect("notification should have params") @@ -139,57 +127,45 @@ async fn send_message(message: &str, conversation_id: ConversationId, mcp: &mut raw_attempt.is_err(), "unexpected raw item notification when not opted in" ); + Ok(()) } #[tokio::test] -async fn test_send_message_raw_notifications_opt_in() { - let responses = vec![ - create_final_assistant_message_sse_response("Done").expect("build mock assistant message"), - ]; +async fn test_send_message_raw_notifications_opt_in() -> Result<()> { + let responses = vec![create_final_assistant_message_sse_response("Done")?]; let server = create_mock_chat_completions_server(responses).await; - let codex_home = TempDir::new().expect("create temp dir"); - create_config_toml(codex_home.path(), &server.uri()).expect("write config.toml"); + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path(), &server.uri())?; - let mut mcp = McpProcess::new(codex_home.path()) - .await - .expect("spawn mcp process"); - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .expect("init timed out") - .expect("init failed"); + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; let new_conv_id = mcp .send_new_conversation_request(NewConversationParams::default()) - .await - .expect("send newConversation"); + .await?; let new_conv_resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(new_conv_id)), ) - .await - .expect("newConversation timeout") - .expect("newConversation resp"); + .await??; let NewConversationResponse { conversation_id, .. - } = to_response::<_>(new_conv_resp).expect("deserialize newConversation response"); + } = to_response::<_>(new_conv_resp)?; let add_listener_id = mcp .send_add_conversation_listener_request(AddConversationListenerParams { conversation_id, experimental_raw_events: true, }) - .await - .expect("send addConversationListener"); + .await?; let add_listener_resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(add_listener_id)), ) - .await - .expect("addConversationListener timeout") - .expect("addConversationListener resp"); + .await??; let AddConversationSubscriptionResponse { subscription_id: _ } = - to_response::<_>(add_listener_resp).expect("deserialize addConversationListener response"); + to_response::<_>(add_listener_resp)?; let send_id = mcp .send_send_user_message_request(SendUserMessageParams { @@ -198,8 +174,7 @@ async fn test_send_message_raw_notifications_opt_in() { text: "Hello".to_string(), }], }) - .await - .expect("send sendUserMessage"); + .await?; let instructions = read_raw_response_item(&mut mcp, conversation_id).await; assert_instructions_message(&instructions); @@ -211,11 +186,8 @@ async fn test_send_message_raw_notifications_opt_in() { DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(send_id)), ) - .await - .expect("sendUserMessage response timeout") - .expect("sendUserMessage response error"); - let _ok: SendUserMessageResponse = to_response::(response) - .expect("deserialize sendUserMessage response"); + .await??; + let _ok: SendUserMessageResponse = to_response::(response)?; let user_message = read_raw_response_item(&mut mcp, conversation_id).await; assert_user_message(&user_message, "Hello"); @@ -228,17 +200,16 @@ async fn test_send_message_raw_notifications_opt_in() { mcp.read_stream_until_notification_message("codex/event/task_complete"), ) .await; + + Ok(()) } #[tokio::test] -async fn test_send_message_session_not_found() { +async fn test_send_message_session_not_found() -> Result<()> { // Start MCP without creating a Codex session - let codex_home = TempDir::new().expect("tempdir"); - let mut mcp = McpProcess::new(codex_home.path()).await.expect("spawn"); - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .expect("timeout") - .expect("init"); + let codex_home = TempDir::new()?; + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; let unknown = ConversationId::new(); let req_id = mcp @@ -248,18 +219,16 @@ async fn test_send_message_session_not_found() { text: "ping".to_string(), }], }) - .await - .expect("send sendUserMessage"); + .await?; // Expect an error response for unknown conversation. let err = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_error_message(RequestId::Integer(req_id)), ) - .await - .expect("timeout") - .expect("error"); + .await??; assert_eq!(err.id, RequestId::Integer(req_id)); + Ok(()) } // --------------------------------------------------------------------------- diff --git a/codex-rs/app-server/tests/suite/set_default_model.rs b/codex-rs/app-server/tests/suite/set_default_model.rs index 6769faa9dc..0c2aa22999 100644 --- a/codex-rs/app-server/tests/suite/set_default_model.rs +++ b/codex-rs/app-server/tests/suite/set_default_model.rs @@ -1,5 +1,4 @@ -use std::path::Path; - +use anyhow::Result; use app_test_support::McpProcess; use app_test_support::to_response; use codex_app_server_protocol::JSONRPCResponse; @@ -8,50 +7,38 @@ use codex_app_server_protocol::SetDefaultModelParams; use codex_app_server_protocol::SetDefaultModelResponse; use codex_core::config::ConfigToml; use pretty_assertions::assert_eq; +use std::path::Path; use tempfile::TempDir; use tokio::time::timeout; const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn set_default_model_persists_overrides() { - let codex_home = TempDir::new().expect("create tempdir"); - create_config_toml(codex_home.path()).expect("write config.toml"); +async fn set_default_model_persists_overrides() -> Result<()> { + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path())?; - let mut mcp = McpProcess::new(codex_home.path()) - .await - .expect("spawn mcp process"); - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .expect("init timeout") - .expect("init failed"); + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; let params = SetDefaultModelParams { model: Some("gpt-4.1".to_string()), reasoning_effort: None, }; - let request_id = mcp - .send_set_default_model_request(params) - .await - .expect("send setDefaultModel"); + let request_id = mcp.send_set_default_model_request(params).await?; let resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(request_id)), ) - .await - .expect("setDefaultModel timeout") - .expect("setDefaultModel response"); + .await??; - let _: SetDefaultModelResponse = - to_response(resp).expect("deserialize setDefaultModel response"); + let _: SetDefaultModelResponse = to_response(resp)?; let config_path = codex_home.path().join("config.toml"); - let config_contents = tokio::fs::read_to_string(&config_path) - .await - .expect("read config.toml"); - let config_toml: ConfigToml = toml::from_str(&config_contents).expect("parse config.toml"); + let config_contents = tokio::fs::read_to_string(&config_path).await?; + let config_toml: ConfigToml = toml::from_str(&config_contents)?; assert_eq!( ConfigToml { @@ -61,6 +48,7 @@ async fn set_default_model_persists_overrides() { }, config_toml, ); + Ok(()) } // Helper to create a config.toml; mirrors create_conversation.rs diff --git a/codex-rs/app-server/tests/suite/user_agent.rs b/codex-rs/app-server/tests/suite/user_agent.rs index 95a0b1a3e0..52ba6e56aa 100644 --- a/codex-rs/app-server/tests/suite/user_agent.rs +++ b/codex-rs/app-server/tests/suite/user_agent.rs @@ -1,3 +1,4 @@ +use anyhow::Result; use app_test_support::McpProcess; use app_test_support::to_response; use codex_app_server_protocol::GetUserAgentResponse; @@ -10,28 +11,18 @@ use tokio::time::timeout; const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn get_user_agent_returns_current_codex_user_agent() { - let codex_home = TempDir::new().unwrap_or_else(|err| panic!("create tempdir: {err}")); +async fn get_user_agent_returns_current_codex_user_agent() -> Result<()> { + let codex_home = TempDir::new()?; - let mut mcp = McpProcess::new(codex_home.path()) - .await - .expect("spawn mcp process"); - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .expect("initialize timeout") - .expect("initialize request"); + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; - let request_id = mcp - .send_get_user_agent_request() - .await - .expect("send getUserAgent"); + let request_id = mcp.send_get_user_agent_request().await?; let response: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(request_id)), ) - .await - .expect("getUserAgent timeout") - .expect("getUserAgent response"); + .await??; let os_info = os_info::get(); let user_agent = format!( @@ -42,9 +33,9 @@ async fn get_user_agent_returns_current_codex_user_agent() { codex_core::terminal::user_agent() ); - let received: GetUserAgentResponse = - to_response(response).expect("deserialize getUserAgent response"); + let received: GetUserAgentResponse = to_response(response)?; let expected = GetUserAgentResponse { user_agent }; assert_eq!(received, expected); + Ok(()) } diff --git a/codex-rs/app-server/tests/suite/user_info.rs b/codex-rs/app-server/tests/suite/user_info.rs index b730b77fed..6a44f1a3ab 100644 --- a/codex-rs/app-server/tests/suite/user_info.rs +++ b/codex-rs/app-server/tests/suite/user_info.rs @@ -1,5 +1,4 @@ -use std::time::Duration; - +use anyhow::Result; use app_test_support::ChatGptAuthFixture; use app_test_support::McpProcess; use app_test_support::to_response; @@ -7,45 +6,41 @@ use app_test_support::write_chatgpt_auth; use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::RequestId; use codex_app_server_protocol::UserInfoResponse; +use codex_core::auth::AuthCredentialsStoreMode; use pretty_assertions::assert_eq; +use std::time::Duration; use tempfile::TempDir; use tokio::time::timeout; const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(10); #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn user_info_returns_email_from_auth_json() { - let codex_home = TempDir::new().expect("create tempdir"); +async fn user_info_returns_email_from_auth_json() -> Result<()> { + let codex_home = TempDir::new()?; write_chatgpt_auth( codex_home.path(), ChatGptAuthFixture::new("access") .refresh_token("refresh") .email("user@example.com"), - ) - .expect("write chatgpt auth"); + AuthCredentialsStoreMode::File, + )?; - let mut mcp = McpProcess::new(codex_home.path()) - .await - .expect("spawn mcp process"); - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()) - .await - .expect("initialize timeout") - .expect("initialize request"); + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; - let request_id = mcp.send_user_info_request().await.expect("send userInfo"); + let request_id = mcp.send_user_info_request().await?; let response: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(request_id)), ) - .await - .expect("userInfo timeout") - .expect("userInfo response"); + .await??; - let received: UserInfoResponse = to_response(response).expect("deserialize userInfo response"); + let received: UserInfoResponse = to_response(response)?; let expected = UserInfoResponse { alleged_user_email: Some("user@example.com".to_string()), }; assert_eq!(received, expected); + Ok(()) } diff --git a/codex-rs/chatgpt/src/apply_command.rs b/codex-rs/chatgpt/src/apply_command.rs index ea7c69d572..e1289c213c 100644 --- a/codex-rs/chatgpt/src/apply_command.rs +++ b/codex-rs/chatgpt/src/apply_command.rs @@ -32,7 +32,8 @@ pub async fn run_apply_command( ) .await?; - init_chatgpt_token_from_auth(&config.codex_home).await?; + init_chatgpt_token_from_auth(&config.codex_home, config.cli_auth_credentials_store_mode) + .await?; let task_response = get_task(&config, apply_cli.task_id).await?; apply_diff_from_task(task_response, cwd).await diff --git a/codex-rs/chatgpt/src/chatgpt_client.rs b/codex-rs/chatgpt/src/chatgpt_client.rs index d450f7c59e..7528631981 100644 --- a/codex-rs/chatgpt/src/chatgpt_client.rs +++ b/codex-rs/chatgpt/src/chatgpt_client.rs @@ -13,7 +13,8 @@ pub(crate) async fn chatgpt_get_request( path: String, ) -> anyhow::Result { let chatgpt_base_url = &config.chatgpt_base_url; - init_chatgpt_token_from_auth(&config.codex_home).await?; + init_chatgpt_token_from_auth(&config.codex_home, config.cli_auth_credentials_store_mode) + .await?; // Make direct HTTP request to ChatGPT backend API with the token let client = create_client(); diff --git a/codex-rs/chatgpt/src/chatgpt_token.rs b/codex-rs/chatgpt/src/chatgpt_token.rs index 70c6940f76..e8879ad21c 100644 --- a/codex-rs/chatgpt/src/chatgpt_token.rs +++ b/codex-rs/chatgpt/src/chatgpt_token.rs @@ -3,6 +3,7 @@ use std::path::Path; use std::sync::LazyLock; use std::sync::RwLock; +use codex_core::auth::AuthCredentialsStoreMode; use codex_core::token_data::TokenData; static CHATGPT_TOKEN: LazyLock>> = LazyLock::new(|| RwLock::new(None)); @@ -18,8 +19,11 @@ pub fn set_chatgpt_token_data(value: TokenData) { } /// Initialize the ChatGPT token from auth.json file -pub async fn init_chatgpt_token_from_auth(codex_home: &Path) -> std::io::Result<()> { - let auth = CodexAuth::from_auth_storage(codex_home)?; +pub async fn init_chatgpt_token_from_auth( + codex_home: &Path, + auth_credentials_store_mode: AuthCredentialsStoreMode, +) -> std::io::Result<()> { + let auth = CodexAuth::from_auth_storage(codex_home, auth_credentials_store_mode)?; if let Some(auth) = auth { let token_data = auth.get_token_data().await?; set_chatgpt_token_data(token_data); diff --git a/codex-rs/cli/src/login.rs b/codex-rs/cli/src/login.rs index bbb58f226a..6681ab20c8 100644 --- a/codex-rs/cli/src/login.rs +++ b/codex-rs/cli/src/login.rs @@ -1,6 +1,7 @@ use codex_app_server_protocol::AuthMode; use codex_common::CliConfigOverrides; use codex_core::CodexAuth; +use codex_core::auth::AuthCredentialsStoreMode; use codex_core::auth::CLIENT_ID; use codex_core::auth::login_with_api_key; use codex_core::auth::logout; @@ -17,11 +18,13 @@ use std::path::PathBuf; pub async fn login_with_chatgpt( codex_home: PathBuf, forced_chatgpt_workspace_id: Option, + cli_auth_credentials_store_mode: AuthCredentialsStoreMode, ) -> std::io::Result<()> { let opts = ServerOptions::new( codex_home, CLIENT_ID.to_string(), forced_chatgpt_workspace_id, + cli_auth_credentials_store_mode, ); let server = run_login_server(opts)?; @@ -43,7 +46,13 @@ pub async fn run_login_with_chatgpt(cli_config_overrides: CliConfigOverrides) -> let forced_chatgpt_workspace_id = config.forced_chatgpt_workspace_id.clone(); - match login_with_chatgpt(config.codex_home, forced_chatgpt_workspace_id).await { + match login_with_chatgpt( + config.codex_home, + forced_chatgpt_workspace_id, + config.cli_auth_credentials_store_mode, + ) + .await + { Ok(_) => { eprintln!("Successfully logged in"); std::process::exit(0); @@ -66,7 +75,11 @@ pub async fn run_login_with_api_key( std::process::exit(1); } - match login_with_api_key(&config.codex_home, &api_key) { + match login_with_api_key( + &config.codex_home, + &api_key, + config.cli_auth_credentials_store_mode, + ) { Ok(_) => { eprintln!("Successfully logged in"); std::process::exit(0); @@ -121,6 +134,7 @@ pub async fn run_login_with_device_code( config.codex_home, client_id.unwrap_or(CLIENT_ID.to_string()), forced_chatgpt_workspace_id, + config.cli_auth_credentials_store_mode, ); if let Some(iss) = issuer_base_url { opts.issuer = iss; @@ -140,7 +154,7 @@ pub async fn run_login_with_device_code( pub async fn run_login_status(cli_config_overrides: CliConfigOverrides) -> ! { let config = load_config_or_exit(cli_config_overrides).await; - match CodexAuth::from_auth_storage(&config.codex_home) { + match CodexAuth::from_auth_storage(&config.codex_home, config.cli_auth_credentials_store_mode) { Ok(Some(auth)) => match auth.mode { AuthMode::ApiKey => match auth.get_token().await { Ok(api_key) => { @@ -171,7 +185,7 @@ pub async fn run_login_status(cli_config_overrides: CliConfigOverrides) -> ! { pub async fn run_logout(cli_config_overrides: CliConfigOverrides) -> ! { let config = load_config_or_exit(cli_config_overrides).await; - match logout(&config.codex_home) { + match logout(&config.codex_home, config.cli_auth_credentials_store_mode) { Ok(true) => { eprintln!("Successfully logged out"); std::process::exit(0); diff --git a/codex-rs/cloud-tasks/src/lib.rs b/codex-rs/cloud-tasks/src/lib.rs index 6087cbea5b..a9bf0a8e19 100644 --- a/codex-rs/cloud-tasks/src/lib.rs +++ b/codex-rs/cloud-tasks/src/lib.rs @@ -58,7 +58,16 @@ async fn init_backend(user_agent_suffix: &str) -> anyhow::Result let auth = match codex_core::config::find_codex_home() .ok() - .map(|home| codex_login::AuthManager::new(home, false)) + .map(|home| { + let store_mode = codex_core::config::Config::load_from_base_config_with_overrides( + codex_core::config::ConfigToml::default(), + codex_core::config::ConfigOverrides::default(), + home.clone(), + ) + .map(|cfg| cfg.cli_auth_credentials_store_mode) + .unwrap_or_default(); + codex_login::AuthManager::new(home, false, store_mode) + }) .and_then(|am| am.auth()) { Some(auth) => auth, diff --git a/codex-rs/cloud-tasks/src/util.rs b/codex-rs/cloud-tasks/src/util.rs index 5d160e54fa..ff17265f3d 100644 --- a/codex-rs/cloud-tasks/src/util.rs +++ b/codex-rs/cloud-tasks/src/util.rs @@ -70,7 +70,14 @@ pub async fn build_chatgpt_headers() -> HeaderMap { HeaderValue::from_str(&ua).unwrap_or(HeaderValue::from_static("codex-cli")), ); if let Ok(home) = codex_core::config::find_codex_home() { - let am = codex_login::AuthManager::new(home, false); + let store_mode = codex_core::config::Config::load_from_base_config_with_overrides( + codex_core::config::ConfigToml::default(), + codex_core::config::ConfigOverrides::default(), + home.clone(), + ) + .map(|cfg| cfg.cli_auth_credentials_store_mode) + .unwrap_or_default(); + let am = codex_login::AuthManager::new(home, false, store_mode); if let Some(auth) = am.auth() && let Ok(tok) = auth.get_token().await && !tok.is_empty() diff --git a/codex-rs/common/src/approval_presets.rs b/codex-rs/common/src/approval_presets.rs index 6dc0cff0ee..6c3bf395ad 100644 --- a/codex-rs/common/src/approval_presets.rs +++ b/codex-rs/common/src/approval_presets.rs @@ -24,21 +24,21 @@ pub fn builtin_approval_presets() -> Vec { ApprovalPreset { id: "read-only", label: "Read Only", - description: "Codex can read files and answer questions. Codex requires approval to make edits, run commands, or access network", + description: "Codex can read files and answer questions. Codex requires approval to make edits, run commands, or access network.", approval: AskForApproval::OnRequest, sandbox: SandboxPolicy::ReadOnly, }, ApprovalPreset { id: "auto", label: "Auto", - description: "Codex can read files, make edits, and run commands in the workspace. Codex requires approval to work outside the workspace or access network", + description: "Codex can read files, make edits, and run commands in the workspace. Codex requires approval to work outside the workspace or access network.", approval: AskForApproval::OnRequest, sandbox: SandboxPolicy::new_workspace_write_policy(), }, ApprovalPreset { id: "full-access", label: "Full Access", - description: "Codex can read files, make edits, and run commands with network access, without approval. Exercise caution", + description: "Codex can read files, make edits, and run commands with network access, without approval. Exercise caution.", approval: AskForApproval::Never, sandbox: SandboxPolicy::DangerFullAccess, }, diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 1962411b6e..3fc6ccd093 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -39,7 +39,12 @@ eventsource-stream = { workspace = true } futures = { workspace = true } http = { workspace = true } indexmap = { workspace = true } -keyring = { workspace = true } +keyring = { workspace = true, features = [ + "apple-native", + "crypto-rust", + "linux-native-async-persistent", + "windows-native", +] } libc = { workspace = true } mcp-types = { workspace = true } os_info = { workspace = true } diff --git a/codex-rs/core/src/auth.rs b/codex-rs/core/src/auth.rs index 49f9b77815..85fb172192 100644 --- a/codex-rs/core/src/auth.rs +++ b/codex-rs/core/src/auth.rs @@ -79,8 +79,11 @@ impl CodexAuth { } /// Loads the available auth information from auth storage. - pub fn from_auth_storage(codex_home: &Path) -> std::io::Result> { - load_auth(codex_home, false) + pub fn from_auth_storage( + codex_home: &Path, + auth_credentials_store_mode: AuthCredentialsStoreMode, + ) -> std::io::Result> { + load_auth(codex_home, false, auth_credentials_store_mode) } pub async fn get_token_data(&self) -> Result { @@ -217,36 +220,55 @@ pub fn read_codex_api_key_from_env() -> Option { /// Delete the auth.json file inside `codex_home` if it exists. Returns `Ok(true)` /// if a file was removed, `Ok(false)` if no auth file was present. -pub fn logout(codex_home: &Path) -> std::io::Result { - let storage = create_auth_storage(codex_home.to_path_buf(), AuthCredentialsStoreMode::File); +pub fn logout( + codex_home: &Path, + auth_credentials_store_mode: AuthCredentialsStoreMode, +) -> std::io::Result { + let storage = create_auth_storage(codex_home.to_path_buf(), auth_credentials_store_mode); storage.delete() } /// Writes an `auth.json` that contains only the API key. -pub fn login_with_api_key(codex_home: &Path, api_key: &str) -> std::io::Result<()> { +pub fn login_with_api_key( + codex_home: &Path, + api_key: &str, + auth_credentials_store_mode: AuthCredentialsStoreMode, +) -> std::io::Result<()> { let auth_dot_json = AuthDotJson { openai_api_key: Some(api_key.to_string()), tokens: None, last_refresh: None, }; - save_auth(codex_home, &auth_dot_json) + save_auth(codex_home, &auth_dot_json, auth_credentials_store_mode) } /// Persist the provided auth payload using the specified backend. -pub fn save_auth(codex_home: &Path, auth: &AuthDotJson) -> std::io::Result<()> { - let storage = create_auth_storage(codex_home.to_path_buf(), AuthCredentialsStoreMode::File); +pub fn save_auth( + codex_home: &Path, + auth: &AuthDotJson, + auth_credentials_store_mode: AuthCredentialsStoreMode, +) -> std::io::Result<()> { + let storage = create_auth_storage(codex_home.to_path_buf(), auth_credentials_store_mode); storage.save(auth) } /// Load CLI auth data using the configured credential store backend. /// Returns `None` when no credentials are stored. -pub fn load_auth_dot_json(codex_home: &Path) -> std::io::Result> { - let storage = create_auth_storage(codex_home.to_path_buf(), AuthCredentialsStoreMode::File); +pub fn load_auth_dot_json( + codex_home: &Path, + auth_credentials_store_mode: AuthCredentialsStoreMode, +) -> std::io::Result> { + let storage = create_auth_storage(codex_home.to_path_buf(), auth_credentials_store_mode); storage.load() } pub async fn enforce_login_restrictions(config: &Config) -> std::io::Result<()> { - let Some(auth) = load_auth(&config.codex_home, true)? else { + let Some(auth) = load_auth( + &config.codex_home, + true, + config.cli_auth_credentials_store_mode, + )? + else { return Ok(()); }; @@ -265,7 +287,11 @@ pub async fn enforce_login_restrictions(config: &Config) -> std::io::Result<()> }; if let Some(message) = method_violation { - return logout_with_message(&config.codex_home, message); + return logout_with_message( + &config.codex_home, + message, + config.cli_auth_credentials_store_mode, + ); } } @@ -282,6 +308,7 @@ pub async fn enforce_login_restrictions(config: &Config) -> std::io::Result<()> format!( "Failed to load ChatGPT credentials while enforcing workspace restrictions: {err}. Logging out." ), + config.cli_auth_credentials_store_mode, ); } }; @@ -297,15 +324,23 @@ pub async fn enforce_login_restrictions(config: &Config) -> std::io::Result<()> "Login is restricted to workspace {expected_account_id}, but current credentials lack a workspace identifier. Logging out." ), }; - return logout_with_message(&config.codex_home, message); + return logout_with_message( + &config.codex_home, + message, + config.cli_auth_credentials_store_mode, + ); } } Ok(()) } -fn logout_with_message(codex_home: &Path, message: String) -> std::io::Result<()> { - match logout(codex_home) { +fn logout_with_message( + codex_home: &Path, + message: String, + auth_credentials_store_mode: AuthCredentialsStoreMode, +) -> std::io::Result<()> { + match logout(codex_home, auth_credentials_store_mode) { Ok(_) => Err(std::io::Error::other(message)), Err(err) => Err(std::io::Error::other(format!( "{message}. Failed to remove auth.json: {err}" @@ -316,6 +351,7 @@ fn logout_with_message(codex_home: &Path, message: String) -> std::io::Result<() fn load_auth( codex_home: &Path, enable_codex_api_key_env: bool, + auth_credentials_store_mode: AuthCredentialsStoreMode, ) -> std::io::Result> { if enable_codex_api_key_env && let Some(api_key) = read_codex_api_key_from_env() { let client = crate::default_client::create_client(); @@ -325,7 +361,7 @@ fn load_auth( ))); } - let storage = create_auth_storage(codex_home.to_path_buf(), AuthCredentialsStoreMode::File); + let storage = create_auth_storage(codex_home.to_path_buf(), auth_credentials_store_mode); let client = crate::default_client::create_client(); let auth_dot_json = match storage.load()? { @@ -512,7 +548,8 @@ mod tests { ) .unwrap(); - super::login_with_api_key(dir.path(), "sk-new").expect("login_with_api_key should succeed"); + super::login_with_api_key(dir.path(), "sk-new", AuthCredentialsStoreMode::File) + .expect("login_with_api_key should succeed"); let storage = FileAuthStorage::new(dir.path().to_path_buf()); let auth = storage @@ -525,7 +562,8 @@ mod tests { #[test] fn missing_auth_json_returns_none() { let dir = tempdir().unwrap(); - let auth = CodexAuth::from_auth_storage(dir.path()).expect("call should succeed"); + let auth = CodexAuth::from_auth_storage(dir.path(), AuthCredentialsStoreMode::File) + .expect("call should succeed"); assert_eq!(auth, None); } @@ -549,7 +587,9 @@ mod tests { auth_dot_json, storage: _, .. - } = super::load_auth(codex_home.path(), false).unwrap().unwrap(); + } = super::load_auth(codex_home.path(), false, AuthCredentialsStoreMode::File) + .unwrap() + .unwrap(); assert_eq!(None, api_key); assert_eq!(AuthMode::ChatGPT, mode); @@ -590,7 +630,9 @@ mod tests { ) .unwrap(); - let auth = super::load_auth(dir.path(), false).unwrap().unwrap(); + let auth = super::load_auth(dir.path(), false, AuthCredentialsStoreMode::File) + .unwrap() + .unwrap(); assert_eq!(auth.mode, AuthMode::ApiKey); assert_eq!(auth.api_key, Some("sk-test-key".to_string())); @@ -605,10 +647,10 @@ mod tests { tokens: None, last_refresh: None, }; - super::save_auth(dir.path(), &auth_dot_json)?; + super::save_auth(dir.path(), &auth_dot_json, AuthCredentialsStoreMode::File)?; let auth_file = get_auth_file(dir.path()); assert!(auth_file.exists()); - assert!(logout(dir.path())?); + assert!(logout(dir.path(), AuthCredentialsStoreMode::File)?); assert!(!auth_file.exists()); Ok(()) } @@ -717,7 +759,8 @@ mod tests { #[tokio::test] async fn enforce_login_restrictions_logs_out_for_method_mismatch() { let codex_home = tempdir().unwrap(); - login_with_api_key(codex_home.path(), "sk-test").expect("seed api key"); + login_with_api_key(codex_home.path(), "sk-test", AuthCredentialsStoreMode::File) + .expect("seed api key"); let config = build_config(codex_home.path(), Some(ForcedLoginMethod::Chatgpt), None); @@ -786,7 +829,8 @@ mod tests { async fn enforce_login_restrictions_allows_api_key_if_login_method_not_set_but_forced_chatgpt_workspace_id_is_set() { let codex_home = tempdir().unwrap(); - login_with_api_key(codex_home.path(), "sk-test").expect("seed api key"); + login_with_api_key(codex_home.path(), "sk-test", AuthCredentialsStoreMode::File) + .expect("seed api key"); let config = build_config(codex_home.path(), None, Some("org_mine".to_string())); @@ -830,6 +874,7 @@ pub struct AuthManager { codex_home: PathBuf, inner: RwLock, enable_codex_api_key_env: bool, + auth_credentials_store_mode: AuthCredentialsStoreMode, } impl AuthManager { @@ -837,14 +882,23 @@ impl AuthManager { /// preferred auth method. Errors loading auth are swallowed; `auth()` will /// simply return `None` in that case so callers can treat it as an /// unauthenticated state. - pub fn new(codex_home: PathBuf, enable_codex_api_key_env: bool) -> Self { - let auth = load_auth(&codex_home, enable_codex_api_key_env) - .ok() - .flatten(); + pub fn new( + codex_home: PathBuf, + enable_codex_api_key_env: bool, + auth_credentials_store_mode: AuthCredentialsStoreMode, + ) -> Self { + let auth = load_auth( + &codex_home, + enable_codex_api_key_env, + auth_credentials_store_mode, + ) + .ok() + .flatten(); Self { codex_home, inner: RwLock::new(CachedAuth { auth }), enable_codex_api_key_env, + auth_credentials_store_mode, } } @@ -855,6 +909,7 @@ impl AuthManager { codex_home: PathBuf::new(), inner: RwLock::new(cached), enable_codex_api_key_env: false, + auth_credentials_store_mode: AuthCredentialsStoreMode::File, }) } @@ -866,9 +921,13 @@ impl AuthManager { /// Force a reload of the auth information from auth.json. Returns /// whether the auth value changed. pub fn reload(&self) -> bool { - let new_auth = load_auth(&self.codex_home, self.enable_codex_api_key_env) - .ok() - .flatten(); + let new_auth = load_auth( + &self.codex_home, + self.enable_codex_api_key_env, + self.auth_credentials_store_mode, + ) + .ok() + .flatten(); if let Ok(mut guard) = self.inner.write() { let changed = !AuthManager::auths_equal(&guard.auth, &new_auth); guard.auth = new_auth; @@ -887,8 +946,16 @@ impl AuthManager { } /// Convenience constructor returning an `Arc` wrapper. - pub fn shared(codex_home: PathBuf, enable_codex_api_key_env: bool) -> Arc { - Arc::new(Self::new(codex_home, enable_codex_api_key_env)) + pub fn shared( + codex_home: PathBuf, + enable_codex_api_key_env: bool, + auth_credentials_store_mode: AuthCredentialsStoreMode, + ) -> Arc { + Arc::new(Self::new( + codex_home, + enable_codex_api_key_env, + auth_credentials_store_mode, + )) } /// Attempt to refresh the current auth token (if any). On success, reload @@ -916,7 +983,7 @@ impl AuthManager { /// reloads the in‑memory auth cache so callers immediately observe the /// unauthenticated state. pub fn logout(&self) -> std::io::Result { - let removed = super::auth::logout(&self.codex_home)?; + let removed = super::auth::logout(&self.codex_home, self.auth_credentials_store_mode)?; // Always reload to clear any cached auth (even if file absent). self.reload(); Ok(removed) diff --git a/codex-rs/core/src/chat_completions.rs b/codex-rs/core/src/chat_completions.rs index 76cbdf9f0a..1fb2230d79 100644 --- a/codex-rs/core/src/chat_completions.rs +++ b/codex-rs/core/src/chat_completions.rs @@ -17,6 +17,7 @@ use crate::util::backoff; use bytes::Bytes; use codex_otel::otel_event_manager::OtelEventManager; use codex_protocol::models::ContentItem; +use codex_protocol::models::FunctionCallOutputContentItem; use codex_protocol::models::ReasoningItemContent; use codex_protocol::models::ResponseItem; use eventsource_stream::Eventsource; @@ -159,16 +160,26 @@ pub(crate) async fn stream_chat_completions( for (idx, item) in input.iter().enumerate() { match item { ResponseItem::Message { role, content, .. } => { + // Build content either as a plain string (typical for assistant text) + // or as an array of content items when images are present (user/tool multimodal). let mut text = String::new(); + let mut items: Vec = Vec::new(); + let mut saw_image = false; + for c in content { match c { ContentItem::InputText { text: t } | ContentItem::OutputText { text: t } => { text.push_str(t); + items.push(json!({"type":"text","text": t})); + } + ContentItem::InputImage { image_url } => { + saw_image = true; + items.push(json!({"type":"image_url","image_url": {"url": image_url}})); } - _ => {} } } + // Skip exact-duplicate assistant messages. if role == "assistant" { if let Some(prev) = &last_assistant_text @@ -179,7 +190,17 @@ pub(crate) async fn stream_chat_completions( last_assistant_text = Some(text.clone()); } - let mut msg = json!({"role": role, "content": text}); + // For assistant messages, always send a plain string for compatibility. + // For user messages, if an image is present, send an array of content items. + let content_value = if role == "assistant" { + json!(text) + } else if saw_image { + json!(items) + } else { + json!(text) + }; + + let mut msg = json!({"role": role, "content": content_value}); if role == "assistant" && let Some(reasoning) = reasoning_by_anchor_index.get(&idx) && let Some(obj) = msg.as_object_mut() @@ -238,10 +259,29 @@ pub(crate) async fn stream_chat_completions( messages.push(msg); } ResponseItem::FunctionCallOutput { call_id, output } => { + // Prefer structured content items when available (e.g., images) + // otherwise fall back to the legacy plain-string content. + let content_value = if let Some(items) = &output.content_items { + let mapped: Vec = items + .iter() + .map(|it| match it { + FunctionCallOutputContentItem::InputText { text } => { + json!({"type":"text","text": text}) + } + FunctionCallOutputContentItem::InputImage { image_url } => { + json!({"type":"image_url","image_url": {"url": image_url}}) + } + }) + .collect(); + json!(mapped) + } else { + json!(output.content) + }; + messages.push(json!({ "role": "tool", "tool_call_id": call_id, - "content": output.content, + "content": content_value, })); } ResponseItem::CustomToolCall { diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index f811211c4f..47d80c5cb4 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -82,7 +82,6 @@ use crate::protocol::ErrorEvent; use crate::protocol::Event; use crate::protocol::EventMsg; use crate::protocol::ExecApprovalRequestEvent; -use crate::protocol::ListCustomPromptsResponseEvent; use crate::protocol::Op; use crate::protocol::RateLimitSnapshot; use crate::protocol::ReviewDecision; @@ -103,13 +102,10 @@ use crate::state::ActiveTurn; use crate::state::SessionServices; use crate::state::SessionState; use crate::state::TaskKind; -use crate::tasks::CompactTask; use crate::tasks::GhostSnapshotTask; -use crate::tasks::RegularTask; use crate::tasks::ReviewTask; use crate::tasks::SessionTask; use crate::tasks::SessionTaskContext; -use crate::tasks::UndoTask; use crate::tools::ToolRouter; use crate::tools::context::SharedTurnDiffTracker; use crate::tools::parallel::ToolCallRuntime; @@ -125,7 +121,6 @@ use codex_async_utils::OrCancelExt; use codex_otel::otel_event_manager::OtelEventManager; use codex_protocol::config_types::ReasoningEffort as ReasoningEffortConfig; use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig; -use codex_protocol::custom_prompts::CustomPrompt; use codex_protocol::models::ContentItem; use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseInputItem; @@ -1243,9 +1238,9 @@ async fn submission_loop(sess: Arc, config: Arc, rx_sub: Receiv // To break out of this loop, send Op::Shutdown. while let Ok(sub) = rx_sub.recv().await { debug!(?sub, "Submission"); - match sub.op { + match sub.op.clone() { Op::Interrupt => { - sess.interrupt_task().await; + handlers::interrupt(&sess).await; } Op::OverrideTurnContext { cwd, @@ -1255,246 +1250,336 @@ async fn submission_loop(sess: Arc, config: Arc, rx_sub: Receiv effort, summary, } => { - let updates = SessionSettingsUpdate { - cwd, - approval_policy, - sandbox_policy, - model, - reasoning_effort: effort, - reasoning_summary: summary, - ..Default::default() - }; - sess.update_settings(updates).await; - } - - Op::UserInput { .. } | Op::UserTurn { .. } => { - let (items, updates) = match sub.op { - Op::UserTurn { + handlers::override_turn_context( + &sess, + SessionSettingsUpdate { cwd, approval_policy, sandbox_policy, model, - effort, - summary, - final_output_json_schema, - items, - } => ( - items, - SessionSettingsUpdate { - cwd: Some(cwd), - approval_policy: Some(approval_policy), - sandbox_policy: Some(sandbox_policy), - model: Some(model), - reasoning_effort: Some(effort), - reasoning_summary: Some(summary), - final_output_json_schema: Some(final_output_json_schema), - }, - ), - Op::UserInput { items } => (items, SessionSettingsUpdate::default()), - _ => unreachable!(), - }; - let current_context = sess.new_turn_with_sub_id(sub.id.clone(), updates).await; - current_context - .client - .get_otel_event_manager() - .user_prompt(&items); - // attempt to inject input into current task - if let Err(items) = sess.inject_input(items).await { - if let Some(env_item) = sess - .build_environment_update_item(previous_context.as_ref(), ¤t_context) - { - sess.record_conversation_items( - ¤t_context, - std::slice::from_ref(&env_item), - ) - .await; - } - - sess.spawn_task(Arc::clone(¤t_context), items, RegularTask) - .await; - previous_context = Some(current_context); - } - } - Op::ExecApproval { id, decision } => match decision { - ReviewDecision::Abort => { - sess.interrupt_task().await; - } - other => sess.notify_approval(&id, other).await, - }, - Op::PatchApproval { id, decision } => match decision { - ReviewDecision::Abort => { - sess.interrupt_task().await; - } - other => sess.notify_approval(&id, other).await, - }, - Op::AddToHistory { text } => { - let id = sess.conversation_id; - let config = config.clone(); - tokio::spawn(async move { - if let Err(e) = crate::message_history::append_entry(&text, &id, &config).await - { - warn!("failed to append to message history: {e}"); - } - }); - } - - Op::GetHistoryEntryRequest { offset, log_id } => { - let config = config.clone(); - let sess_clone = sess.clone(); - let sub_id = sub.id.clone(); - - tokio::spawn(async move { - // Run lookup in blocking thread because it does file IO + locking. - let entry_opt = tokio::task::spawn_blocking(move || { - crate::message_history::lookup(log_id, offset, &config) - }) - .await - .unwrap_or(None); - - let event = Event { - id: sub_id, - msg: EventMsg::GetHistoryEntryResponse( - crate::protocol::GetHistoryEntryResponseEvent { - offset, - log_id, - entry: entry_opt.map(|e| { - codex_protocol::message_history::HistoryEntry { - conversation_id: e.session_id, - ts: e.ts, - text: e.text, - } - }), - }, - ), - }; - - sess_clone.send_event_raw(event).await; - }); - } - Op::ListMcpTools => { - let sub_id = sub.id.clone(); - - // This is a cheap lookup from the connection manager's cache. - let tools = sess.services.mcp_connection_manager.list_all_tools(); - let (auth_status_entries, resources, resource_templates) = tokio::join!( - compute_auth_statuses( - config.mcp_servers.iter(), - config.mcp_oauth_credentials_store_mode, - ), - sess.services.mcp_connection_manager.list_all_resources(), - sess.services - .mcp_connection_manager - .list_all_resource_templates() - ); - let auth_statuses = auth_status_entries - .iter() - .map(|(name, entry)| (name.clone(), entry.auth_status)) - .collect(); - let event = Event { - id: sub_id, - msg: EventMsg::McpListToolsResponse( - crate::protocol::McpListToolsResponseEvent { - tools, - resources, - resource_templates, - auth_statuses, - }, - ), - }; - sess.send_event_raw(event).await; - } - Op::ListCustomPrompts => { - let sub_id = sub.id.clone(); - - let custom_prompts: Vec = - if let Some(dir) = crate::custom_prompts::default_prompts_dir() { - crate::custom_prompts::discover_prompts_in(&dir).await - } else { - Vec::new() - }; - - let event = Event { - id: sub_id, - msg: EventMsg::ListCustomPromptsResponse(ListCustomPromptsResponseEvent { - custom_prompts, - }), - }; - sess.send_event_raw(event).await; - } - Op::Undo => { - let turn_context = sess - .new_turn_with_sub_id(sub.id.clone(), SessionSettingsUpdate::default()) - .await; - sess.spawn_task(turn_context, Vec::new(), UndoTask::new()) - .await; - } - Op::Compact => { - let turn_context = sess - .new_turn_with_sub_id(sub.id.clone(), SessionSettingsUpdate::default()) - .await; - // Attempt to inject input into current task - if let Err(items) = sess - .inject_input(vec![UserInput::Text { - text: compact::SUMMARIZATION_PROMPT.to_string(), - }]) - .await - { - sess.spawn_task(Arc::clone(&turn_context), items, CompactTask) - .await; - } - } - Op::Shutdown => { - sess.abort_all_tasks(TurnAbortReason::Interrupted).await; - info!("Shutting down Codex instance"); - - // Gracefully flush and shutdown rollout recorder on session end so tests - // that inspect the rollout file do not race with the background writer. - let recorder_opt = { - let mut guard = sess.services.rollout.lock().await; - guard.take() - }; - if let Some(rec) = recorder_opt - && let Err(e) = rec.shutdown().await - { - warn!("failed to shutdown rollout recorder: {e}"); - let event = Event { - id: sub.id.clone(), - msg: EventMsg::Error(ErrorEvent { - message: "Failed to shutdown rollout recorder".to_string(), - }), - }; - sess.send_event_raw(event).await; - } - - let event = Event { - id: sub.id.clone(), - msg: EventMsg::ShutdownComplete, - }; - sess.send_event_raw(event).await; - break; - } - - Op::Review { review_request } => { - let turn_context = sess - .new_turn_with_sub_id(sub.id.clone(), SessionSettingsUpdate::default()) - .await; - spawn_review_thread( - sess.clone(), - config.clone(), - turn_context.clone(), - sub.id, - review_request, + reasoning_effort: effort, + reasoning_summary: summary, + ..Default::default() + }, ) .await; } - _ => { - // Ignore unknown ops; enum is non_exhaustive to allow extensions. + Op::UserInput { .. } | Op::UserTurn { .. } => { + handlers::user_input_or_turn(&sess, sub.id.clone(), sub.op, &mut previous_context) + .await; } + Op::ExecApproval { id, decision } => { + handlers::exec_approval(&sess, id, decision).await; + } + Op::PatchApproval { id, decision } => { + handlers::patch_approval(&sess, id, decision).await; + } + Op::AddToHistory { text } => { + handlers::add_to_history(&sess, &config, text).await; + } + Op::GetHistoryEntryRequest { offset, log_id } => { + handlers::get_history_entry_request(&sess, &config, sub.id.clone(), offset, log_id) + .await; + } + Op::ListMcpTools => { + handlers::list_mcp_tools(&sess, &config, sub.id.clone()).await; + } + Op::ListCustomPrompts => { + handlers::list_custom_prompts(&sess, sub.id.clone()).await; + } + Op::Undo => { + handlers::undo(&sess, sub.id.clone()).await; + } + Op::Compact => { + handlers::compact(&sess, sub.id.clone()).await; + } + Op::Shutdown => { + if handlers::shutdown(&sess, sub.id.clone()).await { + break; + } + } + Op::Review { review_request } => { + handlers::review(&sess, &config, sub.id.clone(), review_request).await; + } + _ => {} // Ignore unknown ops; enum is non_exhaustive to allow extensions. } } debug!("Agent loop exited"); } +/// Operation handlers +mod handlers { + use crate::codex::Session; + use crate::codex::SessionSettingsUpdate; + use crate::codex::TurnContext; + use crate::codex::compact; + use crate::codex::spawn_review_thread; + use crate::config::Config; + use crate::mcp::auth::compute_auth_statuses; + use crate::tasks::CompactTask; + use crate::tasks::RegularTask; + use crate::tasks::UndoTask; + use codex_protocol::custom_prompts::CustomPrompt; + use codex_protocol::protocol::ErrorEvent; + use codex_protocol::protocol::Event; + use codex_protocol::protocol::EventMsg; + use codex_protocol::protocol::ListCustomPromptsResponseEvent; + use codex_protocol::protocol::Op; + use codex_protocol::protocol::ReviewDecision; + use codex_protocol::protocol::ReviewRequest; + use codex_protocol::protocol::TurnAbortReason; + use codex_protocol::user_input::UserInput; + use std::sync::Arc; + use tracing::info; + use tracing::warn; + + pub async fn interrupt(sess: &Arc) { + sess.interrupt_task().await; + } + + pub async fn override_turn_context(sess: &Session, updates: SessionSettingsUpdate) { + sess.update_settings(updates).await; + } + + pub async fn user_input_or_turn( + sess: &Arc, + sub_id: String, + op: Op, + previous_context: &mut Option>, + ) { + let (items, updates) = match op { + Op::UserTurn { + cwd, + approval_policy, + sandbox_policy, + model, + effort, + summary, + final_output_json_schema, + items, + } => ( + items, + SessionSettingsUpdate { + cwd: Some(cwd), + approval_policy: Some(approval_policy), + sandbox_policy: Some(sandbox_policy), + model: Some(model), + reasoning_effort: Some(effort), + reasoning_summary: Some(summary), + final_output_json_schema: Some(final_output_json_schema), + }, + ), + Op::UserInput { items } => (items, SessionSettingsUpdate::default()), + _ => unreachable!(), + }; + + let current_context = sess.new_turn_with_sub_id(sub_id, updates).await; + current_context + .client + .get_otel_event_manager() + .user_prompt(&items); + + // Attempt to inject input into current task + if let Err(items) = sess.inject_input(items).await { + if let Some(env_item) = + sess.build_environment_update_item(previous_context.as_ref(), ¤t_context) + { + sess.record_conversation_items(¤t_context, std::slice::from_ref(&env_item)) + .await; + } + + sess.spawn_task(Arc::clone(¤t_context), items, RegularTask) + .await; + *previous_context = Some(current_context); + } + } + + pub async fn exec_approval(sess: &Arc, id: String, decision: ReviewDecision) { + match decision { + ReviewDecision::Abort => { + sess.interrupt_task().await; + } + other => sess.notify_approval(&id, other).await, + } + } + + pub async fn patch_approval(sess: &Arc, id: String, decision: ReviewDecision) { + match decision { + ReviewDecision::Abort => { + sess.interrupt_task().await; + } + other => sess.notify_approval(&id, other).await, + } + } + + pub async fn add_to_history(sess: &Arc, config: &Arc, text: String) { + let id = sess.conversation_id; + let config = Arc::clone(config); + tokio::spawn(async move { + if let Err(e) = crate::message_history::append_entry(&text, &id, &config).await { + warn!("failed to append to message history: {e}"); + } + }); + } + + pub async fn get_history_entry_request( + sess: &Arc, + config: &Arc, + sub_id: String, + offset: usize, + log_id: u64, + ) { + let config = Arc::clone(config); + let sess_clone = Arc::clone(sess); + + tokio::spawn(async move { + // Run lookup in blocking thread because it does file IO + locking. + let entry_opt = tokio::task::spawn_blocking(move || { + crate::message_history::lookup(log_id, offset, &config) + }) + .await + .unwrap_or(None); + + let event = Event { + id: sub_id, + msg: EventMsg::GetHistoryEntryResponse( + crate::protocol::GetHistoryEntryResponseEvent { + offset, + log_id, + entry: entry_opt.map(|e| codex_protocol::message_history::HistoryEntry { + conversation_id: e.session_id, + ts: e.ts, + text: e.text, + }), + }, + ), + }; + + sess_clone.send_event_raw(event).await; + }); + } + + pub async fn list_mcp_tools(sess: &Session, config: &Arc, sub_id: String) { + // This is a cheap lookup from the connection manager's cache. + let tools = sess.services.mcp_connection_manager.list_all_tools(); + let (auth_status_entries, resources, resource_templates) = tokio::join!( + compute_auth_statuses( + config.mcp_servers.iter(), + config.mcp_oauth_credentials_store_mode, + ), + sess.services.mcp_connection_manager.list_all_resources(), + sess.services + .mcp_connection_manager + .list_all_resource_templates() + ); + let auth_statuses = auth_status_entries + .iter() + .map(|(name, entry)| (name.clone(), entry.auth_status)) + .collect(); + let event = Event { + id: sub_id, + msg: EventMsg::McpListToolsResponse(crate::protocol::McpListToolsResponseEvent { + tools, + resources, + resource_templates, + auth_statuses, + }), + }; + sess.send_event_raw(event).await; + } + + pub async fn list_custom_prompts(sess: &Session, sub_id: String) { + let custom_prompts: Vec = + if let Some(dir) = crate::custom_prompts::default_prompts_dir() { + crate::custom_prompts::discover_prompts_in(&dir).await + } else { + Vec::new() + }; + + let event = Event { + id: sub_id, + msg: EventMsg::ListCustomPromptsResponse(ListCustomPromptsResponseEvent { + custom_prompts, + }), + }; + sess.send_event_raw(event).await; + } + + pub async fn undo(sess: &Arc, sub_id: String) { + let turn_context = sess + .new_turn_with_sub_id(sub_id, SessionSettingsUpdate::default()) + .await; + sess.spawn_task(turn_context, Vec::new(), UndoTask::new()) + .await; + } + + pub async fn compact(sess: &Arc, sub_id: String) { + let turn_context = sess + .new_turn_with_sub_id(sub_id, SessionSettingsUpdate::default()) + .await; + // Attempt to inject input into current task + if let Err(items) = sess + .inject_input(vec![UserInput::Text { + text: compact::SUMMARIZATION_PROMPT.to_string(), + }]) + .await + { + sess.spawn_task(Arc::clone(&turn_context), items, CompactTask) + .await; + } + } + + pub async fn shutdown(sess: &Arc, sub_id: String) -> bool { + sess.abort_all_tasks(TurnAbortReason::Interrupted).await; + info!("Shutting down Codex instance"); + + // Gracefully flush and shutdown rollout recorder on session end so tests + // that inspect the rollout file do not race with the background writer. + let recorder_opt = { + let mut guard = sess.services.rollout.lock().await; + guard.take() + }; + if let Some(rec) = recorder_opt + && let Err(e) = rec.shutdown().await + { + warn!("failed to shutdown rollout recorder: {e}"); + let event = Event { + id: sub_id.clone(), + msg: EventMsg::Error(ErrorEvent { + message: "Failed to shutdown rollout recorder".to_string(), + }), + }; + sess.send_event_raw(event).await; + } + + let event = Event { + id: sub_id, + msg: EventMsg::ShutdownComplete, + }; + sess.send_event_raw(event).await; + true + } + + pub async fn review( + sess: &Arc, + config: &Arc, + sub_id: String, + review_request: ReviewRequest, + ) { + let turn_context = sess + .new_turn_with_sub_id(sub_id.clone(), SessionSettingsUpdate::default()) + .await; + spawn_review_thread( + Arc::clone(sess), + Arc::clone(config), + turn_context.clone(), + sub_id, + review_request, + ) + .await; + } +} + /// Spawn a review thread using the given prompt. async fn spawn_review_thread( sess: Arc, @@ -2047,7 +2132,7 @@ async fn try_run_turn( call_id: String::new(), output: FunctionCallOutputPayload { content: msg.to_string(), - success: None, + ..Default::default() }, }; add_completed(ProcessedResponseItem { @@ -2061,7 +2146,7 @@ async fn try_run_turn( call_id: String::new(), output: FunctionCallOutputPayload { content: message, - success: None, + ..Default::default() }, }; add_completed(ProcessedResponseItem { @@ -2199,41 +2284,6 @@ pub(super) fn get_last_assistant_message_from_turn(responses: &[ResponseItem]) - } }) } -pub(crate) fn convert_call_tool_result_to_function_call_output_payload( - call_tool_result: &CallToolResult, -) -> FunctionCallOutputPayload { - let CallToolResult { - content, - is_error, - structured_content, - } = call_tool_result; - - // In terms of what to send back to the model, we prefer structured_content, - // if available, and fallback to content, otherwise. - let mut is_success = is_error != &Some(true); - let content = if let Some(structured_content) = structured_content - && structured_content != &serde_json::Value::Null - && let Ok(serialized_structured_content) = serde_json::to_string(&structured_content) - { - serialized_structured_content - } else { - match serde_json::to_string(&content) { - Ok(serialized_content) => serialized_content, - Err(err) => { - // If we could not serialize either content or structured_content to - // JSON, flag this as an error. - is_success = false; - err.to_string() - } - } - }; - - FunctionCallOutputPayload { - content, - success: Some(is_success), - } -} - /// Emits an ExitedReviewMode Event with optional ReviewOutput, /// and records a developer message with the review output. pub(crate) async fn exit_review_mode( @@ -2366,10 +2416,6 @@ mod tests { use crate::state::TaskKind; use crate::tasks::SessionTask; use crate::tasks::SessionTaskContext; - use crate::tools::MODEL_FORMAT_HEAD_LINES; - use crate::tools::MODEL_FORMAT_MAX_BYTES; - use crate::tools::MODEL_FORMAT_MAX_LINES; - use crate::tools::MODEL_FORMAT_TAIL_LINES; use crate::tools::ToolRouter; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; @@ -2443,7 +2489,7 @@ mod tests { })), }; - let got = convert_call_tool_result_to_function_call_output_payload(&ctr); + let got = FunctionCallOutputPayload::from(&ctr); let expected = FunctionCallOutputPayload { content: serde_json::to_string(&json!({ "ok": true, @@ -2451,100 +2497,12 @@ mod tests { })) .unwrap(), success: Some(true), + ..Default::default() }; assert_eq!(expected, got); } - #[test] - fn model_truncation_head_tail_by_lines() { - // Build 400 short lines so line-count limit, not byte budget, triggers truncation - let lines: Vec = (1..=400).map(|i| format!("line{i}")).collect(); - let full = lines.join("\n"); - - let exec = ExecToolCallOutput { - exit_code: 0, - stdout: StreamOutput::new(String::new()), - stderr: StreamOutput::new(String::new()), - aggregated_output: StreamOutput::new(full), - duration: StdDuration::from_secs(1), - timed_out: false, - }; - - let out = format_exec_output_str(&exec); - - // Strip truncation header if present for subsequent assertions - let body = out - .strip_prefix("Total output lines: ") - .and_then(|rest| rest.split_once("\n\n").map(|x| x.1)) - .unwrap_or(out.as_str()); - - // Expect elision marker with correct counts - let omitted = 400 - MODEL_FORMAT_MAX_LINES; // 144 - let marker = format!("\n[... omitted {omitted} of 400 lines ...]\n\n"); - assert!(out.contains(&marker), "missing marker: {out}"); - - // Validate head and tail - let parts: Vec<&str> = body.split(&marker).collect(); - assert_eq!(parts.len(), 2, "expected one marker split"); - let head = parts[0]; - let tail = parts[1]; - - let expected_head: String = (1..=MODEL_FORMAT_HEAD_LINES) - .map(|i| format!("line{i}")) - .collect::>() - .join("\n"); - assert!(head.starts_with(&expected_head), "head mismatch"); - - let expected_tail: String = ((400 - MODEL_FORMAT_TAIL_LINES + 1)..=400) - .map(|i| format!("line{i}")) - .collect::>() - .join("\n"); - assert!(tail.ends_with(&expected_tail), "tail mismatch"); - } - - #[test] - fn model_truncation_respects_byte_budget() { - // Construct a large output (about 100kB) so byte budget dominates - let big_line = "x".repeat(100); - let full = std::iter::repeat_n(big_line, 1000) - .collect::>() - .join("\n"); - - let exec = ExecToolCallOutput { - exit_code: 0, - stdout: StreamOutput::new(String::new()), - stderr: StreamOutput::new(String::new()), - aggregated_output: StreamOutput::new(full.clone()), - duration: StdDuration::from_secs(1), - timed_out: false, - }; - - let out = format_exec_output_str(&exec); - // Keep strict budget on the truncated body (excluding header) - let body = out - .strip_prefix("Total output lines: ") - .and_then(|rest| rest.split_once("\n\n").map(|x| x.1)) - .unwrap_or(out.as_str()); - assert!(body.len() <= MODEL_FORMAT_MAX_BYTES, "exceeds byte budget"); - assert!(out.contains("omitted"), "should contain elision marker"); - - // Ensure head and tail are drawn from the original - assert!(full.starts_with(body.chars().take(8).collect::().as_str())); - assert!( - full.ends_with( - body.chars() - .rev() - .take(8) - .collect::() - .chars() - .rev() - .collect::() - .as_str() - ) - ); - } - #[test] fn includes_timed_out_message() { let exec = ExecToolCallOutput { @@ -2572,11 +2530,12 @@ mod tests { structured_content: Some(serde_json::Value::Null), }; - let got = convert_call_tool_result_to_function_call_output_payload(&ctr); + let got = FunctionCallOutputPayload::from(&ctr); let expected = FunctionCallOutputPayload { content: serde_json::to_string(&vec![text_block("hello"), text_block("world")]) .unwrap(), success: Some(true), + ..Default::default() }; assert_eq!(expected, got); @@ -2590,10 +2549,11 @@ mod tests { structured_content: Some(json!({ "message": "bad" })), }; - let got = convert_call_tool_result_to_function_call_output_payload(&ctr); + let got = FunctionCallOutputPayload::from(&ctr); let expected = FunctionCallOutputPayload { content: serde_json::to_string(&json!({ "message": "bad" })).unwrap(), success: Some(false), + ..Default::default() }; assert_eq!(expected, got); @@ -2607,10 +2567,11 @@ mod tests { structured_content: None, }; - let got = convert_call_tool_result_to_function_call_output_payload(&ctr); + let got = FunctionCallOutputPayload::from(&ctr); let expected = FunctionCallOutputPayload { content: serde_json::to_string(&vec![text_block("alpha")]).unwrap(), success: Some(true), + ..Default::default() }; assert_eq!(expected, got); @@ -2649,7 +2610,11 @@ mod tests { let config = Arc::new(config); let conversation_id = ConversationId::default(); let otel_event_manager = otel_event_manager(conversation_id, config.as_ref()); - let auth_manager = AuthManager::shared(config.cwd.clone(), false); + let auth_manager = AuthManager::shared( + config.cwd.clone(), + false, + config.cli_auth_credentials_store_mode, + ); let session_configuration = SessionConfiguration { provider: config.model_provider.clone(), @@ -2718,7 +2683,11 @@ mod tests { let config = Arc::new(config); let conversation_id = ConversationId::default(); let otel_event_manager = otel_event_manager(conversation_id, config.as_ref()); - let auth_manager = AuthManager::shared(config.cwd.clone(), false); + let auth_manager = AuthManager::shared( + config.cwd.clone(), + false, + config.cli_auth_credentials_store_mode, + ); let session_configuration = SessionConfiguration { provider: config.model_provider.clone(), diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index c5406b1d27..f671e8dd36 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -1,3 +1,4 @@ +use crate::auth::AuthCredentialsStoreMode; use crate::config_loader::LoadedConfigLayers; pub use crate::config_loader::load_config_as_toml; use crate::config_loader::load_config_layers_with_overrides; @@ -108,6 +109,10 @@ pub struct Config { /// for either of approval_policy or sandbox_mode. pub did_user_set_custom_approval_policy_or_sandbox_mode: bool, + /// On Windows, indicates that a previously configured workspace-write sandbox + /// was coerced to read-only because native auto mode is unsupported. + pub forced_auto_mode_downgraded_on_windows: bool, + pub shell_environment_policy: ShellEnvironmentPolicy, /// When `true`, `AgentReasoning` events emitted by the backend will be @@ -156,6 +161,12 @@ pub struct Config { /// resolved against this path. pub cwd: PathBuf, + /// Preferred store for CLI auth credentials. + /// file (default): Use a file in the Codex home directory. + /// keyring: Use an OS-specific keyring service. + /// auto: Use the OS-specific keyring service if available, otherwise use a file. + pub cli_auth_credentials_store_mode: AuthCredentialsStoreMode, + /// Definition for MCP servers that Codex can reach out to for tool calls. pub mcp_servers: HashMap, @@ -869,6 +880,13 @@ pub struct ConfigToml { #[serde(default)] pub forced_login_method: Option, + /// Preferred backend for storing CLI auth credentials. + /// file (default): Use a file in the Codex home directory. + /// keyring: Use an OS-specific keyring service. + /// auto: Use the keyring if available, otherwise use a file. + #[serde(default)] + pub cli_auth_credentials_store: Option, + /// Definition for MCP servers that Codex can reach out to for tool calls. #[serde(default)] pub mcp_servers: HashMap, @@ -1022,6 +1040,12 @@ impl From for Tools { } } +#[derive(Debug, PartialEq, Eq)] +pub struct SandboxPolicyResolution { + pub policy: SandboxPolicy, + pub forced_auto_mode_downgraded_on_windows: bool, +} + impl ConfigToml { /// Derive the effective sandbox policy from the configuration. fn derive_sandbox_policy( @@ -1029,7 +1053,7 @@ impl ConfigToml { sandbox_mode_override: Option, profile_sandbox_mode: Option, resolved_cwd: &Path, - ) -> SandboxPolicy { + ) -> SandboxPolicyResolution { let resolved_sandbox_mode = sandbox_mode_override .or(profile_sandbox_mode) .or(self.sandbox_mode) @@ -1044,7 +1068,7 @@ impl ConfigToml { }) }) .unwrap_or_default(); - match resolved_sandbox_mode { + let mut sandbox_policy = match resolved_sandbox_mode { SandboxMode::ReadOnly => SandboxPolicy::new_read_only_policy(), SandboxMode::WorkspaceWrite => match self.sandbox_workspace_write.as_ref() { Some(SandboxWorkspaceWrite { @@ -1061,6 +1085,17 @@ impl ConfigToml { None => SandboxPolicy::new_workspace_write_policy(), }, SandboxMode::DangerFullAccess => SandboxPolicy::DangerFullAccess, + }; + let mut forced_auto_mode_downgraded_on_windows = false; + if cfg!(target_os = "windows") + && matches!(resolved_sandbox_mode, SandboxMode::WorkspaceWrite) + { + sandbox_policy = SandboxPolicy::new_read_only_policy(); + forced_auto_mode_downgraded_on_windows = true; + } + SandboxPolicyResolution { + policy: sandbox_policy, + forced_auto_mode_downgraded_on_windows, } } @@ -1221,8 +1256,10 @@ impl Config { .get_active_project(&resolved_cwd) .unwrap_or(ProjectConfig { trust_level: None }); - let mut sandbox_policy = - cfg.derive_sandbox_policy(sandbox_mode, config_profile.sandbox_mode, &resolved_cwd); + let SandboxPolicyResolution { + policy: mut sandbox_policy, + forced_auto_mode_downgraded_on_windows, + } = cfg.derive_sandbox_policy(sandbox_mode, config_profile.sandbox_mode, &resolved_cwd); if let SandboxPolicy::WorkspaceWrite { writable_roots, .. } = &mut sandbox_policy { for path in additional_writable_roots { if !writable_roots.iter().any(|existing| existing == &path) { @@ -1353,10 +1390,14 @@ impl Config { approval_policy, sandbox_policy, did_user_set_custom_approval_policy_or_sandbox_mode, + forced_auto_mode_downgraded_on_windows, shell_environment_policy, notify: cfg.notify, user_instructions, base_instructions, + // The config.toml omits "_mode" because it's a config file. However, "_mode" + // is important in code to differentiate the mode from the store implementation. + cli_auth_credentials_store_mode: cfg.cli_auth_credentials_store.unwrap_or_default(), mcp_servers: cfg.mcp_servers, // The config.toml omits "_mode" because it's a config file. However, "_mode" // is important in code to differentiate the mode from the store implementation. @@ -1604,13 +1645,17 @@ network_access = false # This should be ignored. let sandbox_full_access_cfg = toml::from_str::(sandbox_full_access) .expect("TOML deserialization should succeed"); let sandbox_mode_override = None; + let resolution = sandbox_full_access_cfg.derive_sandbox_policy( + sandbox_mode_override, + None, + &PathBuf::from("/tmp/test"), + ); assert_eq!( - SandboxPolicy::DangerFullAccess, - sandbox_full_access_cfg.derive_sandbox_policy( - sandbox_mode_override, - None, - &PathBuf::from("/tmp/test") - ) + resolution, + SandboxPolicyResolution { + policy: SandboxPolicy::DangerFullAccess, + forced_auto_mode_downgraded_on_windows: false, + } ); let sandbox_read_only = r#" @@ -1623,13 +1668,17 @@ network_access = true # This should be ignored. let sandbox_read_only_cfg = toml::from_str::(sandbox_read_only) .expect("TOML deserialization should succeed"); let sandbox_mode_override = None; + let resolution = sandbox_read_only_cfg.derive_sandbox_policy( + sandbox_mode_override, + None, + &PathBuf::from("/tmp/test"), + ); assert_eq!( - SandboxPolicy::ReadOnly, - sandbox_read_only_cfg.derive_sandbox_policy( - sandbox_mode_override, - None, - &PathBuf::from("/tmp/test") - ) + resolution, + SandboxPolicyResolution { + policy: SandboxPolicy::ReadOnly, + forced_auto_mode_downgraded_on_windows: false, + } ); let sandbox_workspace_write = r#" @@ -1646,19 +1695,33 @@ exclude_slash_tmp = true let sandbox_workspace_write_cfg = toml::from_str::(sandbox_workspace_write) .expect("TOML deserialization should succeed"); let sandbox_mode_override = None; - assert_eq!( - SandboxPolicy::WorkspaceWrite { - writable_roots: vec![PathBuf::from("/my/workspace")], - network_access: false, - exclude_tmpdir_env_var: true, - exclude_slash_tmp: true, - }, - sandbox_workspace_write_cfg.derive_sandbox_policy( - sandbox_mode_override, - None, - &PathBuf::from("/tmp/test") - ) + let resolution = sandbox_workspace_write_cfg.derive_sandbox_policy( + sandbox_mode_override, + None, + &PathBuf::from("/tmp/test"), ); + if cfg!(target_os = "windows") { + assert_eq!( + resolution, + SandboxPolicyResolution { + policy: SandboxPolicy::ReadOnly, + forced_auto_mode_downgraded_on_windows: true, + } + ); + } else { + assert_eq!( + resolution, + SandboxPolicyResolution { + policy: SandboxPolicy::WorkspaceWrite { + writable_roots: vec![PathBuf::from("/my/workspace")], + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }, + forced_auto_mode_downgraded_on_windows: false, + } + ); + } let sandbox_workspace_write = r#" sandbox_mode = "workspace-write" @@ -1677,19 +1740,33 @@ trust_level = "trusted" let sandbox_workspace_write_cfg = toml::from_str::(sandbox_workspace_write) .expect("TOML deserialization should succeed"); let sandbox_mode_override = None; - assert_eq!( - SandboxPolicy::WorkspaceWrite { - writable_roots: vec![PathBuf::from("/my/workspace")], - network_access: false, - exclude_tmpdir_env_var: true, - exclude_slash_tmp: true, - }, - sandbox_workspace_write_cfg.derive_sandbox_policy( - sandbox_mode_override, - None, - &PathBuf::from("/tmp/test") - ) + let resolution = sandbox_workspace_write_cfg.derive_sandbox_policy( + sandbox_mode_override, + None, + &PathBuf::from("/tmp/test"), ); + if cfg!(target_os = "windows") { + assert_eq!( + resolution, + SandboxPolicyResolution { + policy: SandboxPolicy::ReadOnly, + forced_auto_mode_downgraded_on_windows: true, + } + ); + } else { + assert_eq!( + resolution, + SandboxPolicyResolution { + policy: SandboxPolicy::WorkspaceWrite { + writable_roots: vec![PathBuf::from("/my/workspace")], + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }, + forced_auto_mode_downgraded_on_windows: false, + } + ); + } } #[test] @@ -1714,24 +1791,76 @@ trust_level = "trusted" )?; let expected_backend = canonicalize(&backend).expect("canonicalize backend directory"); - match config.sandbox_policy { - SandboxPolicy::WorkspaceWrite { writable_roots, .. } => { - assert_eq!( - writable_roots - .iter() - .filter(|root| **root == expected_backend) - .count(), - 1, - "expected single writable root entry for {}", - expected_backend.display() - ); + if cfg!(target_os = "windows") { + assert!( + config.forced_auto_mode_downgraded_on_windows, + "expected workspace-write request to be downgraded on Windows" + ); + match config.sandbox_policy { + SandboxPolicy::ReadOnly => {} + other => panic!("expected read-only policy on Windows, got {other:?}"), + } + } else { + match config.sandbox_policy { + SandboxPolicy::WorkspaceWrite { writable_roots, .. } => { + assert_eq!( + writable_roots + .iter() + .filter(|root| **root == expected_backend) + .count(), + 1, + "expected single writable root entry for {}", + expected_backend.display() + ); + } + other => panic!("expected workspace-write policy, got {other:?}"), } - other => panic!("expected workspace-write policy, got {other:?}"), } Ok(()) } + #[test] + fn config_defaults_to_file_cli_auth_store_mode() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cfg = ConfigToml::default(); + + let config = Config::load_from_base_config_with_overrides( + cfg, + ConfigOverrides::default(), + codex_home.path().to_path_buf(), + )?; + + assert_eq!( + config.cli_auth_credentials_store_mode, + AuthCredentialsStoreMode::File, + ); + + Ok(()) + } + + #[test] + fn config_honors_explicit_keyring_auth_store_mode() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cfg = ConfigToml { + cli_auth_credentials_store: Some(AuthCredentialsStoreMode::Keyring), + ..Default::default() + }; + + let config = Config::load_from_base_config_with_overrides( + cfg, + ConfigOverrides::default(), + codex_home.path().to_path_buf(), + )?; + + assert_eq!( + config.cli_auth_credentials_store_mode, + AuthCredentialsStoreMode::Keyring, + ); + + Ok(()) + } + #[test] fn config_defaults_to_auto_oauth_store_mode() -> std::io::Result<()> { let codex_home = TempDir::new()?; @@ -1841,10 +1970,16 @@ trust_level = "trusted" codex_home.path().to_path_buf(), )?; - assert!(matches!( - config.sandbox_policy, - SandboxPolicy::WorkspaceWrite { .. } - )); + if cfg!(target_os = "windows") { + assert!(matches!(config.sandbox_policy, SandboxPolicy::ReadOnly)); + assert!(config.forced_auto_mode_downgraded_on_windows); + } else { + assert!(matches!( + config.sandbox_policy, + SandboxPolicy::WorkspaceWrite { .. } + )); + assert!(!config.forced_auto_mode_downgraded_on_windows); + } Ok(()) } @@ -2943,10 +3078,12 @@ model_verbosity = "high" approval_policy: AskForApproval::Never, sandbox_policy: SandboxPolicy::new_read_only_policy(), did_user_set_custom_approval_policy_or_sandbox_mode: true, + forced_auto_mode_downgraded_on_windows: false, shell_environment_policy: ShellEnvironmentPolicy::default(), user_instructions: None, notify: None, cwd: fixture.cwd(), + cli_auth_credentials_store_mode: Default::default(), mcp_servers: HashMap::new(), mcp_oauth_credentials_store_mode: Default::default(), model_providers: fixture.model_provider_map.clone(), @@ -3012,10 +3149,12 @@ model_verbosity = "high" approval_policy: AskForApproval::UnlessTrusted, sandbox_policy: SandboxPolicy::new_read_only_policy(), did_user_set_custom_approval_policy_or_sandbox_mode: true, + forced_auto_mode_downgraded_on_windows: false, shell_environment_policy: ShellEnvironmentPolicy::default(), user_instructions: None, notify: None, cwd: fixture.cwd(), + cli_auth_credentials_store_mode: Default::default(), mcp_servers: HashMap::new(), mcp_oauth_credentials_store_mode: Default::default(), model_providers: fixture.model_provider_map.clone(), @@ -3096,10 +3235,12 @@ model_verbosity = "high" approval_policy: AskForApproval::OnFailure, sandbox_policy: SandboxPolicy::new_read_only_policy(), did_user_set_custom_approval_policy_or_sandbox_mode: true, + forced_auto_mode_downgraded_on_windows: false, shell_environment_policy: ShellEnvironmentPolicy::default(), user_instructions: None, notify: None, cwd: fixture.cwd(), + cli_auth_credentials_store_mode: Default::default(), mcp_servers: HashMap::new(), mcp_oauth_credentials_store_mode: Default::default(), model_providers: fixture.model_provider_map.clone(), @@ -3166,10 +3307,12 @@ model_verbosity = "high" approval_policy: AskForApproval::OnFailure, sandbox_policy: SandboxPolicy::new_read_only_policy(), did_user_set_custom_approval_policy_or_sandbox_mode: true, + forced_auto_mode_downgraded_on_windows: false, shell_environment_policy: ShellEnvironmentPolicy::default(), user_instructions: None, notify: None, cwd: fixture.cwd(), + cli_auth_credentials_store_mode: Default::default(), mcp_servers: HashMap::new(), mcp_oauth_credentials_store_mode: Default::default(), model_providers: fixture.model_provider_map.clone(), diff --git a/codex-rs/core/src/conversation_history.rs b/codex-rs/core/src/conversation_history.rs index 08fa8cebed..27291ebc0c 100644 --- a/codex-rs/core/src/conversation_history.rs +++ b/codex-rs/core/src/conversation_history.rs @@ -1,10 +1,20 @@ +use codex_protocol::models::FunctionCallOutputContentItem; use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseItem; use codex_protocol::protocol::TokenUsage; use codex_protocol::protocol::TokenUsageInfo; +use codex_utils_string::take_bytes_at_char_boundary; +use codex_utils_string::take_last_bytes_at_char_boundary; use std::ops::Deref; use tracing::error; +// Model-formatting limits: clients get full streams; only content sent to the model is truncated. +pub(crate) const MODEL_FORMAT_MAX_BYTES: usize = 10 * 1024; // 10 KiB +pub(crate) const MODEL_FORMAT_MAX_LINES: usize = 256; // lines +pub(crate) const MODEL_FORMAT_HEAD_LINES: usize = MODEL_FORMAT_MAX_LINES / 2; +pub(crate) const MODEL_FORMAT_TAIL_LINES: usize = MODEL_FORMAT_MAX_LINES - MODEL_FORMAT_HEAD_LINES; // 128 +pub(crate) const MODEL_FORMAT_HEAD_BYTES: usize = MODEL_FORMAT_MAX_BYTES / 2; + /// Transcript of conversation history #[derive(Debug, Clone, Default)] pub(crate) struct ConversationHistory { @@ -47,7 +57,8 @@ impl ConversationHistory { continue; } - self.items.push(item.clone()); + let processed = Self::process_item(&item); + self.items.push(processed); } } @@ -68,6 +79,22 @@ impl ConversationHistory { } } + pub(crate) fn replace(&mut self, items: Vec) { + self.items = items; + } + + pub(crate) fn update_token_info( + &mut self, + usage: &TokenUsage, + model_context_window: Option, + ) { + self.token_info = TokenUsageInfo::new_or_append( + &self.token_info, + &Some(usage.clone()), + model_context_window, + ); + } + /// This function enforces a couple of invariants on the in-memory history: /// 1. every call (function/custom) has a corresponding output entry /// 2. every output has a corresponding call entry @@ -110,7 +137,7 @@ impl ConversationHistory { call_id: call_id.clone(), output: FunctionCallOutputPayload { content: "aborted".to_string(), - success: None, + ..Default::default() }, }, )); @@ -157,7 +184,7 @@ impl ConversationHistory { call_id: call_id.clone(), output: FunctionCallOutputPayload { content: "aborted".to_string(), - success: None, + ..Default::default() }, }, )); @@ -253,10 +280,6 @@ impl ConversationHistory { } } - pub(crate) fn replace(&mut self, items: Vec) { - self.items = items; - } - /// Removes the corresponding paired item for the provided `item`, if any. /// /// Pairs: @@ -326,19 +349,126 @@ impl ConversationHistory { } } - pub(crate) fn update_token_info( - &mut self, - usage: &TokenUsage, - model_context_window: Option, - ) { - self.token_info = TokenUsageInfo::new_or_append( - &self.token_info, - &Some(usage.clone()), - model_context_window, - ); + fn process_item(item: &ResponseItem) -> ResponseItem { + match item { + ResponseItem::FunctionCallOutput { call_id, output } => { + let truncated = format_output_for_model_body(output.content.as_str()); + let truncated_items = output.content_items.as_ref().map(|items| { + items + .iter() + .map(|it| match it { + FunctionCallOutputContentItem::InputText { text } => { + FunctionCallOutputContentItem::InputText { + text: format_output_for_model_body(text), + } + } + FunctionCallOutputContentItem::InputImage { image_url } => { + FunctionCallOutputContentItem::InputImage { + image_url: image_url.clone(), + } + } + }) + .collect() + }); + ResponseItem::FunctionCallOutput { + call_id: call_id.clone(), + output: FunctionCallOutputPayload { + content: truncated, + content_items: truncated_items, + success: output.success, + }, + } + } + ResponseItem::CustomToolCallOutput { call_id, output } => { + let truncated = format_output_for_model_body(output); + ResponseItem::CustomToolCallOutput { + call_id: call_id.clone(), + output: truncated, + } + } + ResponseItem::Message { .. } + | ResponseItem::Reasoning { .. } + | ResponseItem::LocalShellCall { .. } + | ResponseItem::FunctionCall { .. } + | ResponseItem::WebSearchCall { .. } + | ResponseItem::CustomToolCall { .. } + | ResponseItem::GhostSnapshot { .. } + | ResponseItem::Other => item.clone(), + } } } +pub(crate) fn format_output_for_model_body(content: &str) -> String { + // Head+tail truncation for the model: show the beginning and end with an elision. + // Clients still receive full streams; only this formatted summary is capped. + let total_lines = content.lines().count(); + if content.len() <= MODEL_FORMAT_MAX_BYTES && total_lines <= MODEL_FORMAT_MAX_LINES { + return content.to_string(); + } + let output = truncate_formatted_exec_output(content, total_lines); + format!("Total output lines: {total_lines}\n\n{output}") +} + +fn truncate_formatted_exec_output(content: &str, total_lines: usize) -> String { + let segments: Vec<&str> = content.split_inclusive('\n').collect(); + let head_take = MODEL_FORMAT_HEAD_LINES.min(segments.len()); + let tail_take = MODEL_FORMAT_TAIL_LINES.min(segments.len().saturating_sub(head_take)); + let omitted = segments.len().saturating_sub(head_take + tail_take); + + let head_slice_end: usize = segments + .iter() + .take(head_take) + .map(|segment| segment.len()) + .sum(); + let tail_slice_start: usize = if tail_take == 0 { + content.len() + } else { + content.len() + - segments + .iter() + .rev() + .take(tail_take) + .map(|segment| segment.len()) + .sum::() + }; + let head_slice = &content[..head_slice_end]; + let tail_slice = &content[tail_slice_start..]; + let truncated_by_bytes = content.len() > MODEL_FORMAT_MAX_BYTES; + // this is a bit wrong. We are counting metadata lines and not just shell output lines. + let marker = if omitted > 0 { + Some(format!( + "\n[... omitted {omitted} of {total_lines} lines ...]\n\n" + )) + } else if truncated_by_bytes { + Some(format!( + "\n[... output truncated to fit {MODEL_FORMAT_MAX_BYTES} bytes ...]\n\n" + )) + } else { + None + }; + + let marker_len = marker.as_ref().map_or(0, String::len); + let base_head_budget = MODEL_FORMAT_HEAD_BYTES.min(MODEL_FORMAT_MAX_BYTES); + let head_budget = base_head_budget.min(MODEL_FORMAT_MAX_BYTES.saturating_sub(marker_len)); + let head_part = take_bytes_at_char_boundary(head_slice, head_budget); + let mut result = String::with_capacity(MODEL_FORMAT_MAX_BYTES.min(content.len())); + + result.push_str(head_part); + if let Some(marker_text) = marker.as_ref() { + result.push_str(marker_text); + } + + let remaining = MODEL_FORMAT_MAX_BYTES.saturating_sub(result.len()); + if remaining == 0 { + return result; + } + + let tail_part = take_last_bytes_at_char_boundary(tail_slice, remaining); + result.push_str(tail_part); + + result +} + #[inline] fn error_or_panic(message: String) { if cfg!(debug_assertions) || env!("CARGO_PKG_VERSION").contains("alpha") { @@ -454,7 +584,7 @@ mod tests { call_id: "call-1".to_string(), output: FunctionCallOutputPayload { content: "ok".to_string(), - success: None, + ..Default::default() }, }, ]; @@ -470,7 +600,7 @@ mod tests { call_id: "call-2".to_string(), output: FunctionCallOutputPayload { content: "ok".to_string(), - success: None, + ..Default::default() }, }, ResponseItem::FunctionCall { @@ -504,7 +634,7 @@ mod tests { call_id: "call-3".to_string(), output: FunctionCallOutputPayload { content: "ok".to_string(), - success: None, + ..Default::default() }, }, ]; @@ -533,6 +663,184 @@ mod tests { assert_eq!(h.contents(), vec![]); } + #[test] + fn record_items_truncates_function_call_output_content() { + let mut history = ConversationHistory::new(); + let long_line = "a very long line to trigger truncation\n"; + let long_output = long_line.repeat(2_500); + let item = ResponseItem::FunctionCallOutput { + call_id: "call-100".to_string(), + output: FunctionCallOutputPayload { + content: long_output.clone(), + success: Some(true), + ..Default::default() + }, + }; + + history.record_items([&item]); + + assert_eq!(history.items.len(), 1); + match &history.items[0] { + ResponseItem::FunctionCallOutput { output, .. } => { + assert_ne!(output.content, long_output); + assert!( + output.content.starts_with("Total output lines:"), + "expected truncated summary, got {}", + output.content + ); + } + other => panic!("unexpected history item: {other:?}"), + } + } + + #[test] + fn record_items_truncates_custom_tool_call_output_content() { + let mut history = ConversationHistory::new(); + let line = "custom output that is very long\n"; + let long_output = line.repeat(2_500); + let item = ResponseItem::CustomToolCallOutput { + call_id: "tool-200".to_string(), + output: long_output.clone(), + }; + + history.record_items([&item]); + + assert_eq!(history.items.len(), 1); + match &history.items[0] { + ResponseItem::CustomToolCallOutput { output, .. } => { + assert_ne!(output, &long_output); + assert!( + output.starts_with("Total output lines:"), + "expected truncated summary, got {output}" + ); + } + other => panic!("unexpected history item: {other:?}"), + } + } + + // The following tests were adapted from tools::mod truncation tests to + // target the new truncation functions in conversation_history. + + use regex_lite::Regex; + + fn assert_truncated_message_matches(message: &str, line: &str, total_lines: usize) { + let pattern = truncated_message_pattern(line, total_lines); + let regex = Regex::new(&pattern).unwrap_or_else(|err| { + panic!("failed to compile regex {pattern}: {err}"); + }); + let captures = regex + .captures(message) + .unwrap_or_else(|| panic!("message failed to match pattern {pattern}: {message}")); + let body = captures + .name("body") + .expect("missing body capture") + .as_str(); + assert!( + body.len() <= MODEL_FORMAT_MAX_BYTES, + "body exceeds byte limit: {} bytes", + body.len() + ); + } + + fn truncated_message_pattern(line: &str, total_lines: usize) -> String { + let head_take = MODEL_FORMAT_HEAD_LINES.min(total_lines); + let tail_take = MODEL_FORMAT_TAIL_LINES.min(total_lines.saturating_sub(head_take)); + let omitted = total_lines.saturating_sub(head_take + tail_take); + let escaped_line = regex_lite::escape(line); + if omitted == 0 { + return format!( + r"(?s)^Total output lines: {total_lines}\n\n(?P{escaped_line}.*\n\[\.{{3}} output truncated to fit {MODEL_FORMAT_MAX_BYTES} bytes \.{{3}}]\n\n.*)$", + ); + } + format!( + r"(?s)^Total output lines: {total_lines}\n\n(?P{escaped_line}.*\n\[\.{{3}} omitted {omitted} of {total_lines} lines \.{{3}}]\n\n.*)$", + ) + } + + #[test] + fn format_exec_output_truncates_large_error() { + let line = "very long execution error line that should trigger truncation\n"; + let large_error = line.repeat(2_500); // way beyond both byte and line limits + + let truncated = format_output_for_model_body(&large_error); + + let total_lines = large_error.lines().count(); + assert_truncated_message_matches(&truncated, line, total_lines); + assert_ne!(truncated, large_error); + } + + #[test] + fn format_exec_output_marks_byte_truncation_without_omitted_lines() { + let long_line = "a".repeat(MODEL_FORMAT_MAX_BYTES + 50); + let truncated = format_output_for_model_body(&long_line); + + assert_ne!(truncated, long_line); + let marker_line = + format!("[... output truncated to fit {MODEL_FORMAT_MAX_BYTES} bytes ...]"); + assert!( + truncated.contains(&marker_line), + "missing byte truncation marker: {truncated}" + ); + assert!( + !truncated.contains("omitted"), + "line omission marker should not appear when no lines were dropped: {truncated}" + ); + } + + #[test] + fn format_exec_output_returns_original_when_within_limits() { + let content = "example output\n".repeat(10); + + assert_eq!(format_output_for_model_body(&content), content); + } + + #[test] + fn format_exec_output_reports_omitted_lines_and_keeps_head_and_tail() { + let total_lines = MODEL_FORMAT_MAX_LINES + 100; + let content: String = (0..total_lines) + .map(|idx| format!("line-{idx}\n")) + .collect(); + + let truncated = format_output_for_model_body(&content); + let omitted = total_lines - MODEL_FORMAT_MAX_LINES; + let expected_marker = format!("[... omitted {omitted} of {total_lines} lines ...]"); + + assert!( + truncated.contains(&expected_marker), + "missing omitted marker: {truncated}" + ); + assert!( + truncated.contains("line-0\n"), + "expected head line to remain: {truncated}" + ); + + let last_line = format!("line-{}\n", total_lines - 1); + assert!( + truncated.contains(&last_line), + "expected tail line to remain: {truncated}" + ); + } + + #[test] + fn format_exec_output_prefers_line_marker_when_both_limits_exceeded() { + let total_lines = MODEL_FORMAT_MAX_LINES + 42; + let long_line = "x".repeat(256); + let content: String = (0..total_lines) + .map(|idx| format!("line-{idx}-{long_line}\n")) + .collect(); + + let truncated = format_output_for_model_body(&content); + + assert!( + truncated.contains("[... omitted 42 of 298 lines ...]"), + "expected omitted marker when line count exceeds limit: {truncated}" + ); + assert!( + !truncated.contains("output truncated to fit"), + "line omission marker should take precedence over byte marker: {truncated}" + ); + } + //TODO(aibrahim): run CI in release mode. #[cfg(not(debug_assertions))] #[test] @@ -560,7 +868,7 @@ mod tests { call_id: "call-x".to_string(), output: FunctionCallOutputPayload { content: "aborted".to_string(), - success: None, + ..Default::default() }, }, ] @@ -637,7 +945,7 @@ mod tests { call_id: "shell-1".to_string(), output: FunctionCallOutputPayload { content: "aborted".to_string(), - success: None, + ..Default::default() }, }, ] @@ -651,7 +959,7 @@ mod tests { call_id: "orphan-1".to_string(), output: FunctionCallOutputPayload { content: "ok".to_string(), - success: None, + ..Default::default() }, }]; let mut h = create_history_with_items(items); @@ -691,7 +999,7 @@ mod tests { call_id: "c2".to_string(), output: FunctionCallOutputPayload { content: "ok".to_string(), - success: None, + ..Default::default() }, }, // Will get an inserted custom tool output @@ -733,7 +1041,7 @@ mod tests { call_id: "c1".to_string(), output: FunctionCallOutputPayload { content: "aborted".to_string(), - success: None, + ..Default::default() }, }, ResponseItem::CustomToolCall { @@ -763,7 +1071,7 @@ mod tests { call_id: "s1".to_string(), output: FunctionCallOutputPayload { content: "aborted".to_string(), - success: None, + ..Default::default() }, }, ] @@ -828,7 +1136,7 @@ mod tests { call_id: "orphan-1".to_string(), output: FunctionCallOutputPayload { content: "ok".to_string(), - success: None, + ..Default::default() }, }]; let mut h = create_history_with_items(items); @@ -862,7 +1170,7 @@ mod tests { call_id: "c2".to_string(), output: FunctionCallOutputPayload { content: "ok".to_string(), - success: None, + ..Default::default() }, }, ResponseItem::CustomToolCall { diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index 09846b719a..5166410372 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -35,6 +35,7 @@ pub(crate) async fn handle_mcp_tool_call( output: FunctionCallOutputPayload { content: format!("err: {e}"), success: Some(false), + ..Default::default() }, }; } diff --git a/codex-rs/core/src/response_processing.rs b/codex-rs/core/src/response_processing.rs index e18fdd45c8..5c30c1126c 100644 --- a/codex-rs/core/src/response_processing.rs +++ b/codex-rs/core/src/response_processing.rs @@ -61,14 +61,11 @@ pub(crate) async fn process_items( ) => { items_to_record_in_conversation_history.push(item); let output = match result { - Ok(call_tool_result) => { - crate::codex::convert_call_tool_result_to_function_call_output_payload( - call_tool_result, - ) - } + Ok(call_tool_result) => FunctionCallOutputPayload::from(call_tool_result), Err(err) => FunctionCallOutputPayload { content: err.clone(), success: Some(false), + ..Default::default() }, }; items_to_record_in_conversation_history.push(ResponseItem::FunctionCallOutput { diff --git a/codex-rs/core/src/shell.rs b/codex-rs/core/src/shell.rs index 9a3f88976e..e164e5b48d 100644 --- a/codex-rs/core/src/shell.rs +++ b/codex-rs/core/src/shell.rs @@ -1,6 +1,5 @@ use serde::Deserialize; use serde::Serialize; -use shlex; use std::path::PathBuf; #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] @@ -30,67 +29,6 @@ pub enum Shell { } impl Shell { - pub fn format_default_shell_invocation(&self, command: Vec) -> Option> { - match self { - Shell::Zsh(zsh) => format_shell_invocation_with_rc( - command.as_slice(), - &zsh.shell_path, - &zsh.zshrc_path, - ), - Shell::Bash(bash) => format_shell_invocation_with_rc( - command.as_slice(), - &bash.shell_path, - &bash.bashrc_path, - ), - Shell::PowerShell(ps) => { - // If model generated a bash command, prefer a detected bash fallback - if let Some(script) = strip_bash_lc(command.as_slice()) { - return match &ps.bash_exe_fallback { - Some(bash) => Some(vec![ - bash.to_string_lossy().to_string(), - "-lc".to_string(), - script, - ]), - - // No bash fallback → run the script under PowerShell. - // It will likely fail (except for some simple commands), but the error - // should give a clue to the model to fix upon retry that it's running under PowerShell. - None => Some(vec![ - ps.exe.clone(), - "-NoProfile".to_string(), - "-Command".to_string(), - script, - ]), - }; - } - - // Not a bash command. If model did not generate a PowerShell command, - // turn it into a PowerShell command. - let first = command.first().map(String::as_str); - if first != Some(ps.exe.as_str()) { - // TODO (CODEX_2900): Handle escaping newlines. - if command.iter().any(|a| a.contains('\n') || a.contains('\r')) { - return Some(command); - } - - let joined = shlex::try_join(command.iter().map(String::as_str)).ok(); - return joined.map(|arg| { - vec![ - ps.exe.clone(), - "-NoProfile".to_string(), - "-Command".to_string(), - arg, - ] - }); - } - - // Model generated a PowerShell command. Run it. - Some(command) - } - Shell::Unknown => None, - } - } - pub fn name(&self) -> Option { match self { Shell::Zsh(zsh) => std::path::Path::new(&zsh.shell_path) @@ -105,36 +43,6 @@ impl Shell { } } -fn format_shell_invocation_with_rc( - command: &[String], - shell_path: &str, - rc_path: &str, -) -> Option> { - let joined = strip_bash_lc(command) - .or_else(|| shlex::try_join(command.iter().map(String::as_str)).ok())?; - - let rc_command = if std::path::Path::new(rc_path).exists() { - format!("source {rc_path} && ({joined})") - } else { - joined - }; - - Some(vec![shell_path.to_string(), "-lc".to_string(), rc_command]) -} - -fn strip_bash_lc(command: &[String]) -> Option { - match command { - // exactly three items - [first, second, third] - // first two must be "bash", "-lc" - if first == "bash" && second == "-lc" => - { - Some(third.clone()) - } - _ => None, - } -} - #[cfg(unix)] fn detect_default_user_shell() -> Shell { use libc::getpwuid; @@ -223,8 +131,8 @@ pub async fn default_user_shell() -> Shell { #[cfg(unix)] mod tests { use super::*; + use std::path::PathBuf; use std::process::Command; - use std::string::ToString; #[tokio::test] async fn test_current_shell_detects_zsh() { @@ -247,40 +155,6 @@ mod tests { } } - #[tokio::test] - async fn test_run_with_profile_zshrc_not_exists() { - let shell = Shell::Zsh(ZshShell { - shell_path: "/bin/zsh".to_string(), - zshrc_path: "/does/not/exist/.zshrc".to_string(), - }); - let actual_cmd = shell.format_default_shell_invocation(vec!["myecho".to_string()]); - assert_eq!( - actual_cmd, - Some(vec![ - "/bin/zsh".to_string(), - "-lc".to_string(), - "myecho".to_string() - ]) - ); - } - - #[tokio::test] - async fn test_run_with_profile_bashrc_not_exists() { - let shell = Shell::Bash(BashShell { - shell_path: "/bin/bash".to_string(), - bashrc_path: "/does/not/exist/.bashrc".to_string(), - }); - let actual_cmd = shell.format_default_shell_invocation(vec!["myecho".to_string()]); - assert_eq!( - actual_cmd, - Some(vec![ - "/bin/bash".to_string(), - "-lc".to_string(), - "myecho".to_string() - ]) - ); - } - #[tokio::test] async fn test_run_with_profile_bash_escaping_and_execution() { let shell_path = "/bin/bash"; @@ -315,30 +189,21 @@ mod tests { std::fs::write( &bashrc_path, r#" - set -x - function myecho { - echo 'It works!' - } - "#, + set -x + function myecho { + echo 'It works!' + } + "#, ) .unwrap(); - let shell = Shell::Bash(BashShell { - shell_path: shell_path.to_string(), - bashrc_path: bashrc_path.to_str().unwrap().to_string(), - }); - - let actual_cmd = shell - .format_default_shell_invocation(input.iter().map(ToString::to_string).collect()); - let expected_cmd = expected_cmd + let command = expected_cmd .iter() .map(|s| s.replace("BASHRC_PATH", bashrc_path.to_str().unwrap())) - .collect(); - - assert_eq!(actual_cmd, Some(expected_cmd)); + .collect::>(); let output = process_exec_tool_call( ExecParams { - command: actual_cmd.unwrap(), + command: command.clone(), cwd: PathBuf::from(temp_home.path()), timeout_ms: None, env: HashMap::from([( @@ -372,8 +237,7 @@ mod tests { #[cfg(test)] #[cfg(target_os = "macos")] mod macos_tests { - use super::*; - use std::string::ToString; + use std::path::PathBuf; #[tokio::test] async fn test_run_with_profile_escaping_and_execution() { @@ -411,43 +275,32 @@ mod macos_tests { ]; for (input, expected_cmd, expected_output) in cases { use std::collections::HashMap; - use std::path::PathBuf; use crate::exec::ExecParams; use crate::exec::SandboxType; use crate::exec::process_exec_tool_call; use crate::protocol::SandboxPolicy; - // create a temp directory with a zshrc file in it let temp_home = tempfile::tempdir().unwrap(); let zshrc_path = temp_home.path().join(".zshrc"); std::fs::write( &zshrc_path, r#" - set -x - function myecho { - echo 'It works!' - } - "#, + set -x + function myecho { + echo 'It works!' + } + "#, ) .unwrap(); - let shell = Shell::Zsh(ZshShell { - shell_path: shell_path.to_string(), - zshrc_path: zshrc_path.to_str().unwrap().to_string(), - }); - - let actual_cmd = shell - .format_default_shell_invocation(input.iter().map(ToString::to_string).collect()); - let expected_cmd = expected_cmd + let command = expected_cmd .iter() .map(|s| s.replace("ZSHRC_PATH", zshrc_path.to_str().unwrap())) - .collect(); + .collect::>(); - assert_eq!(actual_cmd, Some(expected_cmd)); - // Actually run the command and check output/exit code let output = process_exec_tool_call( ExecParams { - command: actual_cmd.unwrap(), + command: command.clone(), cwd: PathBuf::from(temp_home.path()), timeout_ms: None, env: HashMap::from([( @@ -485,36 +338,38 @@ mod tests_windows { #[test] fn test_format_default_shell_invocation_powershell() { + use std::path::PathBuf; + let cases = vec![ ( - Shell::PowerShell(PowerShellConfig { + PowerShellConfig { exe: "pwsh.exe".to_string(), bash_exe_fallback: None, - }), + }, vec!["bash", "-lc", "echo hello"], vec!["pwsh.exe", "-NoProfile", "-Command", "echo hello"], ), ( - Shell::PowerShell(PowerShellConfig { + PowerShellConfig { exe: "powershell.exe".to_string(), bash_exe_fallback: None, - }), + }, vec!["bash", "-lc", "echo hello"], vec!["powershell.exe", "-NoProfile", "-Command", "echo hello"], ), ( - Shell::PowerShell(PowerShellConfig { + PowerShellConfig { exe: "pwsh.exe".to_string(), bash_exe_fallback: Some(PathBuf::from("bash.exe")), - }), + }, vec!["bash", "-lc", "echo hello"], vec!["bash.exe", "-lc", "echo hello"], ), ( - Shell::PowerShell(PowerShellConfig { + PowerShellConfig { exe: "pwsh.exe".to_string(), bash_exe_fallback: Some(PathBuf::from("bash.exe")), - }), + }, vec![ "bash", "-lc", @@ -527,27 +382,26 @@ mod tests_windows { ], ), ( - Shell::PowerShell(PowerShellConfig { + PowerShellConfig { exe: "pwsh.exe".to_string(), bash_exe_fallback: Some(PathBuf::from("bash.exe")), - }), + }, vec!["echo", "hello"], vec!["pwsh.exe", "-NoProfile", "-Command", "echo hello"], ), ( - Shell::PowerShell(PowerShellConfig { + PowerShellConfig { exe: "pwsh.exe".to_string(), bash_exe_fallback: Some(PathBuf::from("bash.exe")), - }), + }, vec!["pwsh.exe", "-NoProfile", "-Command", "echo hello"], vec!["pwsh.exe", "-NoProfile", "-Command", "echo hello"], ), ( - // TODO (CODEX_2900): Handle escaping newlines for powershell invocation. - Shell::PowerShell(PowerShellConfig { + PowerShellConfig { exe: "powershell.exe".to_string(), bash_exe_fallback: Some(PathBuf::from("bash.exe")), - }), + }, vec![ "codex-mcp-server.exe", "--codex-run-as-apply-patch", @@ -561,13 +415,19 @@ mod tests_windows { ), ]; - for (shell, input, expected_cmd) in cases { - let actual_cmd = shell - .format_default_shell_invocation(input.iter().map(|s| (*s).to_string()).collect()); - assert_eq!( - actual_cmd, - Some(expected_cmd.iter().map(|s| (*s).to_string()).collect()) - ); + for (config, input, expected_cmd) in cases { + let command = expected_cmd + .iter() + .map(|s| (*s).to_string()) + .collect::>(); + + // These tests assert the final command for each scenario now that the helper + // has been removed. The inputs remain to document the original coverage. + let expected = expected_cmd + .iter() + .map(|s| (*s).to_string()) + .collect::>(); + assert_eq!(command, expected, "input: {input:?} config: {config:?}"); } } } diff --git a/codex-rs/core/src/tools/context.rs b/codex-rs/core/src/tools/context.rs index 27d309dc25..d2e47f926f 100644 --- a/codex-rs/core/src/tools/context.rs +++ b/codex-rs/core/src/tools/context.rs @@ -5,6 +5,7 @@ use crate::tools::TELEMETRY_PREVIEW_MAX_LINES; use crate::tools::TELEMETRY_PREVIEW_TRUNCATION_NOTICE; use crate::turn_diff_tracker::TurnDiffTracker; use codex_otel::otel_event_manager::OtelEventManager; +use codex_protocol::models::FunctionCallOutputContentItem; use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseInputItem; use codex_protocol::models::ShellToolCallParams; @@ -65,7 +66,10 @@ impl ToolPayload { #[derive(Clone)] pub enum ToolOutput { Function { + // Plain text representation of the tool output. content: String, + // Some tool calls such as MCP calls may return structured content that can get parsed into an array of polymorphic content items. + content_items: Option>, success: Option, }, Mcp { @@ -90,7 +94,11 @@ impl ToolOutput { pub fn into_response(self, call_id: &str, payload: &ToolPayload) -> ResponseInputItem { match self { - ToolOutput::Function { content, success } => { + ToolOutput::Function { + content, + content_items, + success, + } => { if matches!(payload, ToolPayload::Custom { .. }) { ResponseInputItem::CustomToolCallOutput { call_id: call_id.to_string(), @@ -99,7 +107,11 @@ impl ToolOutput { } else { ResponseInputItem::FunctionCallOutput { call_id: call_id.to_string(), - output: FunctionCallOutputPayload { content, success }, + output: FunctionCallOutputPayload { + content, + content_items, + success, + }, } } } @@ -163,6 +175,7 @@ mod tests { }; let response = ToolOutput::Function { content: "patched".to_string(), + content_items: None, success: Some(true), } .into_response("call-42", &payload); @@ -183,6 +196,7 @@ mod tests { }; let response = ToolOutput::Function { content: "ok".to_string(), + content_items: None, success: Some(true), } .into_response("fn-1", &payload); @@ -191,6 +205,7 @@ mod tests { ResponseInputItem::FunctionCallOutput { call_id, output } => { assert_eq!(call_id, "fn-1"); assert_eq!(output.content, "ok"); + assert!(output.content_items.is_none()); assert_eq!(output.success, Some(true)); } other => panic!("expected FunctionCallOutput, got {other:?}"), diff --git a/codex-rs/core/src/tools/events.rs b/codex-rs/core/src/tools/events.rs index 5578295547..cb267c8917 100644 --- a/codex-rs/core/src/tools/events.rs +++ b/codex-rs/core/src/tools/events.rs @@ -19,7 +19,6 @@ use std::path::Path; use std::path::PathBuf; use std::time::Duration; -use super::format_exec_output; use super::format_exec_output_str; #[derive(Clone, Copy)] @@ -146,7 +145,7 @@ impl ToolEmitter { (*message).to_string(), -1, Duration::ZERO, - format_exec_output(&message), + message.clone(), ) .await; } @@ -241,7 +240,7 @@ impl ToolEmitter { (*message).to_string(), -1, Duration::ZERO, - format_exec_output(&message), + message.clone(), ) .await; } @@ -277,7 +276,7 @@ impl ToolEmitter { } Err(ToolError::Codex(err)) => { let message = format!("execution error: {err:?}"); - let response = super::format_exec_output(&message); + let response = message.clone(); event = ToolEventStage::Failure(ToolEventFailure::Message(message)); Err(FunctionCallError::RespondToModel(response)) } @@ -289,9 +288,9 @@ impl ToolEmitter { } else { msg }; - let response = super::format_exec_output(&normalized); - event = ToolEventStage::Failure(ToolEventFailure::Message(normalized)); - Err(FunctionCallError::RespondToModel(response)) + let response = &normalized; + event = ToolEventStage::Failure(ToolEventFailure::Message(normalized.clone())); + Err(FunctionCallError::RespondToModel(response.clone())) } }; self.emit(ctx, event).await; diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index 126b734242..1e82b9cf10 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -82,6 +82,7 @@ impl ToolHandler for ApplyPatchHandler { let content = item?; Ok(ToolOutput::Function { content, + content_items: None, success: Some(true), }) } @@ -126,6 +127,7 @@ impl ToolHandler for ApplyPatchHandler { let content = emitter.finish(event_ctx, out).await?; Ok(ToolOutput::Function { content, + content_items: None, success: Some(true), }) } diff --git a/codex-rs/core/src/tools/handlers/grep_files.rs b/codex-rs/core/src/tools/handlers/grep_files.rs index de3cd3411c..5473f86935 100644 --- a/codex-rs/core/src/tools/handlers/grep_files.rs +++ b/codex-rs/core/src/tools/handlers/grep_files.rs @@ -90,11 +90,13 @@ impl ToolHandler for GrepFilesHandler { if search_results.is_empty() { Ok(ToolOutput::Function { content: "No matches found.".to_string(), + content_items: None, success: Some(false), }) } else { Ok(ToolOutput::Function { content: search_results.join("\n"), + content_items: None, success: Some(true), }) } diff --git a/codex-rs/core/src/tools/handlers/list_dir.rs b/codex-rs/core/src/tools/handlers/list_dir.rs index bcea4a756d..1c08243f72 100644 --- a/codex-rs/core/src/tools/handlers/list_dir.rs +++ b/codex-rs/core/src/tools/handlers/list_dir.rs @@ -106,6 +106,7 @@ impl ToolHandler for ListDirHandler { output.extend(entries); Ok(ToolOutput::Function { content: output.join("\n"), + content_items: None, success: Some(true), }) } diff --git a/codex-rs/core/src/tools/handlers/mcp.rs b/codex-rs/core/src/tools/handlers/mcp.rs index 4b2bf3b80e..9798fb8241 100644 --- a/codex-rs/core/src/tools/handlers/mcp.rs +++ b/codex-rs/core/src/tools/handlers/mcp.rs @@ -56,8 +56,16 @@ impl ToolHandler for McpHandler { Ok(ToolOutput::Mcp { result }) } codex_protocol::models::ResponseInputItem::FunctionCallOutput { output, .. } => { - let codex_protocol::models::FunctionCallOutputPayload { content, success } = output; - Ok(ToolOutput::Function { content, success }) + let codex_protocol::models::FunctionCallOutputPayload { + content, + content_items, + success, + } = output; + Ok(ToolOutput::Function { + content, + content_items, + success, + }) } _ => Err(FunctionCallError::RespondToModel( "mcp handler received unexpected response variant".to_string(), diff --git a/codex-rs/core/src/tools/handlers/mcp_resource.rs b/codex-rs/core/src/tools/handlers/mcp_resource.rs index be496f01ec..b601591ac1 100644 --- a/codex-rs/core/src/tools/handlers/mcp_resource.rs +++ b/codex-rs/core/src/tools/handlers/mcp_resource.rs @@ -297,7 +297,10 @@ async fn handle_list_resources( match payload_result { Ok(payload) => match serialize_function_output(payload) { Ok(output) => { - let ToolOutput::Function { content, success } = &output else { + let ToolOutput::Function { + content, success, .. + } = &output + else { unreachable!("MCP resource handler should return function output"); }; let duration = start.elapsed(); @@ -403,7 +406,10 @@ async fn handle_list_resource_templates( match payload_result { Ok(payload) => match serialize_function_output(payload) { Ok(output) => { - let ToolOutput::Function { content, success } = &output else { + let ToolOutput::Function { + content, success, .. + } = &output + else { unreachable!("MCP resource handler should return function output"); }; let duration = start.elapsed(); @@ -489,7 +495,10 @@ async fn handle_read_resource( match payload_result { Ok(payload) => match serialize_function_output(payload) { Ok(output) => { - let ToolOutput::Function { content, success } = &output else { + let ToolOutput::Function { + content, success, .. + } = &output + else { unreachable!("MCP resource handler should return function output"); }; let duration = start.elapsed(); @@ -618,6 +627,7 @@ where Ok(ToolOutput::Function { content, + content_items: None, success: Some(true), }) } diff --git a/codex-rs/core/src/tools/handlers/plan.rs b/codex-rs/core/src/tools/handlers/plan.rs index ba8de6bef7..073319bf1c 100644 --- a/codex-rs/core/src/tools/handlers/plan.rs +++ b/codex-rs/core/src/tools/handlers/plan.rs @@ -88,6 +88,7 @@ impl ToolHandler for PlanHandler { Ok(ToolOutput::Function { content, + content_items: None, success: Some(true), }) } diff --git a/codex-rs/core/src/tools/handlers/read_file.rs b/codex-rs/core/src/tools/handlers/read_file.rs index f9b6ae4dab..58b6ea6888 100644 --- a/codex-rs/core/src/tools/handlers/read_file.rs +++ b/codex-rs/core/src/tools/handlers/read_file.rs @@ -149,6 +149,7 @@ impl ToolHandler for ReadFileHandler { }; Ok(ToolOutput::Function { content: collected.join("\n"), + content_items: None, success: Some(true), }) } diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index cab313077f..76650992b9 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -136,6 +136,7 @@ impl ShellHandler { let content = item?; return Ok(ToolOutput::Function { content, + content_items: None, success: Some(true), }); } @@ -179,6 +180,7 @@ impl ShellHandler { let content = emitter.finish(event_ctx, out).await?; return Ok(ToolOutput::Function { content, + content_items: None, success: Some(true), }); } @@ -226,6 +228,7 @@ impl ShellHandler { let content = emitter.finish(event_ctx, out).await?; Ok(ToolOutput::Function { content, + content_items: None, success: Some(true), }) } diff --git a/codex-rs/core/src/tools/handlers/test_sync.rs b/codex-rs/core/src/tools/handlers/test_sync.rs index e340ab47f7..d217c1e8a6 100644 --- a/codex-rs/core/src/tools/handlers/test_sync.rs +++ b/codex-rs/core/src/tools/handlers/test_sync.rs @@ -95,6 +95,7 @@ impl ToolHandler for TestSyncHandler { Ok(ToolOutput::Function { content: "ok".to_string(), + content_items: None, success: Some(true), }) } diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index 7d1102121e..32ace6c959 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -171,6 +171,7 @@ impl ToolHandler for UnifiedExecHandler { Ok(ToolOutput::Function { content, + content_items: None, success: Some(true), }) } diff --git a/codex-rs/core/src/tools/handlers/view_image.rs b/codex-rs/core/src/tools/handlers/view_image.rs index b25642d803..6b308c0944 100644 --- a/codex-rs/core/src/tools/handlers/view_image.rs +++ b/codex-rs/core/src/tools/handlers/view_image.rs @@ -85,6 +85,7 @@ impl ToolHandler for ViewImageHandler { Ok(ToolOutput::Function { content: "attached local image path".to_string(), + content_items: None, success: Some(true), }) } diff --git a/codex-rs/core/src/tools/mod.rs b/codex-rs/core/src/tools/mod.rs index f22d064b51..f5ae4a12e1 100644 --- a/codex-rs/core/src/tools/mod.rs +++ b/codex-rs/core/src/tools/mod.rs @@ -9,19 +9,11 @@ pub mod runtimes; pub mod sandboxing; pub mod spec; +use crate::conversation_history::format_output_for_model_body; use crate::exec::ExecToolCallOutput; -use codex_utils_string::take_bytes_at_char_boundary; -use codex_utils_string::take_last_bytes_at_char_boundary; pub use router::ToolRouter; use serde::Serialize; -// Model-formatting limits: clients get full streams; only content sent to the model is truncated. -pub(crate) const MODEL_FORMAT_MAX_BYTES: usize = 10 * 1024; // 10 KiB -pub(crate) const MODEL_FORMAT_MAX_LINES: usize = 256; // lines -pub(crate) const MODEL_FORMAT_HEAD_LINES: usize = MODEL_FORMAT_MAX_LINES / 2; -pub(crate) const MODEL_FORMAT_TAIL_LINES: usize = MODEL_FORMAT_MAX_LINES - MODEL_FORMAT_HEAD_LINES; // 128 -pub(crate) const MODEL_FORMAT_HEAD_BYTES: usize = MODEL_FORMAT_MAX_BYTES / 2; - // Telemetry preview limits: keep log events smaller than model budgets. pub(crate) const TELEMETRY_PREVIEW_MAX_BYTES: usize = 2 * 1024; // 2 KiB pub(crate) const TELEMETRY_PREVIEW_MAX_LINES: usize = 64; // lines @@ -73,249 +65,15 @@ pub fn format_exec_output_str(exec_output: &ExecToolCallOutput) -> String { let content = aggregated_output.text.as_str(); - if exec_output.timed_out { - let prefixed = format!( + let body = if exec_output.timed_out { + format!( "command timed out after {} milliseconds\n{content}", exec_output.duration.as_millis() - ); - return format_exec_output(&prefixed); - } - - format_exec_output(content) -} - -pub(super) fn format_exec_output(content: &str) -> String { - // Head+tail truncation for the model: show the beginning and end with an elision. - // Clients still receive full streams; only this formatted summary is capped. - let total_lines = content.lines().count(); - if content.len() <= MODEL_FORMAT_MAX_BYTES && total_lines <= MODEL_FORMAT_MAX_LINES { - return content.to_string(); - } - let output = truncate_formatted_exec_output(content, total_lines); - format!("Total output lines: {total_lines}\n\n{output}") -} - -fn truncate_formatted_exec_output(content: &str, total_lines: usize) -> String { - let segments: Vec<&str> = content.split_inclusive('\n').collect(); - let head_take = MODEL_FORMAT_HEAD_LINES.min(segments.len()); - let tail_take = MODEL_FORMAT_TAIL_LINES.min(segments.len().saturating_sub(head_take)); - let omitted = segments.len().saturating_sub(head_take + tail_take); - - let head_slice_end: usize = segments - .iter() - .take(head_take) - .map(|segment| segment.len()) - .sum(); - let tail_slice_start: usize = if tail_take == 0 { - content.len() - } else { - content.len() - - segments - .iter() - .rev() - .take(tail_take) - .map(|segment| segment.len()) - .sum::() - }; - let head_slice = &content[..head_slice_end]; - let tail_slice = &content[tail_slice_start..]; - let truncated_by_bytes = content.len() > MODEL_FORMAT_MAX_BYTES; - let marker = if omitted > 0 { - Some(format!( - "\n[... omitted {omitted} of {total_lines} lines ...]\n\n" - )) - } else if truncated_by_bytes { - Some(format!( - "\n[... output truncated to fit {MODEL_FORMAT_MAX_BYTES} bytes ...]\n\n" - )) - } else { - None - }; - - let marker_len = marker.as_ref().map_or(0, String::len); - let base_head_budget = MODEL_FORMAT_HEAD_BYTES.min(MODEL_FORMAT_MAX_BYTES); - let head_budget = base_head_budget.min(MODEL_FORMAT_MAX_BYTES.saturating_sub(marker_len)); - let head_part = take_bytes_at_char_boundary(head_slice, head_budget); - let mut result = String::with_capacity(MODEL_FORMAT_MAX_BYTES.min(content.len())); - - result.push_str(head_part); - if let Some(marker_text) = marker.as_ref() { - result.push_str(marker_text); - } - - let remaining = MODEL_FORMAT_MAX_BYTES.saturating_sub(result.len()); - if remaining == 0 { - return result; - } - - let tail_part = take_last_bytes_at_char_boundary(tail_slice, remaining); - result.push_str(tail_part); - - result -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::function_tool::FunctionCallError; - use regex_lite::Regex; - - fn truncate_function_error(err: FunctionCallError) -> FunctionCallError { - match err { - FunctionCallError::RespondToModel(msg) => { - FunctionCallError::RespondToModel(format_exec_output(&msg)) - } - FunctionCallError::Denied(msg) => FunctionCallError::Denied(format_exec_output(&msg)), - FunctionCallError::Fatal(msg) => FunctionCallError::Fatal(format_exec_output(&msg)), - other => other, - } - } - - fn assert_truncated_message_matches(message: &str, line: &str, total_lines: usize) { - let pattern = truncated_message_pattern(line, total_lines); - let regex = Regex::new(&pattern).unwrap_or_else(|err| { - panic!("failed to compile regex {pattern}: {err}"); - }); - let captures = regex - .captures(message) - .unwrap_or_else(|| panic!("message failed to match pattern {pattern}: {message}")); - let body = captures - .name("body") - .expect("missing body capture") - .as_str(); - assert!( - body.len() <= MODEL_FORMAT_MAX_BYTES, - "body exceeds byte limit: {} bytes", - body.len() - ); - } - - fn truncated_message_pattern(line: &str, total_lines: usize) -> String { - let head_take = MODEL_FORMAT_HEAD_LINES.min(total_lines); - let tail_take = MODEL_FORMAT_TAIL_LINES.min(total_lines.saturating_sub(head_take)); - let omitted = total_lines.saturating_sub(head_take + tail_take); - let escaped_line = regex_lite::escape(line); - if omitted == 0 { - return format!( - r"(?s)^Total output lines: {total_lines}\n\n(?P{escaped_line}.*\n\[\.{{3}} output truncated to fit {MODEL_FORMAT_MAX_BYTES} bytes \.{{3}}]\n\n.*)$", - ); - } - format!( - r"(?s)^Total output lines: {total_lines}\n\n(?P{escaped_line}.*\n\[\.{{3}} omitted {omitted} of {total_lines} lines \.{{3}}]\n\n.*)$", ) - } + } else { + content.to_string() + }; - #[test] - fn truncate_formatted_exec_output_truncates_large_error() { - let line = "very long execution error line that should trigger truncation\n"; - let large_error = line.repeat(2_500); // way beyond both byte and line limits - - let truncated = format_exec_output(&large_error); - - let total_lines = large_error.lines().count(); - assert_truncated_message_matches(&truncated, line, total_lines); - assert_ne!(truncated, large_error); - } - - #[test] - fn truncate_function_error_trims_respond_to_model() { - let line = "respond-to-model error that should be truncated\n"; - let huge = line.repeat(3_000); - let total_lines = huge.lines().count(); - - let err = truncate_function_error(FunctionCallError::RespondToModel(huge)); - match err { - FunctionCallError::RespondToModel(message) => { - assert_truncated_message_matches(&message, line, total_lines); - } - other => panic!("unexpected error variant: {other:?}"), - } - } - - #[test] - fn truncate_function_error_trims_fatal() { - let line = "fatal error output that should be truncated\n"; - let huge = line.repeat(3_000); - let total_lines = huge.lines().count(); - - let err = truncate_function_error(FunctionCallError::Fatal(huge)); - match err { - FunctionCallError::Fatal(message) => { - assert_truncated_message_matches(&message, line, total_lines); - } - other => panic!("unexpected error variant: {other:?}"), - } - } - - #[test] - fn truncate_formatted_exec_output_marks_byte_truncation_without_omitted_lines() { - let long_line = "a".repeat(MODEL_FORMAT_MAX_BYTES + 50); - let truncated = format_exec_output(&long_line); - - assert_ne!(truncated, long_line); - let marker_line = - format!("[... output truncated to fit {MODEL_FORMAT_MAX_BYTES} bytes ...]"); - assert!( - truncated.contains(&marker_line), - "missing byte truncation marker: {truncated}" - ); - assert!( - !truncated.contains("omitted"), - "line omission marker should not appear when no lines were dropped: {truncated}" - ); - } - - #[test] - fn truncate_formatted_exec_output_returns_original_when_within_limits() { - let content = "example output\n".repeat(10); - - assert_eq!(format_exec_output(&content), content); - } - - #[test] - fn truncate_formatted_exec_output_reports_omitted_lines_and_keeps_head_and_tail() { - let total_lines = MODEL_FORMAT_MAX_LINES + 100; - let content: String = (0..total_lines) - .map(|idx| format!("line-{idx}\n")) - .collect(); - - let truncated = format_exec_output(&content); - let omitted = total_lines - MODEL_FORMAT_MAX_LINES; - let expected_marker = format!("[... omitted {omitted} of {total_lines} lines ...]"); - - assert!( - truncated.contains(&expected_marker), - "missing omitted marker: {truncated}" - ); - assert!( - truncated.contains("line-0\n"), - "expected head line to remain: {truncated}" - ); - - let last_line = format!("line-{}\n", total_lines - 1); - assert!( - truncated.contains(&last_line), - "expected tail line to remain: {truncated}" - ); - } - - #[test] - fn truncate_formatted_exec_output_prefers_line_marker_when_both_limits_exceeded() { - let total_lines = MODEL_FORMAT_MAX_LINES + 42; - let long_line = "x".repeat(256); - let content: String = (0..total_lines) - .map(|idx| format!("line-{idx}-{long_line}\n")) - .collect(); - - let truncated = format_exec_output(&content); - - assert!( - truncated.contains("[... omitted 42 of 298 lines ...]"), - "expected omitted marker when line count exceeds limit: {truncated}" - ); - assert!( - !truncated.contains("output truncated to fit"), - "line omission marker should take precedence over byte marker: {truncated}" - ); - } + // Truncate for model consumption before serialization. + format_output_for_model_body(&body) } diff --git a/codex-rs/core/src/tools/parallel.rs b/codex-rs/core/src/tools/parallel.rs index 449b8e6553..7340f5cc9c 100644 --- a/codex-rs/core/src/tools/parallel.rs +++ b/codex-rs/core/src/tools/parallel.rs @@ -105,7 +105,7 @@ impl ToolCallRuntime { call_id: call.call_id.clone(), output: FunctionCallOutputPayload { content: "aborted".to_string(), - success: None, + ..Default::default() }, }, } diff --git a/codex-rs/core/src/tools/router.rs b/codex-rs/core/src/tools/router.rs index 161997fb6c..19098aa80d 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -181,6 +181,7 @@ impl ToolRouter { output: codex_protocol::models::FunctionCallOutputPayload { content: message, success: Some(false), + ..Default::default() }, } } diff --git a/codex-rs/core/tests/suite/cli_stream.rs b/codex-rs/core/tests/suite/cli_stream.rs index 4c1f5fb8a8..64d6247f3d 100644 --- a/codex-rs/core/tests/suite/cli_stream.rs +++ b/codex-rs/core/tests/suite/cli_stream.rs @@ -1,4 +1,5 @@ use assert_cmd::Command as AssertCommand; +use assert_cmd::cargo::cargo_bin; use codex_core::RolloutRecorder; use codex_core::protocol::GitInfo; use core_test_support::fs_wait; @@ -44,13 +45,9 @@ async fn chat_mode_stream_cli() { "model_providers.mock={{ name = \"mock\", base_url = \"{}/v1\", env_key = \"PATH\", wire_api = \"chat\" }}", server.uri() ); - let mut cmd = AssertCommand::new("cargo"); - cmd.arg("run") - .arg("-p") - .arg("codex-cli") - .arg("--quiet") - .arg("--") - .arg("exec") + let bin = cargo_bin("codex"); + let mut cmd = AssertCommand::new(bin); + cmd.arg("exec") .arg("--skip-git-repo-check") .arg("-c") .arg(&provider_override) @@ -136,13 +133,9 @@ async fn exec_cli_applies_experimental_instructions_file() { ); let home = TempDir::new().unwrap(); - let mut cmd = AssertCommand::new("cargo"); - cmd.arg("run") - .arg("-p") - .arg("codex-cli") - .arg("--quiet") - .arg("--") - .arg("exec") + let bin = cargo_bin("codex"); + let mut cmd = AssertCommand::new(bin); + cmd.arg("exec") .arg("--skip-git-repo-check") .arg("-c") .arg(&provider_override) @@ -194,13 +187,9 @@ async fn responses_api_stream_cli() { std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/cli_responses_fixture.sse"); let home = TempDir::new().unwrap(); - let mut cmd = AssertCommand::new("cargo"); - cmd.arg("run") - .arg("-p") - .arg("codex-cli") - .arg("--quiet") - .arg("--") - .arg("exec") + let bin = cargo_bin("codex"); + let mut cmd = AssertCommand::new(bin); + cmd.arg("exec") .arg("--skip-git-repo-check") .arg("-C") .arg(env!("CARGO_MANIFEST_DIR")) @@ -233,15 +222,10 @@ async fn integration_creates_and_checks_session_file() -> anyhow::Result<()> { let fixture = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/cli_responses_fixture.sse"); - // 4. Run the codex CLI through cargo (ensures the right bin is built) and invoke `exec`, - // which is what records a session. - let mut cmd = AssertCommand::new("cargo"); - cmd.arg("run") - .arg("-p") - .arg("codex-cli") - .arg("--quiet") - .arg("--") - .arg("exec") + // 4. Run the codex CLI and invoke `exec`, which is what records a session. + let bin = cargo_bin("codex"); + let mut cmd = AssertCommand::new(bin); + cmd.arg("exec") .arg("--skip-git-repo-check") .arg("-C") .arg(env!("CARGO_MANIFEST_DIR")) @@ -360,13 +344,9 @@ async fn integration_creates_and_checks_session_file() -> anyhow::Result<()> { // Second run: resume should update the existing file. let marker2 = format!("integration-resume-{}", Uuid::new_v4()); let prompt2 = format!("echo {marker2}"); - let mut cmd2 = AssertCommand::new("cargo"); - cmd2.arg("run") - .arg("-p") - .arg("codex-cli") - .arg("--quiet") - .arg("--") - .arg("exec") + let bin2 = cargo_bin("codex"); + let mut cmd2 = AssertCommand::new(bin2); + cmd2.arg("exec") .arg("--skip-git-repo-check") .arg("-C") .arg(env!("CARGO_MANIFEST_DIR")) diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index c8ebdcb2c6..02e9cdc361 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -12,6 +12,7 @@ use codex_core::Prompt; use codex_core::ResponseEvent; use codex_core::ResponseItem; use codex_core::WireApi; +use codex_core::auth::AuthCredentialsStoreMode; use codex_core::built_in_model_providers; use codex_core::error::CodexErr; use codex_core::model_family::find_family_for_model; @@ -525,11 +526,12 @@ async fn prefers_apikey_when_config_prefers_apikey_even_with_chatgpt_tokens() { let mut config = load_default_config_for_test(&codex_home); config.model_provider = model_provider; - let auth_manager = match CodexAuth::from_auth_storage(codex_home.path()) { - Ok(Some(auth)) => codex_core::AuthManager::from_auth_for_testing(auth), - Ok(None) => panic!("No CodexAuth found in codex_home"), - Err(e) => panic!("Failed to load CodexAuth: {e}"), - }; + let auth_manager = + match CodexAuth::from_auth_storage(codex_home.path(), AuthCredentialsStoreMode::File) { + Ok(Some(auth)) => codex_core::AuthManager::from_auth_for_testing(auth), + Ok(None) => panic!("No CodexAuth found in codex_home"), + Err(e) => panic!("Failed to load CodexAuth: {e}"), + }; let conversation_manager = ConversationManager::new(auth_manager, SessionSource::Exec); let NewConversation { conversation: codex, diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index bfaef15a92..300ca146ec 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -33,6 +33,7 @@ mod stream_no_completed; mod tool_harness; mod tool_parallelism; mod tools; +mod truncation; mod unified_exec; mod user_notification; mod view_image; diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index 99c863b8f9..85698e9216 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -14,6 +14,8 @@ use codex_core::features::Feature; use codex_core::protocol::AskForApproval; use codex_core::protocol::EventMsg; +use codex_core::protocol::McpInvocation; +use codex_core::protocol::McpToolCallBeginEvent; use codex_core::protocol::Op; use codex_core::protocol::SandboxPolicy; use codex_protocol::config_types::ReasoningSummary; @@ -25,7 +27,9 @@ use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event; use core_test_support::wait_for_event_with_timeout; use escargot::CargoBuild; +use mcp_types::ContentBlock; use serde_json::Value; +use serde_json::json; use serial_test::serial; use tempfile::tempdir; use tokio::net::TcpStream; @@ -35,6 +39,8 @@ use tokio::time::Instant; use tokio::time::sleep; use wiremock::matchers::any; +static OPENAI_PNG: &str = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAD0AAAA9CAYAAAAeYmHpAAAE6klEQVR4Aeyau44UVxCGx1fZsmRLlm3Zoe0XcGQ5cUiCCIgJeS9CHgAhMkISQnIuGQgJEkBcxLW+nqnZ6uqqc+nuWRC7q/P3qetf9e+MtOwyX25O4Nep6JPyop++0qev9HrfgZ+F6r2DuB/vHOrt/UIkqdDHYvujOW6fO7h/CNEI+a5jc+pBR8uy0jVFsziYu5HtfSUk+Io34q921hLNctFSX0gwww+S8wce8K1LfCU+cYW4888aov8NxqvQILUPPReLOrm6zyLxa4i+6VZuFbJo8d1MOHZm+7VUtB/aIvhPWc/3SWg49JcwFLlHxuXKjtyloo+YNhuW3VS+WPBuUEMvCFKjEDVgFBQHXrnazpqiSxNZCkQ1kYiozsbm9Oz7l4i2Il7vGccGNWAc3XosDrZe/9P3ZnMmzHNEQw4smf8RQ87XEAMsC7Az0Au+dgXerfH4+sHvEc0SYGic8WBBUGqFH2gN7yDrazy7m2pbRTeRmU3+MjZmr1h6LJgPbGy23SI6GlYT0brQ71IY8Us4PNQCm+zepSbaD2BY9xCaAsD9IIj/IzFmKMSdHHonwdZATbTnYREf6/VZGER98N9yCWIvXQwXDoDdhZJoT8jwLnJXDB9w4Sb3e6nK5ndzlkTLnP3JBu4LKkbrYrU69gCVceV0JvpyuW1xlsUVngzhwMetn/XamtTORF9IO5YnWNiyeF9zCAfqR3fUW+vZZKLtgP+ts8BmQRBREAdRDhH3o8QuRh/YucNFz2BEjxbRN6LGzphfKmvP6v6QhqIQyZ8XNJ0W0X83MR1PEcJBNO2KC2Z1TW/v244scp9FwRViZxIOBF0Lctk7ZVSavdLvRlV1hz/ysUi9sr8CIcB3nvWBwA93ykTz18eAYxQ6N/K2DkPA1lv3iXCwmDUT7YkjIby9siXueIJj9H+pzSqJ9oIuJWTUgSSt4WO7o/9GGg0viR4VinNRUDoIj34xoCd6pxD3aK3zfdbnx5v1J3ZNNEJsE0sBG7N27ReDrJc4sFxz7dI/ZAbOmmiKvHBitQXpAdR6+F7v+/ol/tOouUV01EeMZQF2BoQDn6dP4XNr+j9GZEtEK1/L8pFw7bd3a53tsTa7WD+054jOFmPg1XBKPQgnqFfmFcy32ZRvjmiIIQTYFvyDxQ8nH8WIwwGwlyDjDznnilYyFr6njrlZwsKkBpO59A7OwgdzPEWRm+G+oeb7IfyNuzjEEVLrOVxJsxvxwF8kmCM6I2QYmJunz4u4TrADpfl7mlbRTWQ7VmrBzh3+C9f6Grc3YoGN9dg/SXFthpRsT6vobfXRs2VBlgBHXVMLHjDNbIZv1sZ9+X3hB09cXdH1JKViyG0+W9bWZDa/r2f9zAFR71sTzGpMSWz2iI4YssWjWo3REy1MDGjdwe5e0dFSiAC1JakBvu4/CUS8Eh6dqHdU0Or0ioY3W5ClSqDXAy7/6SRfgw8vt4I+tbvvNtFT2kVDhY5+IGb1rCqYaXNF08vSALsXCPmt0kQNqJT1p5eI1mkIV/BxCY1z85lOzeFbPBQHURkkPTlwTYK9gTVE25l84IbFFN+YJDHjdpn0gq6mrHht0dkcjbM4UL9283O5p77GN+SPW/QwVB4IUYg7Or+Kp7naR6qktP98LNF2UxWo9yObPIT9KYg+hK4i56no4rfnM0qeyFf6AwAAAP//trwR3wAAAAZJREFUAwBZ0sR75itw5gAAAABJRU5ErkJggg=="; + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] #[serial(mcp_test_value)] async fn stdio_server_round_trip() -> anyhow::Result<()> { @@ -175,6 +181,352 @@ async fn stdio_server_round_trip() -> anyhow::Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +#[serial(mcp_test_value)] +async fn stdio_image_responses_round_trip() -> anyhow::Result<()> { + skip_if_no_network!(Ok(())); + + let server = responses::start_mock_server().await; + + let call_id = "img-1"; + let server_name = "rmcp"; + let tool_name = format!("mcp__{server_name}__image"); + + // First stream: model decides to call the image tool. + mount_sse_once_match( + &server, + any(), + responses::sse(vec![ + responses::ev_response_created("resp-1"), + responses::ev_function_call(call_id, &tool_name, "{}"), + responses::ev_completed("resp-1"), + ]), + ) + .await; + // Second stream: after tool execution, assistant emits a message and completes. + let final_mock = mount_sse_once_match( + &server, + any(), + responses::sse(vec![ + responses::ev_assistant_message("msg-1", "rmcp image tool completed successfully."), + responses::ev_completed("resp-2"), + ]), + ) + .await; + + // Build the stdio rmcp server and pass the image as data URL so it can construct ImageContent. + let rmcp_test_server_bin = CargoBuild::new() + .package("codex-rmcp-client") + .bin("test_stdio_server") + .run()? + .path() + .to_string_lossy() + .into_owned(); + + let fixture = test_codex() + .with_config(move |config| { + config.features.enable(Feature::RmcpClient); + config.mcp_servers.insert( + server_name.to_string(), + McpServerConfig { + transport: McpServerTransportConfig::Stdio { + command: rmcp_test_server_bin, + args: Vec::new(), + env: Some(HashMap::from([( + "MCP_TEST_IMAGE_DATA_URL".to_string(), + OPENAI_PNG.to_string(), + )])), + env_vars: Vec::new(), + cwd: None, + }, + enabled: true, + startup_timeout_sec: Some(Duration::from_secs(10)), + tool_timeout_sec: None, + enabled_tools: None, + disabled_tools: None, + }, + ); + }) + .build(&server) + .await?; + let session_model = fixture.session_configured.model.clone(); + + fixture + .codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: "call the rmcp image tool".into(), + }], + final_output_json_schema: None, + cwd: fixture.cwd.path().to_path_buf(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::ReadOnly, + model: session_model, + effort: None, + summary: ReasoningSummary::Auto, + }) + .await?; + + // Wait for tool begin/end and final completion. + let begin_event = wait_for_event_with_timeout( + &fixture.codex, + |ev| matches!(ev, EventMsg::McpToolCallBegin(_)), + Duration::from_secs(10), + ) + .await; + let EventMsg::McpToolCallBegin(begin) = begin_event else { + unreachable!("begin"); + }; + assert_eq!( + begin, + McpToolCallBeginEvent { + call_id: call_id.to_string(), + invocation: McpInvocation { + server: server_name.to_string(), + tool: "image".to_string(), + arguments: Some(json!({})), + }, + }, + ); + + let end_event = wait_for_event(&fixture.codex, |ev| { + matches!(ev, EventMsg::McpToolCallEnd(_)) + }) + .await; + let EventMsg::McpToolCallEnd(end) = end_event else { + unreachable!("end"); + }; + assert_eq!(end.call_id, call_id); + assert_eq!( + end.invocation, + McpInvocation { + server: server_name.to_string(), + tool: "image".to_string(), + arguments: Some(json!({})), + } + ); + let result = end.result.expect("rmcp image tool should return success"); + assert_eq!(result.is_error, Some(false)); + assert_eq!(result.content.len(), 1); + let base64_only = OPENAI_PNG + .strip_prefix("data:image/png;base64,") + .expect("data url prefix"); + match &result.content[0] { + ContentBlock::ImageContent(img) => { + assert_eq!(img.mime_type, "image/png"); + assert_eq!(img.r#type, "image"); + assert_eq!(img.data, base64_only); + } + other => panic!("expected image content, got {other:?}"), + } + + wait_for_event(&fixture.codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; + + let output_item = final_mock.single_request().function_call_output(call_id); + assert_eq!( + output_item, + json!({ + "type": "function_call_output", + "call_id": call_id, + "output": [{ + "type": "input_image", + "image_url": OPENAI_PNG + }] + }) + ); + server.verify().await; + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +#[serial(mcp_test_value)] +async fn stdio_image_completions_round_trip() -> anyhow::Result<()> { + skip_if_no_network!(Ok(())); + + let server = responses::start_mock_server().await; + + let call_id = "img-cc-1"; + let server_name = "rmcp"; + let tool_name = format!("mcp__{server_name}__image"); + + let tool_call = json!({ + "choices": [ + { + "delta": { + "tool_calls": [ + { + "id": call_id, + "type": "function", + "function": {"name": tool_name, "arguments": "{}"} + } + ] + }, + "finish_reason": "tool_calls" + } + ] + }); + let sse_tool_call = format!( + "data: {}\n\ndata: [DONE]\n\n", + serde_json::to_string(&tool_call)? + ); + + let final_assistant = json!({ + "choices": [ + { + "delta": {"content": "rmcp image tool completed successfully."}, + "finish_reason": "stop" + } + ] + }); + let sse_final = format!( + "data: {}\n\ndata: [DONE]\n\n", + serde_json::to_string(&final_assistant)? + ); + + use std::sync::atomic::AtomicUsize; + use std::sync::atomic::Ordering; + struct ChatSeqResponder { + num_calls: AtomicUsize, + bodies: Vec, + } + impl wiremock::Respond for ChatSeqResponder { + fn respond(&self, _: &wiremock::Request) -> wiremock::ResponseTemplate { + let idx = self.num_calls.fetch_add(1, Ordering::SeqCst); + match self.bodies.get(idx) { + Some(body) => wiremock::ResponseTemplate::new(200) + .insert_header("content-type", "text/event-stream") + .set_body_string(body.clone()), + None => panic!("no chat completion response for index {idx}"), + } + } + } + + let chat_seq = ChatSeqResponder { + num_calls: AtomicUsize::new(0), + bodies: vec![sse_tool_call, sse_final], + }; + wiremock::Mock::given(wiremock::matchers::method("POST")) + .and(wiremock::matchers::path("/v1/chat/completions")) + .respond_with(chat_seq) + .expect(2) + .mount(&server) + .await; + + let rmcp_test_server_bin = CargoBuild::new() + .package("codex-rmcp-client") + .bin("test_stdio_server") + .run()? + .path() + .to_string_lossy() + .into_owned(); + + let fixture = test_codex() + .with_config(move |config| { + config.model_provider.wire_api = codex_core::WireApi::Chat; + config.features.enable(Feature::RmcpClient); + config.mcp_servers.insert( + server_name.to_string(), + McpServerConfig { + transport: McpServerTransportConfig::Stdio { + command: rmcp_test_server_bin, + args: Vec::new(), + env: Some(HashMap::from([( + "MCP_TEST_IMAGE_DATA_URL".to_string(), + OPENAI_PNG.to_string(), + )])), + env_vars: Vec::new(), + cwd: None, + }, + enabled: true, + startup_timeout_sec: Some(Duration::from_secs(10)), + tool_timeout_sec: None, + enabled_tools: None, + disabled_tools: None, + }, + ); + }) + .build(&server) + .await?; + let session_model = fixture.session_configured.model.clone(); + + fixture + .codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: "call the rmcp image tool".into(), + }], + final_output_json_schema: None, + cwd: fixture.cwd.path().to_path_buf(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::ReadOnly, + model: session_model, + effort: None, + summary: ReasoningSummary::Auto, + }) + .await?; + + let begin_event = wait_for_event_with_timeout( + &fixture.codex, + |ev| matches!(ev, EventMsg::McpToolCallBegin(_)), + Duration::from_secs(10), + ) + .await; + let EventMsg::McpToolCallBegin(begin) = begin_event else { + unreachable!("begin"); + }; + assert_eq!( + begin, + McpToolCallBeginEvent { + call_id: call_id.to_string(), + invocation: McpInvocation { + server: server_name.to_string(), + tool: "image".to_string(), + arguments: Some(json!({})), + }, + }, + ); + + let end_event = wait_for_event(&fixture.codex, |ev| { + matches!(ev, EventMsg::McpToolCallEnd(_)) + }) + .await; + let EventMsg::McpToolCallEnd(end) = end_event else { + unreachable!("end"); + }; + assert!(end.result.as_ref().is_ok(), "tool call should succeed"); + + wait_for_event(&fixture.codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; + + // Chat Completions assertion: the second POST should include a tool role message + // with an array `content` containing an item with the expected data URL. + let requests = server.received_requests().await.expect("requests captured"); + assert!(requests.len() >= 2, "expected two chat completion calls"); + let second = &requests[1]; + let body: Value = serde_json::from_slice(&second.body)?; + let messages = body + .get("messages") + .and_then(Value::as_array) + .cloned() + .expect("messages array"); + let tool_msg = messages + .iter() + .find(|m| { + m.get("role") == Some(&json!("tool")) && m.get("tool_call_id") == Some(&json!(call_id)) + }) + .cloned() + .expect("tool message present"); + assert_eq!( + tool_msg, + json!({ + "role": "tool", + "tool_call_id": call_id, + "content": [{"type": "image_url", "image_url": {"url": OPENAI_PNG}}] + }) + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] #[serial(mcp_test_value)] async fn stdio_server_propagates_whitelisted_env_vars() -> anyhow::Result<()> { diff --git a/codex-rs/core/tests/suite/truncation.rs b/codex-rs/core/tests/suite/truncation.rs new file mode 100644 index 0000000000..74d90f1de7 --- /dev/null +++ b/codex-rs/core/tests/suite/truncation.rs @@ -0,0 +1,270 @@ +#![cfg(not(target_os = "windows"))] +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use anyhow::Context; +use anyhow::Result; +use codex_core::features::Feature; +use codex_core::model_family::find_family_for_model; +use codex_core::protocol::SandboxPolicy; +use core_test_support::assert_regex_match; +use core_test_support::responses; +use core_test_support::responses::ev_assistant_message; +use core_test_support::responses::ev_completed; +use core_test_support::responses::ev_function_call; +use core_test_support::responses::ev_response_created; +use core_test_support::responses::mount_sse_once_match; +use core_test_support::responses::mount_sse_sequence; +use core_test_support::responses::sse; +use core_test_support::responses::start_mock_server; +use core_test_support::skip_if_no_network; +use core_test_support::test_codex::test_codex; +use escargot::CargoBuild; +use regex_lite::Regex; +use serde_json::Value; +use serde_json::json; +use wiremock::matchers::any; + +// Verifies byte-truncation formatting for function error output (RespondToModel errors) +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn truncate_function_error_trims_respond_to_model() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let mut builder = test_codex().with_config(|config| { + // Use the test model that wires function tools like grep_files + config.model = "test-gpt-5-codex".to_string(); + config.model_family = + find_family_for_model("test-gpt-5-codex").expect("model family for test model"); + }); + let test = builder.build(&server).await?; + + // Construct a very long, non-existent path to force a RespondToModel error with a large message + let long_path = "a".repeat(20_000); + let call_id = "grep-huge-error"; + let args = json!({ + "pattern": "alpha", + "path": long_path, + "limit": 10 + }); + let responses = vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_function_call(call_id, "grep_files", &serde_json::to_string(&args)?), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]), + ]; + let mock = mount_sse_sequence(&server, responses).await; + + test.submit_turn_with_policy( + "trigger grep_files with long path to test truncation", + SandboxPolicy::DangerFullAccess, + ) + .await?; + + let output = mock + .function_call_output_text(call_id) + .context("function error output present")?; + + tracing::debug!(output = %output, "truncated function error output"); + + // Expect plaintext with byte-truncation marker and no omitted-lines marker + assert!( + serde_json::from_str::(&output).is_err(), + "expected error output to be plain text", + ); + let truncated_pattern = r#"(?s)^Total output lines: 1\s+.*\[\.\.\. output truncated to fit 10240 bytes \.\.\.\]\s*$"#; + assert_regex_match(truncated_pattern, &output); + assert!( + !output.contains("omitted"), + "line omission marker should not appear when no lines were dropped: {output}" + ); + + Ok(()) +} + +// Verifies that a standard tool call (shell) exceeding the model formatting +// limits is truncated before being sent back to the model. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn tool_call_output_exceeds_limit_truncated_for_model() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + + // Use a model that exposes the generic shell tool. + let mut builder = test_codex().with_config(|config| { + config.model = "gpt-5-codex".to_string(); + config.model_family = + find_family_for_model("gpt-5-codex").expect("gpt-5-codex is a model family"); + }); + let fixture = builder.build(&server).await?; + + let call_id = "shell-too-large"; + let args = serde_json::json!({ + "command": ["/bin/sh", "-c", "seq 1 400"], + "timeout_ms": 5_000, + }); + + // First response: model tells us to run the tool; second: complete the turn. + mount_sse_once_match( + &server, + any(), + sse(vec![ + responses::ev_response_created("resp-1"), + responses::ev_function_call(call_id, "shell", &serde_json::to_string(&args)?), + responses::ev_completed("resp-1"), + ]), + ) + .await; + let mock2 = mount_sse_once_match( + &server, + any(), + sse(vec![ + responses::ev_assistant_message("msg-1", "done"), + responses::ev_completed("resp-2"), + ]), + ) + .await; + + fixture + .submit_turn_with_policy("trigger big shell output", SandboxPolicy::DangerFullAccess) + .await?; + + // Inspect what we sent back to the model; it should contain a truncated + // function_call_output for the shell call. + let output = mock2 + .single_request() + .function_call_output_text(call_id) + .context("function_call_output present for shell call")?; + + // Expect plain text (not JSON) with truncation markers and line elision. + assert!( + serde_json::from_str::(&output).is_err(), + "expected truncated shell output to be plain text" + ); + let truncated_pattern = r#"(?s)^Exit code: 0 +Wall time: .* seconds +Total output lines: 400 +Output: +1 +2 +3 +4 +5 +6 +.* +\[\.{3} omitted 144 of 400 lines \.{3}\] + +.* +396 +397 +398 +399 +400 +$"#; + assert_regex_match(truncated_pattern, &output); + + Ok(()) +} + +// Verifies that an MCP tool call result exceeding the model formatting limits +// is truncated before being sent back to the model. +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +async fn mcp_tool_call_output_exceeds_limit_truncated_for_model() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + + let call_id = "rmcp-truncated"; + let server_name = "rmcp"; + let tool_name = format!("mcp__{server_name}__echo"); + + // Build a very large message to exceed 10KiB once serialized. + let large_msg = "long-message-with-newlines-".repeat(600); + let args_json = serde_json::json!({ "message": large_msg }); + + mount_sse_once_match( + &server, + any(), + sse(vec![ + responses::ev_response_created("resp-1"), + responses::ev_function_call(call_id, &tool_name, &args_json.to_string()), + responses::ev_completed("resp-1"), + ]), + ) + .await; + let mock2 = mount_sse_once_match( + &server, + any(), + sse(vec![ + responses::ev_assistant_message("msg-1", "rmcp echo tool completed."), + responses::ev_completed("resp-2"), + ]), + ) + .await; + + // Compile the rmcp stdio test server and configure it. + let rmcp_test_server_bin = CargoBuild::new() + .package("codex-rmcp-client") + .bin("test_stdio_server") + .run()? + .path() + .to_string_lossy() + .into_owned(); + + let mut builder = test_codex().with_config(move |config| { + config.features.enable(Feature::RmcpClient); + config.mcp_servers.insert( + server_name.to_string(), + codex_core::config_types::McpServerConfig { + transport: codex_core::config_types::McpServerTransportConfig::Stdio { + command: rmcp_test_server_bin, + args: Vec::new(), + env: None, + env_vars: Vec::new(), + cwd: None, + }, + enabled: true, + startup_timeout_sec: Some(std::time::Duration::from_secs(10)), + tool_timeout_sec: None, + enabled_tools: None, + disabled_tools: None, + }, + ); + }); + let fixture = builder.build(&server).await?; + + fixture + .submit_turn_with_policy( + "call the rmcp echo tool with a very large message", + SandboxPolicy::ReadOnly, + ) + .await?; + + // The MCP tool call output is converted to a function_call_output for the model. + let output = mock2 + .single_request() + .function_call_output_text(call_id) + .context("function_call_output present for rmcp call")?; + + // Expect plain text with byte-based truncation marker. + assert!( + serde_json::from_str::(&output).is_err(), + "expected truncated MCP output to be plain text" + ); + assert!( + output.starts_with("Total output lines: 1\n\n{"), + "expected total line header and JSON head, got: {output}" + ); + let byte_marker = Regex::new(r"\[\.\.\. output truncated to fit 10240 bytes \.\.\.\]") + .expect("compile regex"); + assert!( + byte_marker.is_match(&output), + "expected byte truncation marker, got: {output}" + ); + + Ok(()) +} diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 10a4376161..559fd98002 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -249,7 +249,11 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> any std::process::exit(1); } - let auth_manager = AuthManager::shared(config.codex_home.clone(), true); + let auth_manager = AuthManager::shared( + config.codex_home.clone(), + true, + config.cli_auth_credentials_store_mode, + ); let conversation_manager = ConversationManager::new(auth_manager.clone(), SessionSource::Exec); // Handle resume subcommand by resolving a rollout path and using explicit resume API. diff --git a/codex-rs/exec/tests/suite/resume.rs b/codex-rs/exec/tests/suite/resume.rs index 320c5766da..24b8cb0bce 100644 --- a/codex-rs/exec/tests/suite/resume.rs +++ b/codex-rs/exec/tests/suite/resume.rs @@ -234,10 +234,17 @@ fn exec_resume_preserves_cli_configuration_overrides() -> anyhow::Result<()> { stderr.contains("model: gpt-5-high"), "stderr missing model override: {stderr}" ); - assert!( - stderr.contains("sandbox: workspace-write"), - "stderr missing sandbox override: {stderr}" - ); + if cfg!(target_os = "windows") { + assert!( + stderr.contains("sandbox: read-only"), + "stderr missing downgraded sandbox note: {stderr}" + ); + } else { + assert!( + stderr.contains("sandbox: workspace-write"), + "stderr missing sandbox override: {stderr}" + ); + } let resumed_path = find_session_file_containing_marker(&sessions_dir, &marker2) .expect("no resumed session file containing marker2"); diff --git a/codex-rs/keyring-store/Cargo.toml b/codex-rs/keyring-store/Cargo.toml index 94d3d54493..f662e5d4ff 100644 --- a/codex-rs/keyring-store/Cargo.toml +++ b/codex-rs/keyring-store/Cargo.toml @@ -7,5 +7,10 @@ version = { workspace = true } workspace = true [dependencies] -keyring = { workspace = true } +keyring = { workspace = true, features = [ + "apple-native", + "crypto-rust", + "linux-native-async-persistent", + "windows-native", +] } tracing = { workspace = true } diff --git a/codex-rs/login/src/device_code_auth.rs b/codex-rs/login/src/device_code_auth.rs index f1999a8303..acaf30ba0c 100644 --- a/codex-rs/login/src/device_code_auth.rs +++ b/codex-rs/login/src/device_code_auth.rs @@ -199,6 +199,7 @@ pub async fn run_device_code_login(opts: ServerOptions) -> std::io::Result<()> { tokens.id_token, tokens.access_token, tokens.refresh_token, + opts.cli_auth_credentials_store_mode, ) .await } diff --git a/codex-rs/login/src/server.rs b/codex-rs/login/src/server.rs index 36186ec017..999c19072e 100644 --- a/codex-rs/login/src/server.rs +++ b/codex-rs/login/src/server.rs @@ -14,6 +14,7 @@ use crate::pkce::PkceCodes; use crate::pkce::generate_pkce; use base64::Engine; use chrono::Utc; +use codex_core::auth::AuthCredentialsStoreMode; use codex_core::auth::AuthDotJson; use codex_core::auth::save_auth; use codex_core::default_client::originator; @@ -39,6 +40,7 @@ pub struct ServerOptions { pub open_browser: bool, pub force_state: Option, pub forced_chatgpt_workspace_id: Option, + pub cli_auth_credentials_store_mode: AuthCredentialsStoreMode, } impl ServerOptions { @@ -46,6 +48,7 @@ impl ServerOptions { codex_home: PathBuf, client_id: String, forced_chatgpt_workspace_id: Option, + cli_auth_credentials_store_mode: AuthCredentialsStoreMode, ) -> Self { Self { codex_home, @@ -55,6 +58,7 @@ impl ServerOptions { open_browser: true, force_state: None, forced_chatgpt_workspace_id, + cli_auth_credentials_store_mode, } } } @@ -270,6 +274,7 @@ async fn process_request( tokens.id_token.clone(), tokens.access_token.clone(), tokens.refresh_token.clone(), + opts.cli_auth_credentials_store_mode, ) .await { @@ -536,6 +541,7 @@ pub(crate) async fn persist_tokens_async( id_token: String, access_token: String, refresh_token: String, + auth_credentials_store_mode: AuthCredentialsStoreMode, ) -> io::Result<()> { // Reuse existing synchronous logic but run it off the async runtime. let codex_home = codex_home.to_path_buf(); @@ -557,7 +563,7 @@ pub(crate) async fn persist_tokens_async( tokens: Some(tokens), last_refresh: Some(Utc::now()), }; - save_auth(&codex_home, &auth) + save_auth(&codex_home, &auth, auth_credentials_store_mode) }) .await .map_err(|e| io::Error::other(format!("persist task failed: {e}")))? diff --git a/codex-rs/login/tests/suite/device_code_login.rs b/codex-rs/login/tests/suite/device_code_login.rs index 195d72573d..266930e419 100644 --- a/codex-rs/login/tests/suite/device_code_login.rs +++ b/codex-rs/login/tests/suite/device_code_login.rs @@ -3,6 +3,7 @@ use anyhow::Context; use base64::Engine; use base64::engine::general_purpose::URL_SAFE_NO_PAD; +use codex_core::auth::AuthCredentialsStoreMode; use codex_core::auth::load_auth_dot_json; use codex_login::ServerOptions; use codex_login::run_device_code_login; @@ -96,11 +97,16 @@ async fn mock_oauth_token_single(server: &MockServer, jwt: String) { .await; } -fn server_opts(codex_home: &tempfile::TempDir, issuer: String) -> ServerOptions { +fn server_opts( + codex_home: &tempfile::TempDir, + issuer: String, + cli_auth_credentials_store_mode: AuthCredentialsStoreMode, +) -> ServerOptions { let mut opts = ServerOptions::new( codex_home.path().to_path_buf(), "client-id".to_string(), None, + cli_auth_credentials_store_mode, ); opts.issuer = issuer; opts.open_browser = false; @@ -127,13 +133,13 @@ async fn device_code_login_integration_succeeds() -> anyhow::Result<()> { mock_oauth_token_single(&mock_server, jwt.clone()).await; let issuer = mock_server.uri(); - let opts = server_opts(&codex_home, issuer); + let opts = server_opts(&codex_home, issuer, AuthCredentialsStoreMode::File); run_device_code_login(opts) .await .expect("device code login integration should succeed"); - let auth = load_auth_dot_json(codex_home.path()) + let auth = load_auth_dot_json(codex_home.path(), AuthCredentialsStoreMode::File) .context("auth.json should load after login succeeds")? .context("auth.json written")?; // assert_eq!(auth.openai_api_key.as_deref(), Some("api-key-321")); @@ -166,7 +172,7 @@ async fn device_code_login_rejects_workspace_mismatch() -> anyhow::Result<()> { mock_oauth_token_single(&mock_server, jwt).await; let issuer = mock_server.uri(); - let mut opts = server_opts(&codex_home, issuer); + let mut opts = server_opts(&codex_home, issuer, AuthCredentialsStoreMode::File); opts.forced_chatgpt_workspace_id = Some("org-required".to_string()); let err = run_device_code_login(opts) @@ -174,8 +180,8 @@ async fn device_code_login_rejects_workspace_mismatch() -> anyhow::Result<()> { .expect_err("device code login should fail when workspace mismatches"); assert_eq!(err.kind(), std::io::ErrorKind::PermissionDenied); - let auth = - load_auth_dot_json(codex_home.path()).context("auth.json should load after login fails")?; + let auth = load_auth_dot_json(codex_home.path(), AuthCredentialsStoreMode::File) + .context("auth.json should load after login fails")?; assert!( auth.is_none(), "auth.json should not be created when workspace validation fails" @@ -194,7 +200,7 @@ async fn device_code_login_integration_handles_usercode_http_failure() -> anyhow let issuer = mock_server.uri(); - let opts = server_opts(&codex_home, issuer); + let opts = server_opts(&codex_home, issuer, AuthCredentialsStoreMode::File); let err = run_device_code_login(opts) .await @@ -205,8 +211,8 @@ async fn device_code_login_integration_handles_usercode_http_failure() -> anyhow "unexpected error: {err:?}" ); - let auth = - load_auth_dot_json(codex_home.path()).context("auth.json should load after login fails")?; + let auth = load_auth_dot_json(codex_home.path(), AuthCredentialsStoreMode::File) + .context("auth.json should load after login fails")?; assert!( auth.is_none(), "auth.json should not be created when login fails" @@ -237,6 +243,7 @@ async fn device_code_login_integration_persists_without_api_key_on_exchange_fail codex_home.path().to_path_buf(), "client-id".to_string(), None, + AuthCredentialsStoreMode::File, ); opts.issuer = issuer; opts.open_browser = false; @@ -245,7 +252,7 @@ async fn device_code_login_integration_persists_without_api_key_on_exchange_fail .await .expect("device login should succeed without API key exchange"); - let auth = load_auth_dot_json(codex_home.path()) + let auth = load_auth_dot_json(codex_home.path(), AuthCredentialsStoreMode::File) .context("auth.json should load after login succeeds")? .context("auth.json written")?; assert!(auth.openai_api_key.is_none()); @@ -286,6 +293,7 @@ async fn device_code_login_integration_handles_error_payload() -> anyhow::Result codex_home.path().to_path_buf(), "client-id".to_string(), None, + AuthCredentialsStoreMode::File, ); opts.issuer = issuer; opts.open_browser = false; @@ -300,8 +308,8 @@ async fn device_code_login_integration_handles_error_payload() -> anyhow::Result "Expected an authorization_declined / 400 / 404 error, got {err:?}" ); - let auth = - load_auth_dot_json(codex_home.path()).context("auth.json should load after login fails")?; + let auth = load_auth_dot_json(codex_home.path(), AuthCredentialsStoreMode::File) + .context("auth.json should load after login fails")?; assert!( auth.is_none(), "auth.json should not be created when device auth fails" diff --git a/codex-rs/login/tests/suite/login_server_e2e.rs b/codex-rs/login/tests/suite/login_server_e2e.rs index 3a5f869b5a..73cb8bd420 100644 --- a/codex-rs/login/tests/suite/login_server_e2e.rs +++ b/codex-rs/login/tests/suite/login_server_e2e.rs @@ -7,6 +7,7 @@ use std::time::Duration; use anyhow::Result; use base64::Engine; +use codex_core::auth::AuthCredentialsStoreMode; use codex_login::ServerOptions; use codex_login::run_login_server; use core_test_support::skip_if_no_network; @@ -110,6 +111,7 @@ async fn end_to_end_login_flow_persists_auth_json() -> Result<()> { let opts = ServerOptions { codex_home: server_home, + cli_auth_credentials_store_mode: AuthCredentialsStoreMode::File, client_id: codex_login::CLIENT_ID.to_string(), issuer, port: 0, @@ -170,6 +172,7 @@ async fn creates_missing_codex_home_dir() -> Result<()> { let server_home = codex_home.clone(); let opts = ServerOptions { codex_home: server_home, + cli_auth_credentials_store_mode: AuthCredentialsStoreMode::File, client_id: codex_login::CLIENT_ID.to_string(), issuer, port: 0, @@ -208,6 +211,7 @@ async fn forced_chatgpt_workspace_id_mismatch_blocks_login() -> Result<()> { let opts = ServerOptions { codex_home: codex_home.clone(), + cli_auth_credentials_store_mode: AuthCredentialsStoreMode::File, client_id: codex_login::CLIENT_ID.to_string(), issuer, port: 0, @@ -263,6 +267,7 @@ async fn cancels_previous_login_server_when_port_is_in_use() -> Result<()> { let first_opts = ServerOptions { codex_home: first_codex_home, + cli_auth_credentials_store_mode: AuthCredentialsStoreMode::File, client_id: codex_login::CLIENT_ID.to_string(), issuer: issuer.clone(), port: 0, @@ -282,6 +287,7 @@ async fn cancels_previous_login_server_when_port_is_in_use() -> Result<()> { let second_opts = ServerOptions { codex_home: second_codex_home, + cli_auth_credentials_store_mode: AuthCredentialsStoreMode::File, client_id: codex_login::CLIENT_ID.to_string(), issuer, port: login_port, diff --git a/codex-rs/mcp-server/src/message_processor.rs b/codex-rs/mcp-server/src/message_processor.rs index b21d5a6503..81eb80764b 100644 --- a/codex-rs/mcp-server/src/message_processor.rs +++ b/codex-rs/mcp-server/src/message_processor.rs @@ -53,7 +53,11 @@ impl MessageProcessor { config: Arc, ) -> Self { let outgoing = Arc::new(outgoing); - let auth_manager = AuthManager::shared(config.codex_home.clone(), false); + let auth_manager = AuthManager::shared( + config.codex_home.clone(), + false, + config.cli_auth_credentials_store_mode, + ); let conversation_manager = Arc::new(ConversationManager::new(auth_manager, SessionSource::Mcp)); Self { diff --git a/codex-rs/protocol/src/account.rs b/codex-rs/protocol/src/account.rs index 1d63910c64..fb707c3a73 100644 --- a/codex-rs/protocol/src/account.rs +++ b/codex-rs/protocol/src/account.rs @@ -18,18 +18,3 @@ pub enum PlanType { #[serde(other)] Unknown, } - -#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema, TS)] -#[serde(tag = "type")] -#[ts(tag = "type")] -pub enum Account { - ApiKey { - api_key: String, - }, - #[serde(rename = "chatgpt")] - #[ts(rename = "chatgpt")] - ChatGpt { - email: Option, - plan_type: PlanType, - }, -} diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 4430a0998a..614a5ff2b0 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -3,6 +3,7 @@ use std::collections::HashMap; use base64::Engine; use codex_utils_image::load_and_resize_to_fit; use mcp_types::CallToolResult; +use mcp_types::ContentBlock; use serde::Deserialize; use serde::Deserializer; use serde::Serialize; @@ -83,9 +84,8 @@ pub enum ResponseItem { // NOTE: The input schema for `function_call_output` objects that clients send to the // OpenAI /v1/responses endpoint is NOT the same shape as the objects the server returns on the // SSE stream. When *sending* we must wrap the string output inside an object that includes a - // required `success` boolean. The upstream TypeScript CLI does this implicitly. To ensure we - // serialize exactly the expected shape we introduce a dedicated payload struct and flatten it - // here. + // required `success` boolean. To ensure we serialize exactly the expected shape we introduce + // a dedicated payload struct and flatten it here. FunctionCallOutput { call_id: String, output: FunctionCallOutputPayload, @@ -160,19 +160,17 @@ impl From for ResponseItem { ResponseInputItem::FunctionCallOutput { call_id, output } => { Self::FunctionCallOutput { call_id, output } } - ResponseInputItem::McpToolCallOutput { call_id, result } => Self::FunctionCallOutput { - call_id, - output: FunctionCallOutputPayload { - success: Some(result.is_ok()), - content: result.map_or_else( - |tool_call_err| format!("err: {tool_call_err:?}"), - |result| { - serde_json::to_string(&result) - .unwrap_or_else(|e| format!("JSON serialization error: {e}")) - }, - ), - }, - }, + ResponseInputItem::McpToolCallOutput { call_id, result } => { + let output = match result { + Ok(result) => FunctionCallOutputPayload::from(&result), + Err(tool_call_err) => FunctionCallOutputPayload { + content: format!("err: {tool_call_err:?}"), + success: Some(false), + ..Default::default() + }, + }; + Self::FunctionCallOutput { call_id, output } + } ResponseInputItem::CustomToolCallOutput { call_id, output } => { Self::CustomToolCallOutput { call_id, output } } @@ -290,31 +288,53 @@ pub struct ShellToolCallParams { pub justification: Option, } -#[derive(Debug, Clone, PartialEq, JsonSchema, TS)] +/// Responses API compatible content items that can be returned by a tool call. +/// This is a subset of ContentItem with the types we support as function call outputs. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, JsonSchema, TS)] +#[serde(tag = "type", rename_all = "snake_case")] +pub enum FunctionCallOutputContentItem { + // Do not rename, these are serialized and used directly in the responses API. + InputText { text: String }, + // Do not rename, these are serialized and used directly in the responses API. + InputImage { image_url: String }, +} + +/// The payload we send back to OpenAI when reporting a tool call result. +/// +/// `content` preserves the historical plain-string payload so downstream +/// integrations (tests, logging, etc.) can keep treating tool output as +/// `String`. When an MCP server returns richer data we additionally populate +/// `content_items` with the structured form that the Responses/Chat +/// Completions APIs understand. +#[derive(Debug, Default, Clone, PartialEq, JsonSchema, TS)] pub struct FunctionCallOutputPayload { pub content: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub content_items: Option>, // TODO(jif) drop this. pub success: Option, } +#[derive(Deserialize)] +#[serde(untagged)] +enum FunctionCallOutputPayloadSerde { + Text(String), + Items(Vec), +} + // The Responses API expects two *different* shapes depending on success vs failure: // • success → output is a plain string (no nested object) // • failure → output is an object { content, success:false } -// The upstream TypeScript CLI implements this by special‑casing the serialize path. -// We replicate that behavior with a manual Serialize impl. - impl Serialize for FunctionCallOutputPayload { fn serialize(&self, serializer: S) -> Result where S: Serializer, { - // The upstream TypeScript CLI always serializes `output` as a *plain string* regardless - // of whether the function call succeeded or failed. The boolean is purely informational - // for local bookkeeping and is NOT sent to the OpenAI endpoint. Sending the nested object - // form `{ content, success:false }` triggers the 400 we are still seeing. Mirror the JS CLI - // exactly: always emit a bare string. - - serializer.serialize_str(&self.content) + if let Some(items) = &self.content_items { + items.serialize(serializer) + } else { + serializer.serialize_str(&self.content) + } } } @@ -323,14 +343,106 @@ impl<'de> Deserialize<'de> for FunctionCallOutputPayload { where D: Deserializer<'de>, { - let s = String::deserialize(deserializer)?; - Ok(FunctionCallOutputPayload { - content: s, - success: None, - }) + match FunctionCallOutputPayloadSerde::deserialize(deserializer)? { + FunctionCallOutputPayloadSerde::Text(content) => Ok(FunctionCallOutputPayload { + content, + ..Default::default() + }), + FunctionCallOutputPayloadSerde::Items(items) => { + let content = serde_json::to_string(&items).map_err(serde::de::Error::custom)?; + Ok(FunctionCallOutputPayload { + content, + content_items: Some(items), + success: None, + }) + } + } } } +impl From<&CallToolResult> for FunctionCallOutputPayload { + fn from(call_tool_result: &CallToolResult) -> Self { + let CallToolResult { + content, + structured_content, + is_error, + } = call_tool_result; + + let is_success = is_error != &Some(true); + + if let Some(structured_content) = structured_content + && !structured_content.is_null() + { + match serde_json::to_string(structured_content) { + Ok(serialized_structured_content) => { + return FunctionCallOutputPayload { + content: serialized_structured_content, + success: Some(is_success), + ..Default::default() + }; + } + Err(err) => { + return FunctionCallOutputPayload { + content: err.to_string(), + success: Some(false), + ..Default::default() + }; + } + } + } + + let serialized_content = match serde_json::to_string(content) { + Ok(serialized_content) => serialized_content, + Err(err) => { + return FunctionCallOutputPayload { + content: err.to_string(), + success: Some(false), + ..Default::default() + }; + } + }; + + let content_items = convert_content_blocks_to_items(content); + + FunctionCallOutputPayload { + content: serialized_content, + content_items, + success: Some(is_success), + } + } +} + +fn convert_content_blocks_to_items( + blocks: &[ContentBlock], +) -> Option> { + let mut saw_image = false; + let mut items = Vec::with_capacity(blocks.len()); + + for block in blocks { + match block { + ContentBlock::TextContent(text) => { + items.push(FunctionCallOutputContentItem::InputText { + text: text.text.clone(), + }); + } + ContentBlock::ImageContent(image) => { + saw_image = true; + // Just in case the content doesn't include a data URL, add it. + let image_url = if image.data.starts_with("data:") { + image.data.clone() + } else { + format!("data:{};base64,{}", image.mime_type, image.data) + }; + items.push(FunctionCallOutputContentItem::InputImage { image_url }); + } + // TODO: render audio, resource, and embedded resource content to the model. + _ => return None, + } + } + + if saw_image { Some(items) } else { None } +} + // Implement Display so callers can treat the payload like a plain string when logging or doing // trivial substring checks in tests (existing tests call `.contains()` on the output). Display // returns the raw `content` field. @@ -354,6 +466,8 @@ impl std::ops::Deref for FunctionCallOutputPayload { mod tests { use super::*; use anyhow::Result; + use mcp_types::ImageContent; + use mcp_types::TextContent; use tempfile::tempdir; #[test] @@ -362,7 +476,7 @@ mod tests { call_id: "call1".into(), output: FunctionCallOutputPayload { content: "ok".into(), - success: None, + ..Default::default() }, }; @@ -381,6 +495,7 @@ mod tests { output: FunctionCallOutputPayload { content: "bad".into(), success: Some(false), + ..Default::default() }, }; @@ -391,6 +506,81 @@ mod tests { Ok(()) } + #[test] + fn serializes_image_outputs_as_array() -> Result<()> { + let call_tool_result = CallToolResult { + content: vec![ + ContentBlock::TextContent(TextContent { + annotations: None, + text: "caption".into(), + r#type: "text".into(), + }), + ContentBlock::ImageContent(ImageContent { + annotations: None, + data: "BASE64".into(), + mime_type: "image/png".into(), + r#type: "image".into(), + }), + ], + is_error: None, + structured_content: None, + }; + + let payload = FunctionCallOutputPayload::from(&call_tool_result); + assert_eq!(payload.success, Some(true)); + let items = payload.content_items.clone().expect("content items"); + assert_eq!( + items, + vec![ + FunctionCallOutputContentItem::InputText { + text: "caption".into(), + }, + FunctionCallOutputContentItem::InputImage { + image_url: "data:image/png;base64,BASE64".into(), + }, + ] + ); + + let item = ResponseInputItem::FunctionCallOutput { + call_id: "call1".into(), + output: payload, + }; + + let json = serde_json::to_string(&item)?; + let v: serde_json::Value = serde_json::from_str(&json)?; + + let output = v.get("output").expect("output field"); + assert!(output.is_array(), "expected array output"); + + Ok(()) + } + + #[test] + fn deserializes_array_payload_into_items() -> Result<()> { + let json = r#"[ + {"type": "input_text", "text": "note"}, + {"type": "input_image", "image_url": "data:image/png;base64,XYZ"} + ]"#; + + let payload: FunctionCallOutputPayload = serde_json::from_str(json)?; + + assert_eq!(payload.success, None); + let expected_items = vec![ + FunctionCallOutputContentItem::InputText { + text: "note".into(), + }, + FunctionCallOutputContentItem::InputImage { + image_url: "data:image/png;base64,XYZ".into(), + }, + ]; + assert_eq!(payload.content_items, Some(expected_items.clone())); + + let expected_content = serde_json::to_string(&expected_items)?; + assert_eq!(payload.content, expected_content); + + Ok(()) + } + #[test] fn deserialize_shell_tool_call_params() -> Result<()> { let json = r#"{ diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index bb14f3797b..a7f5241bb3 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -814,7 +814,7 @@ pub struct AgentReasoningDeltaEvent { pub delta: String, } -#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] +#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS, PartialEq)] pub struct McpInvocation { /// Name of the MCP server as defined in the config. pub server: String, @@ -824,14 +824,14 @@ pub struct McpInvocation { pub arguments: Option, } -#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] +#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS, PartialEq)] pub struct McpToolCallBeginEvent { /// Identifier so this can be paired with the McpToolCallEnd event. pub call_id: String, pub invocation: McpInvocation, } -#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] +#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS, PartialEq)] pub struct McpToolCallEndEvent { /// Identifier for the corresponding McpToolCallBegin that finished. pub call_id: String, diff --git a/codex-rs/rmcp-client/src/bin/test_stdio_server.rs b/codex-rs/rmcp-client/src/bin/test_stdio_server.rs index 44ae50f02f..aafba59324 100644 --- a/codex-rs/rmcp-client/src/bin/test_stdio_server.rs +++ b/codex-rs/rmcp-client/src/bin/test_stdio_server.rs @@ -40,7 +40,7 @@ pub fn stdio() -> (tokio::io::Stdin, tokio::io::Stdout) { } impl TestToolServer { fn new() -> Self { - let tools = vec![Self::echo_tool()]; + let tools = vec![Self::echo_tool(), Self::image_tool()]; let resources = vec![Self::memo_resource()]; let resource_templates = vec![Self::memo_template()]; Self { @@ -70,6 +70,22 @@ impl TestToolServer { ) } + fn image_tool() -> Tool { + #[expect(clippy::expect_used)] + let schema: JsonObject = serde_json::from_value(serde_json::json!({ + "type": "object", + "properties": {}, + "additionalProperties": false + })) + .expect("image tool schema should deserialize"); + + Tool::new( + Cow::Borrowed("image"), + Cow::Borrowed("Return a single image content block."), + Arc::new(schema), + ) + } + fn memo_resource() -> Resource { let raw = RawResource { uri: MEMO_URI.to_string(), @@ -214,6 +230,35 @@ impl ServerHandler for TestToolServer { meta: None, }) } + "image" => { + // Read a data URL (e.g. data:image/png;base64,AAA...) from env and convert to + // an MCP image content block. Tests set MCP_TEST_IMAGE_DATA_URL. + let data_url = std::env::var("MCP_TEST_IMAGE_DATA_URL").map_err(|_| { + McpError::invalid_params( + "missing MCP_TEST_IMAGE_DATA_URL env var for image tool", + None, + ) + })?; + + fn parse_data_url(url: &str) -> Option<(String, String)> { + let rest = url.strip_prefix("data:")?; + let (mime_and_opts, data) = rest.split_once(',')?; + let (mime, _opts) = + mime_and_opts.split_once(';').unwrap_or((mime_and_opts, "")); + Some((mime.to_string(), data.to_string())) + } + + let (mime_type, data_b64) = parse_data_url(&data_url).ok_or_else(|| { + McpError::invalid_params( + format!("invalid data URL for image tool: {data_url}"), + None, + ) + })?; + + Ok(CallToolResult::success(vec![rmcp::model::Content::image( + data_b64, mime_type, + )])) + } other => Err(McpError::invalid_params( format!("unknown tool: {other}"), None, diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index c8eab72969..8fc18a1d9b 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -369,6 +369,9 @@ impl App { AppEvent::OpenFeedbackConsent { category } => { self.chat_widget.open_feedback_consent(category); } + AppEvent::ShowWindowsAutoModeInstructions => { + self.chat_widget.open_windows_auto_mode_instructions(); + } AppEvent::PersistModelSelection { model, effort } => { let profile = self.active_profile.as_deref(); match persist_model_selection(&self.config.codex_home, profile, &model, effort) diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 8b14b0be6f..514e234743 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -72,6 +72,9 @@ pub(crate) enum AppEvent { preset: ApprovalPreset, }, + /// Show Windows Subsystem for Linux setup instructions for auto mode. + ShowWindowsAutoModeInstructions, + /// Update the current approval policy in the running app and widget. UpdateAskForApprovalPolicy(AskForApproval), diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 7b4d2f7ec4..8691b5b127 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -573,6 +573,16 @@ impl ChatComposer { #[inline] fn handle_non_ascii_char(&mut self, input: KeyEvent) -> (InputResult, bool) { + if let KeyEvent { + code: KeyCode::Char(ch), + .. + } = input + { + let now = Instant::now(); + if self.paste_burst.try_append_char_if_active(ch, now) { + return (InputResult::None, true); + } + } if let Some(pasted) = self.paste_burst.flush_before_modified_input() { self.handle_paste(pasted); } diff --git a/codex-rs/tui/src/bottom_pane/paste_burst.rs b/codex-rs/tui/src/bottom_pane/paste_burst.rs index 33a02bdefa..49377cb21c 100644 --- a/codex-rs/tui/src/bottom_pane/paste_burst.rs +++ b/codex-rs/tui/src/bottom_pane/paste_burst.rs @@ -163,6 +163,18 @@ impl PasteBurst { self.burst_window_until = Some(now + PASTE_ENTER_SUPPRESS_WINDOW); } + /// Try to append a char into the burst buffer only if a burst is already active. + /// + /// Returns true when the char was captured into the existing burst, false otherwise. + pub fn try_append_char_if_active(&mut self, ch: char, now: Instant) -> bool { + if self.active || !self.buffer.is_empty() { + self.append_char_to_buffer(ch, now); + true + } else { + false + } + } + /// Decide whether to begin buffering by retroactively capturing recent /// chars from the slice before the cursor. /// diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 13411df404..e1255c84dd 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -88,6 +88,8 @@ use crate::history_cell::AgentMessageCell; use crate::history_cell::HistoryCell; use crate::history_cell::McpToolCallCell; use crate::markdown::append_markdown; +#[cfg(target_os = "windows")] +use crate::onboarding::WSL_INSTRUCTIONS; use crate::render::renderable::ColumnRenderable; use crate::render::renderable::Renderable; use crate::slash_command::SlashCommand; @@ -1217,7 +1219,10 @@ impl ChatWidget { self.app_event_tx.send(AppEvent::ExitRequest); } SlashCommand::Logout => { - if let Err(e) = codex_core::auth::logout(&self.config.codex_home) { + if let Err(e) = codex_core::auth::logout( + &self.config.codex_home, + self.config.cli_auth_credentials_store_mode, + ) { tracing::error!("failed to logout: {e}"); } self.app_event_tx.send(AppEvent::ExitRequest); @@ -1787,11 +1792,38 @@ impl ChatWidget { let current_sandbox = self.config.sandbox_policy.clone(); let mut items: Vec = Vec::new(); let presets: Vec = builtin_approval_presets(); + #[cfg(target_os = "windows")] + let header_renderable: Box = if self + .config + .forced_auto_mode_downgraded_on_windows + { + use ratatui_macros::line; + + let mut header = ColumnRenderable::new(); + header.push(line![ + "Codex forced your settings back to Read Only on this Windows machine.".bold() + ]); + header.push(line![ + "To re-enable Auto mode, run Codex inside Windows Subsystem for Linux (WSL) or enable Full Access manually.".dim() + ]); + Box::new(header) + } else { + Box::new(()) + }; + #[cfg(not(target_os = "windows"))] + let header_renderable: Box = Box::new(()); for preset in presets.into_iter() { let is_current = current_approval == preset.approval && current_sandbox == preset.sandbox; let name = preset.label.to_string(); - let description = Some(preset.description.to_string()); + let description_text = preset.description; + let description = if cfg!(target_os = "windows") && preset.id == "auto" { + Some(format!( + "{description_text}\nRequires Windows Subsystem for Linux (WSL). Show installation instructions..." + )) + } else { + Some(description_text.to_string()) + }; let requires_confirmation = preset.id == "full-access" && !self .config @@ -1805,6 +1837,10 @@ impl ChatWidget { preset: preset_clone.clone(), }); })] + } else if cfg!(target_os = "windows") && preset.id == "auto" { + vec![Box::new(|tx| { + tx.send(AppEvent::ShowWindowsAutoModeInstructions); + })] } else { Self::approval_preset_actions(preset.approval, preset.sandbox.clone()) }; @@ -1822,6 +1858,7 @@ impl ChatWidget { title: Some("Select Approval Mode".to_string()), footer_hint: Some(standard_popup_hint_line()), items, + header: header_renderable, ..Default::default() }); } @@ -1909,6 +1946,43 @@ impl ChatWidget { }); } + #[cfg(target_os = "windows")] + pub(crate) fn open_windows_auto_mode_instructions(&mut self) { + use ratatui_macros::line; + + let mut header = ColumnRenderable::new(); + header.push(line![ + "Auto mode requires Windows Subsystem for Linux (WSL2).".bold() + ]); + header.push(line!["Run Codex inside WSL to enable sandboxed commands."]); + header.push(line![""]); + header.push(Paragraph::new(WSL_INSTRUCTIONS).wrap(Wrap { trim: false })); + + let items = vec![SelectionItem { + name: "Back".to_string(), + description: Some( + "Return to the approval mode list. Auto mode stays disabled outside WSL." + .to_string(), + ), + actions: vec![Box::new(|tx| { + tx.send(AppEvent::OpenApprovalsPopup); + })], + dismiss_on_select: true, + ..Default::default() + }]; + + self.bottom_pane.show_selection_view(SelectionViewParams { + title: None, + footer_hint: Some(standard_popup_hint_line()), + items, + header: Box::new(header), + ..Default::default() + }); + } + + #[cfg(not(target_os = "windows"))] + pub(crate) fn open_windows_auto_mode_instructions(&mut self) {} + /// Set the approval policy in the widget's config copy. pub(crate) fn set_approval_policy(&mut self, policy: AskForApproval) { self.config.approval_policy = policy; diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approvals_selection_popup.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approvals_selection_popup.snap index 1d612966de..190594b1b3 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approvals_selection_popup.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approvals_selection_popup.snap @@ -6,12 +6,12 @@ expression: popup › 1. Read Only (current) Codex can read files and answer questions. Codex requires approval to make edits, run commands, or - access network + access network. 2. Auto Codex can read files, make edits, and run commands in the workspace. Codex requires approval to work - outside the workspace or access network + outside the workspace or access network. 3. Full Access Codex can read files, make edits, and run commands with network access, without approval. Exercise - caution + caution. Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approvals_selection_popup@windows.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approvals_selection_popup@windows.snap new file mode 100644 index 0000000000..7d16ad57b8 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approvals_selection_popup@windows.snap @@ -0,0 +1,19 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: popup +--- + Select Approval Mode + +› 1. Read Only (current) Codex can read files and answer questions. Codex + requires approval to make edits, run commands, or + access network. + 2. Auto Codex can read files, make edits, and run commands + in the workspace. Codex requires approval to work + outside the workspace or access network. + Requires Windows Subsystem for Linux (WSL). Show + installation instructions... + 3. Full Access Codex can read files, make edits, and run commands + with network access, without approval. Exercise + caution. + + Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index a9c1d58c67..526643627b 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -1276,9 +1276,36 @@ fn approvals_selection_popup_snapshot() { chat.open_approvals_popup(); let popup = render_bottom_popup(&chat, 80); + #[cfg(target_os = "windows")] + insta::with_settings!({ snapshot_suffix => "windows" }, { + assert_snapshot!("approvals_selection_popup", popup); + }); + #[cfg(not(target_os = "windows"))] assert_snapshot!("approvals_selection_popup", popup); } +#[test] +fn approvals_popup_includes_wsl_note_for_auto_mode() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(); + + if cfg!(target_os = "windows") { + chat.config.forced_auto_mode_downgraded_on_windows = true; + } + chat.open_approvals_popup(); + + let popup = render_bottom_popup(&chat, 80); + assert_eq!( + popup.contains("Requires Windows Subsystem for Linux (WSL)"), + cfg!(target_os = "windows"), + "expected auto preset description to mention WSL requirement only on Windows, popup: {popup}" + ); + assert_eq!( + popup.contains("Codex forced your settings back to Read Only on this Windows machine."), + cfg!(target_os = "windows") && chat.config.forced_auto_mode_downgraded_on_windows, + "expected downgrade notice only when auto mode is forced off on Windows, popup: {popup}" + ); +} + #[test] fn full_access_confirmation_popup_snapshot() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(); @@ -1293,6 +1320,20 @@ fn full_access_confirmation_popup_snapshot() { assert_snapshot!("full_access_confirmation_popup", popup); } +#[cfg(target_os = "windows")] +#[test] +fn windows_auto_mode_instructions_popup_lists_install_steps() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(); + + chat.open_windows_auto_mode_instructions(); + + let popup = render_bottom_popup(&chat, 120); + assert!( + popup.contains("wsl --install"), + "expected WSL instructions popup to include install command, popup: {popup}" + ); +} + #[test] fn model_reasoning_selection_popup_snapshot() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(); diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index e630e00175..406a892561 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -465,7 +465,7 @@ struct CompletedMcpToolCallWithImageOutput { } impl HistoryCell for CompletedMcpToolCallWithImageOutput { fn display_lines(&self, _width: u16) -> Vec> { - vec!["tool result (image output omitted)".into()] + vec!["tool result (image output)".into()] } } diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index ea19d51ad6..028bf68e87 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -315,7 +315,11 @@ async fn run_ratatui_app( // Initialize high-fidelity session event logging if enabled. session_log::maybe_init(&initial_config); - let auth_manager = AuthManager::shared(initial_config.codex_home.clone(), false); + let auth_manager = AuthManager::shared( + initial_config.codex_home.clone(), + false, + initial_config.cli_auth_credentials_store_mode, + ); let login_status = get_login_status(&initial_config); let should_show_trust_screen = should_show_trust_screen(&initial_config); let should_show_windows_wsl_screen = @@ -476,7 +480,7 @@ fn get_login_status(config: &Config) -> LoginStatus { // Reading the OpenAI API key is an async operation because it may need // to refresh the token. Block on it. let codex_home = config.codex_home.clone(); - match CodexAuth::from_auth_storage(&codex_home) { + match CodexAuth::from_auth_storage(&codex_home, config.cli_auth_credentials_store_mode) { Ok(Some(auth)) => LoginStatus::AuthMode(auth.mode), Ok(None) => LoginStatus::NotAuthenticated, Err(err) => { @@ -507,14 +511,16 @@ async fn load_config_or_exit( /// or if the current cwd project is already trusted. If not, we need to /// show the trust screen. fn should_show_trust_screen(config: &Config) -> bool { - if config.did_user_set_custom_approval_policy_or_sandbox_mode { - // if the user has overridden either approval policy or sandbox mode, - // skip the trust flow - false - } else { - // otherwise, skip iff the active project is trusted - !config.active_project.is_trusted() + if cfg!(target_os = "windows") { + // Native Windows cannot enforce sandboxed write access without WSL; skip the trust prompt entirely. + return false; } + if config.did_user_set_custom_approval_policy_or_sandbox_mode { + // Respect explicit approval/sandbox overrides made by the user. + return false; + } + // otherwise, skip iff the active project is trusted + !config.active_project.is_trusted() } fn should_show_onboarding( @@ -543,3 +549,38 @@ fn should_show_login_screen(login_status: LoginStatus, config: &Config) -> bool login_status == LoginStatus::NotAuthenticated } + +#[cfg(test)] +mod tests { + use super::*; + use codex_core::config::ConfigOverrides; + use codex_core::config::ConfigToml; + use codex_core::config::ProjectConfig; + use tempfile::TempDir; + + #[test] + fn windows_skips_trust_prompt() -> std::io::Result<()> { + let temp_dir = TempDir::new()?; + let mut config = Config::load_from_base_config_with_overrides( + ConfigToml::default(), + ConfigOverrides::default(), + temp_dir.path().to_path_buf(), + )?; + config.did_user_set_custom_approval_policy_or_sandbox_mode = false; + config.active_project = ProjectConfig { trust_level: None }; + + let should_show = should_show_trust_screen(&config); + if cfg!(target_os = "windows") { + assert!( + !should_show, + "Windows trust prompt should always be skipped on native Windows" + ); + } else { + assert!( + should_show, + "Non-Windows should still show trust prompt when project is untrusted" + ); + } + Ok(()) + } +} diff --git a/codex-rs/tui/src/onboarding/auth.rs b/codex-rs/tui/src/onboarding/auth.rs index 05687bb1bd..56527ac8ef 100644 --- a/codex-rs/tui/src/onboarding/auth.rs +++ b/codex-rs/tui/src/onboarding/auth.rs @@ -1,6 +1,7 @@ #![allow(clippy::unwrap_used)] use codex_core::AuthManager; +use codex_core::auth::AuthCredentialsStoreMode; use codex_core::auth::CLIENT_ID; use codex_core::auth::login_with_api_key; use codex_core::auth::read_openai_api_key_from_env; @@ -148,6 +149,7 @@ pub(crate) struct AuthModeWidget { pub error: Option, pub sign_in_state: Arc>, pub codex_home: PathBuf, + pub cli_auth_credentials_store_mode: AuthCredentialsStoreMode, pub login_status: LoginStatus, pub auth_manager: Arc, pub forced_chatgpt_workspace_id: Option, @@ -512,7 +514,11 @@ impl AuthModeWidget { self.disallow_api_login(); return; } - match login_with_api_key(&self.codex_home, &api_key) { + match login_with_api_key( + &self.codex_home, + &api_key, + self.cli_auth_credentials_store_mode, + ) { Ok(()) => { self.error = None; self.login_status = LoginStatus::AuthMode(AuthMode::ApiKey); @@ -553,6 +559,7 @@ impl AuthModeWidget { self.codex_home.clone(), CLIENT_ID.to_string(), self.forced_chatgpt_workspace_id.clone(), + self.cli_auth_credentials_store_mode, ); match run_login_server(opts) { Ok(child) => { @@ -640,6 +647,8 @@ mod tests { use pretty_assertions::assert_eq; use tempfile::TempDir; + use codex_core::auth::AuthCredentialsStoreMode; + fn widget_forced_chatgpt() -> (AuthModeWidget, TempDir) { let codex_home = TempDir::new().unwrap(); let codex_home_path = codex_home.path().to_path_buf(); @@ -649,8 +658,13 @@ mod tests { error: None, sign_in_state: Arc::new(RwLock::new(SignInState::PickMode)), codex_home: codex_home_path.clone(), + cli_auth_credentials_store_mode: AuthCredentialsStoreMode::File, login_status: LoginStatus::NotAuthenticated, - auth_manager: AuthManager::shared(codex_home_path, false), + auth_manager: AuthManager::shared( + codex_home_path, + false, + AuthCredentialsStoreMode::File, + ), forced_chatgpt_workspace_id: None, forced_login_method: Some(ForcedLoginMethod::Chatgpt), }; diff --git a/codex-rs/tui/src/onboarding/onboarding_screen.rs b/codex-rs/tui/src/onboarding/onboarding_screen.rs index 369e6ddbb0..709f158b80 100644 --- a/codex-rs/tui/src/onboarding/onboarding_screen.rs +++ b/codex-rs/tui/src/onboarding/onboarding_screen.rs @@ -87,6 +87,7 @@ impl OnboardingScreen { let forced_chatgpt_workspace_id = config.forced_chatgpt_workspace_id.clone(); let forced_login_method = config.forced_login_method; let codex_home = config.codex_home; + let cli_auth_credentials_store_mode = config.cli_auth_credentials_store_mode; let mut steps: Vec = Vec::new(); if show_windows_wsl_screen { steps.push(Step::Windows(WindowsSetupWidget::new(codex_home.clone()))); @@ -106,6 +107,7 @@ impl OnboardingScreen { error: None, sign_in_state: Arc::new(RwLock::new(SignInState::PickMode)), codex_home: codex_home.clone(), + cli_auth_credentials_store_mode, login_status, auth_manager, forced_chatgpt_workspace_id, diff --git a/codex-rs/tui/src/status/helpers.rs b/codex-rs/tui/src/status/helpers.rs index e039f0e898..fdbaabd38c 100644 --- a/codex-rs/tui/src/status/helpers.rs +++ b/codex-rs/tui/src/status/helpers.rs @@ -83,7 +83,8 @@ pub(crate) fn compose_agents_summary(config: &Config) -> String { } pub(crate) fn compose_account_display(config: &Config) -> Option { - let auth = load_auth_dot_json(&config.codex_home).ok()??; + let auth = + load_auth_dot_json(&config.codex_home, config.cli_auth_credentials_store_mode).ok()??; if let Some(tokens) = auth.tokens.as_ref() { let info = &tokens.id_token; diff --git a/docs/config.md b/docs/config.md index 9223f48d26..0d81a64efc 100644 --- a/docs/config.md +++ b/docs/config.md @@ -836,7 +836,9 @@ notifications = [ "agent-turn-complete", "approval-requested" ] > [!NOTE] > `tui.notifications` is built‑in and limited to the TUI session. For programmatic or cross‑environment notifications—or to integrate with OS‑specific notifiers—use the top‑level `notify` option to run an external program that receives event JSON. The two settings are independent and can be used together. -## Forcing a login method +## Authentication and authorization + +### Forcing a login method To force users on a given machine to use a specific login method or workspace, use a combination of [managed configurations](https://developers.openai.com/codex/security#managed-configuration) as well as either or both of the following fields: @@ -852,6 +854,22 @@ If the active credentials don't match the config, the user will be logged out an If `forced_chatgpt_workspace_id` is set but `forced_login_method` is not set, API key login will still work. +### Control where login credentials are stored + +```toml +cli_auth_credentials_store = "keyring" +``` + +Valid values: + +- `file` (default) – Store credentials in `auth.json` under `$CODEX_HOME`. +- `keyring` – Store credentials in the operating system keyring via the [`keyring` crate](https://crates.io/crates/keyring); the CLI reports an error if secure storage is unavailable. Backends by OS: + - macOS: macOS Keychain + - Windows: Windows Credential Manager + - Linux: DBus‑based Secret Service, the kernel keyutils, or a combination + - FreeBSD/OpenBSD: DBus‑based Secret Service +- `auto` – Save credentials to the operating system keyring when available; otherwise, fall back to `auth.json` under `$CODEX_HOME`. + ## Config reference | Key | Type / Values | Notes | @@ -910,4 +928,5 @@ If `forced_chatgpt_workspace_id` is set but `forced_login_method` is not set, AP | `tools.web_search` | boolean | Enable web search tool (alias: `web_search_request`) (default: false). | | `forced_login_method` | `chatgpt` \| `api` | Only allow Codex to be used with ChatGPT or API keys. | | `forced_chatgpt_workspace_id` | string (uuid) | Only allow Codex to be used with the specified ChatGPT workspace. | +| `cli_auth_credentials_store` | `file` \| `keyring` \| `auto` | Where to store CLI login credentials (default: `file`). | | `tools.view_image` | boolean | Enable the `view_image` tool so Codex can attach local image files from the workspace (default: false). |