Commit Graph

13 Commits

  • remove temporary ownership re-exports (#16626)
    Stacked on #16508.
    
    This removes the temporary `codex-core` / `codex-login` re-export shims
    from the ownership split and rewrites callsites to import directly from
    `codex-model-provider-info`, `codex-models-manager`, `codex-api`,
    `codex-protocol`, `codex-feedback`, and `codex-response-debug-context`.
    
    No behavior change intended; this is the mechanical import cleanup layer
    split out from the ownership move.
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • extract models manager and related ownership from core (#16508)
    ## Summary
    - split `models-manager` out of `core` and add `ModelsManagerConfig`
    plus `Config::to_models_manager_config()` so model metadata paths stop
    depending on `core::Config`
    - move login-owned/auth-owned code out of `core` into `codex-login`,
    move model provider config into `codex-model-provider-info`, move API
    bridge mapping into `codex-api`, move protocol-owned types/impls into
    `codex-protocol`, and move response debug helpers into a dedicated
    `response-debug-context` crate
    - move feedback tag emission into `codex-feedback`, relocate tests to
    the crates that now own the code, and keep broad temporary re-exports so
    this PR avoids a giant import-only rewrite
    
    ## Major moves and decisions
    - created `codex-models-manager` as the owner for model
    cache/catalog/config/model info logic, including the new
    `ModelsManagerConfig` struct
    - created `codex-model-provider-info` as the owner for provider config
    parsing/defaults and kept temporary `codex-login`/`codex-core`
    re-exports for old import paths
    - moved `api_bridge` error mapping + `CoreAuthProvider` into
    `codex-api`, while `codex-login::api_bridge` temporarily re-exports
    those symbols and keeps the `auth_provider_from_auth` wrapper
    - moved `auth_env_telemetry` and `provider_auth` ownership to
    `codex-login`
    - moved `CodexErr` ownership to `codex-protocol::error`, plus
    `StreamOutput`, `bytes_to_string_smart`, and network policy helpers to
    protocol-owned modules
    - created `codex-response-debug-context` for
    `extract_response_debug_context`, `telemetry_transport_error_message`,
    and related response-debug plumbing instead of leaving that behavior in
    `core`
    - moved `FeedbackRequestTags`, `emit_feedback_request_tags`, and
    `emit_feedback_request_tags_with_auth_env` to `codex-feedback`
    - deferred removal of temporary re-exports and the mechanical import
    rewrites to a stacked follow-up PR so this PR stays reviewable
    
    ## Test moves
    - moved auth refresh coverage from `core/tests/suite/auth_refresh.rs` to
    `login/tests/suite/auth_refresh.rs`
    - moved text encoding coverage from
    `core/tests/suite/text_encoding_fix.rs` to
    `protocol/src/exec_output_tests.rs`
    - moved model info override coverage from
    `core/tests/suite/model_info_overrides.rs` to
    `models-manager/src/model_info_overrides_tests.rs`
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • test: use cmd.exe for ProviderAuthScript on Windows (#16629)
    ## Why
    
    The Windows `ProviderAuthScript` test helpers do not need PowerShell.
    Running them through `cmd.exe` is enough to emit the next fixture token
    and rotate `tokens.txt`, and it avoids a PowerShell-specific dependency
    in these tests.
    
    ## What changed
    
    - Replaced the Windows `print-token.ps1` fixtures with `print-token.cmd`
    in `codex-rs/core/src/models_manager/manager_tests.rs` and
    `codex-rs/login/src/auth/auth_tests.rs`.
    - Switched the failing external-auth helper in
    `codex-rs/login/src/auth/auth_tests.rs` from `powershell.exe -Command
    'exit 1'` to `cmd.exe /d /s /c 'exit /b 1'`.
    - Updated Windows timeout comments so they no longer call out PowerShell
    specifically.
    
    ## Verification
    
    - `cargo test -p codex-login`
    - `cargo test -p codex-core` (fails in unrelated
    `core/src/config/config_tests.rs` assertions in this checkout)
  • test: deflake external bearer auth token tests on Windows (#16604)
    ## Why
    
    `external_bearer_only_auth_manager_uses_cached_provider_token` can fail
    on Windows when cold `powershell.exe` startup exceeds the provider-auth
    helper's 1s timeout. When that happens,
    `AuthManager::resolve_external_api_key_auth()` [logs the resolver error
    and returns
    `None`](https://github.com/openai/codex/blob/024b08b411fe/codex-rs/login/src/auth/manager.rs#L1449-L1455),
    which is exactly the assertion failure from the flake.
    
    ## What
    
    - Invoke `powershell.exe` explicitly in the Windows provider-auth test
    helpers in `login/src/auth/auth_tests.rs`.
    - Increase the helper timeout to `10_000` ms and document why that slack
    exists.
    
    ## Verification
    
    - `cargo test -p codex-login`
  • login: treat provider auth refresh_interval_ms=0 as no auto-refresh (#16480)
    ## Why
    
    Follow-up to #16288: the new dynamic provider auth token flow currently
    defaults `refresh_interval_ms` to a non-zero value and rejects `0`
    entirely.
    
    For command-backed bearer auth, `0` should mean "never auto-refresh".
    That lets callers keep using the cached token until the backend actually
    returns `401 Unauthorized`, at which point Codex can rerun the auth
    command as part of the existing retry path.
    
    ## What changed
    
    - changed `ModelProviderAuthInfo.refresh_interval_ms` to accept `0` and
    documented that value as disabling proactive refresh
    - updated the external bearer token refresher to treat
    `refresh_interval_ms = 0` as an indefinitely reusable cached token,
    while still rerunning the auth command during unauthorized recovery
    - regenerated `core/config.schema.json` so the schema minimum is `0` and
    the new behavior is described in the field docs
    - added coverage for both config deserialization and the no-auto-refresh
    plus `401` recovery behavior
    
    ## How tested
    
    - `cargo test -p codex-protocol`
    - `cargo test -p codex-login`
    - `cargo test -p codex-core test_deserialize_provider_auth_config_`
  • Fix Windows external bearer refresh test (#16366)
    ## Why
    
    https://github.com/openai/codex/pull/16287 introduced a change to
    `codex-rs/login/src/auth/auth_tests.rs` that uses a PowerShell helper to
    read the next token from `tokens.txt` and rewrite the remainder back to
    disk. On Windows, `Get-Content` can return a scalar when the file has
    only one remaining line, so `$lines[0]` reads the first character
    instead of the full token. That breaks the external bearer refresh test
    once the token list is nearly exhausted.
    
    https://github.com/openai/codex/pull/16288 introduced similar changes to
    `codex-rs/core/src/models_manager/manager_tests.rs` and
    `codex-rs/core/tests/suite/client.rs`.
    
    These went unnoticed because the failures showed up when the test was
    run via Cargo on Windows, but not in our Bazel harness. Figuring out
    that Cargo-vs-Bazel delta will happen in a follow-up PR.
    
    ## Verification
    
    On my Windows machine, I verified `cargo test` passes when run in
    `codex-rs/login` and `codex-rs/core`. Once this PR is merged, I will
    keep an eye on
    https://github.com/openai/codex/actions/workflows/rust-ci-full.yml to
    verify it goes green.
    
    ## What changed
    
    - Wrap `Get-Content -Path tokens.txt` in `@(...)` so the script always
    gets array semantics before counting, indexing, and rewriting the
    remaining lines.
  • Refactor external auth to use a single trait (#16356)
    ## Summary
    - Replace the separate external auth enum and refresher trait with a
    single `ExternalAuth` trait in login auth flow
    - Move bearer token auth behind `BearerTokenRefresher` and update
    `AuthManager` and app-server wiring to use the generic external auth API
  • auth: let AuthManager own external bearer auth (#16287)
    ## Summary
    
    `AuthManager` and `UnauthorizedRecovery` already own token resolution
    and staged `401` recovery. The missing piece for provider auth was a
    bearer-only mode that still fit that design, instead of pushing a second
    auth abstraction into `codex-core`.
    
    This PR keeps the design centered on `AuthManager`: it teaches
    `codex-login` how to own external bearer auth directly so later provider
    work can keep calling `AuthManager.auth()` and `UnauthorizedRecovery`.
    
    ## Motivation
    
    This is the middle layer for #15189.
    
    The intended design is still:
    
    - `AuthManager` encapsulates token storage and refresh
    - `UnauthorizedRecovery` powers staged `401` recovery
    - all request tokens go through `AuthManager.auth()`
    
    This PR makes that possible for provider-backed bearer tokens by adding
    a bearer-only auth mode inside `AuthManager` instead of building
    parallel request-auth plumbing in `core`.
    
    ## What Changed
    
    - move `ModelProviderAuthInfo` into `codex-protocol` so `core` and
    `login` share one config shape
    - add `login/src/auth/external_bearer.rs`, which runs the configured
    command, caches the bearer token in memory, and refreshes it after `401`
    - add `AuthManager::external_bearer_only(...)` for provider-scoped
    request paths that should use command-backed bearer auth without
    mutating the shared OpenAI auth manager
    - add `AuthManager::shared_with_external_chatgpt_auth_refresher(...)`
    and rename the other `AuthManager` helpers that only apply to external
    ChatGPT auth so the ChatGPT-only path is explicit at the call site
    - keep external ChatGPT refresh behavior unchanged while ensuring
    bearer-only external auth never persists to `auth.json`
    
    ## Testing
    
    - `cargo test -p codex-login`
    - `cargo test -p codex-protocol`
    
    
    
    
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/16287).
    * #16288
    * __->__ #16287
  • auth: generalize external auth tokens for bearer-only sources (#16286)
    ## Summary
    
    `ExternalAuthRefresher` was still shaped around external ChatGPT auth:
    `ExternalAuthTokens` always implied ChatGPT account metadata even when a
    caller only needed a bearer token.
    
    This PR generalizes that contract so bearer-only sources are
    first-class, while keeping the existing ChatGPT paths strict anywhere we
    persist or rebuild ChatGPT auth state.
    
    ## Motivation
    
    This is the first step toward #15189.
    
    The follow-on provider-auth work needs one shared external-auth contract
    that can do both of these things:
    
    - resolve the current bearer token before a request is sent
    - return a refreshed bearer token after a `401`
    
    That should not require a second token result type just because there is
    no ChatGPT account metadata attached.
    
    ## What Changed
    
    - change `ExternalAuthTokens` to carry `access_token` plus optional
    `ExternalAuthChatgptMetadata`
    - add helper constructors for bearer-only tokens and ChatGPT-backed
    tokens
    - add `ExternalAuthRefresher::resolve()` with a default no-op
    implementation so refreshers can optionally provide the current token
    before a request is sent
    - keep ChatGPT-only persistence strict by continuing to require ChatGPT
    metadata anywhere the login layer seeds or reloads ChatGPT auth state
    - update the app-server bridge to construct the new token shape for
    external ChatGPT auth refreshes
    
    ## Testing
    
    - `cargo test -p codex-login`
    
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/16286).
    * #16288
    * #16287
    * __->__ #16286
  • chore: clean up argument-comment lint and roll out all-target CI on macOS (#16054)
    ## Why
    
    `argument-comment-lint` was green in CI even though the repo still had
    many uncommented literal arguments. The main gap was target coverage:
    the repo wrapper did not force Cargo to inspect test-only call sites, so
    examples like the `latest_session_lookup_params(true, ...)` tests in
    `codex-rs/tui_app_server/src/lib.rs` never entered the blocking CI path.
    
    This change cleans up the existing backlog, makes the default repo lint
    path cover all Cargo targets, and starts rolling that stricter CI
    enforcement out on the platform where it is currently validated.
    
    ## What changed
    
    - mechanically fixed existing `argument-comment-lint` violations across
    the `codex-rs` workspace, including tests, examples, and benches
    - updated `tools/argument-comment-lint/run-prebuilt-linter.sh` and
    `tools/argument-comment-lint/run.sh` so non-`--fix` runs default to
    `--all-targets` unless the caller explicitly narrows the target set
    - fixed both wrappers so forwarded cargo arguments after `--` are
    preserved with a single separator
    - documented the new default behavior in
    `tools/argument-comment-lint/README.md`
    - updated `rust-ci` so the macOS lint lane keeps the plain wrapper
    invocation and therefore enforces `--all-targets`, while Linux and
    Windows temporarily pass `-- --lib --bins`
    
    That temporary CI split keeps the stricter all-targets check where it is
    already cleaned up, while leaving room to finish the remaining Linux-
    and Windows-specific target-gated cleanup before enabling
    `--all-targets` on those runners. The Linux and Windows failures on the
    intermediate revision were caused by the wrapper forwarding bug, not by
    additional lint findings in those lanes.
    
    ## Validation
    
    - `bash -n tools/argument-comment-lint/run.sh`
    - `bash -n tools/argument-comment-lint/run-prebuilt-linter.sh`
    - shell-level wrapper forwarding check for `-- --lib --bins`
    - shell-level wrapper forwarding check for `-- --tests`
    - `just argument-comment-lint`
    - `cargo test` in `tools/argument-comment-lint`
    - `cargo test -p codex-terminal-detection`
    
    ## Follow-up
    
    - Clean up remaining Linux-only target-gated callsites, then switch the
    Linux lint lane back to the plain wrapper invocation.
    - Clean up remaining Windows-only target-gated callsites, then switch
    the Windows lint lane back to the plain wrapper invocation.
  • Add usage-based business plan types (#15934)
    ## Summary
    - add `self_serve_business_usage_based` and `enterprise_cbp_usage_based`
    to the public/internal plan enums and regenerate the app-server + Python
    SDK artifacts
    - map both plans through JWT login and backend rate-limit payloads, then
    bucket them with the existing Team/Business entitlement behavior in
    cloud requirements, usage-limit copy, tooltips, and status display
    - keep the earlier display-label remap commit on this branch so the new
    Team-like and Business-like plans render consistently in the UI
    
    ## Testing
    - `just write-app-server-schema`
    - `uv run --project sdk/python python
    sdk/python/scripts/update_sdk_artifacts.py generate-types`
    - `just fix -p codex-protocol -p codex-login -p codex-core -p
    codex-backend-client -p codex-cloud-requirements -p codex-tui -p
    codex-tui-app-server -p codex-backend-openapi-models`
    - `just fmt`
    - `just argument-comment-lint`
    - `cargo test -p codex-protocol
    usage_based_plan_types_use_expected_wire_names`
    - `cargo test -p codex-login usage_based`
    - `cargo test -p codex-backend-client usage_based`
    - `cargo test -p codex-cloud-requirements usage_based`
    - `cargo test -p codex-core usage_limit_reached_error_formats_`
    - `cargo test -p codex-tui plan_type_display_name_remaps_display_labels`
    - `cargo test -p codex-tui remapped`
    - `cargo test -p codex-tui-app-server
    plan_type_display_name_remaps_display_labels`
    - `cargo test -p codex-tui-app-server remapped`
    - `cargo test -p codex-tui-app-server
    preserves_usage_based_plan_type_wire_name`
    
    ## Notes
    - a broader multi-crate `cargo test` run still hits unrelated existing
    guardian-approval config failures in
    `codex-rs/core/src/config/config_tests.rs`
  • chore: stop app-server auth refresh storms after permanent token failure (#15530)
    built from #14256. PR description from @etraut-openai:
    
    This PR addresses a hole in [PR
    11802](https://github.com/openai/codex/pull/11802). The previous PR
    assumed that app server clients would respond to token refresh failures
    by presenting the user with an error ("you must log in again") and then
    not making further attempts to call network endpoints using the expired
    token. While they do present the user with this error, they don't
    prevent further attempts to call network endpoints and can repeatedly
    call `getAuthStatus(refreshToken=true)` resulting in many failed calls
    to the token refresh endpoint.
    
    There are three solutions I considered here:
    1. Change the getAuthStatus app server call to return a null auth if the
    caller specified "refreshToken" on input and the refresh attempt fails.
    This will cause clients to immediately log out the user and return them
    to the log in screen. This is a really bad user experience. It's also a
    breaking change in the app server contract that could break third-party
    clients.
    2. Augment the getAuthStatus app server call to return an additional
    field that indicates the state of "token could not be refreshed". This
    is a non-breaking change to the app server API, but it requires
    non-trivial changes for all clients to properly handle this new field
    properly.
    3. Change the getAuthStatus implementation to handle the case where a
    token refresh fails by marking the AuthManager's in-memory access and
    refresh tokens as "poisoned" so it they are no longer used. This is the
    simplest fix that requires no client changes.
    
    I chose option 3.
    
    Here's Codex's explanation of this change:
    
    When an app-server client asks `getAuthStatus(refreshToken=true)`, we
    may try to refresh a stale ChatGPT access token. If that refresh fails
    permanently (for example `refresh_token_reused`, expired, or revoked),
    the old behavior was bad in two ways:
    
    1. We kept the in-memory auth snapshot alive as if it were still usable.
    2. Later auth checks could retry refresh again and again, creating a
    storm of doomed `/oauth/token` requests and repeatedly surfacing the
    same failure.
    
    This is especially painful for app-server clients because they poll auth
    status and can keep driving the refresh path without any real chance of
    recovery.
    
    This change makes permanent refresh failures terminal for the current
    managed auth snapshot without changing the app-server API contract.
    
    What changed:
    - `AuthManager` now poisons the current managed auth snapshot in memory
    after a permanent refresh failure, keyed to the unchanged `AuthDotJson`.
    - Once poisoned, later refresh attempts for that same snapshot fail fast
    locally without calling the auth service again.
    - The poison is cleared automatically when auth materially changes, such
    as a new login, logout, or reload of different auth state from storage.
    - `getAuthStatus(includeToken=true)` now omits `authToken` after a
    permanent refresh failure instead of handing out the stale cached bearer
    token.
    
    This keeps the current auth method visible to clients, avoids forcing an
    immediate logout flow, and stops repeated refresh attempts for
    credentials that cannot recover.
    
    ---------
    
    Co-authored-by: Eric Traut <etraut@openai.com>
  • Move auth code into login crate (#15150)
    - Move the auth implementation and token data into codex-login.
    - Keep codex-core re-exporting that surface from codex-login for
    existing callers.
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>