Commit Graph

12 Commits

  • core: cut codex-core compile time 48% with native async SessionTask (#16631)
    ## Why
    
    This continues the compile-time cleanup from #16630. `SessionTask`
    implementations are monomorphized, but `Session` stores the task behind
    a `dyn` boundary so it can drive and abort heterogenous turn tasks
    uniformly. That means we can move the `#[async_trait]` expansion off the
    implementation trait, keep a small boxed adapter only at the storage
    boundary, and preserve the existing task lifecycle semantics while
    reducing the amount of generated async-trait glue in `codex-core`.
    
    One measurement caveat showed up while exploring this: a warm
    incremental benchmark based on `touch core/src/tasks/mod.rs && cargo
    check -p codex-core --lib` was basically flat, but that was the wrong
    benchmark for this change. Using package-clean `codex-core` rebuilds,
    like #16630, shows the real win.
    
    Relevant pre-change code:
    
    - [`SessionTask` with
    `#[async_trait]`](https://github.com/openai/codex/blob/3c7f013f9735e67796c70d95f75f436b7f97e3ec/codex-rs/core/src/tasks/mod.rs#L129-L182)
    - [`RunningTask` storing `Arc<dyn
    SessionTask>`](https://github.com/openai/codex/blob/3c7f013f9735e67796c70d95f75f436b7f97e3ec/codex-rs/core/src/state/turn.rs#L69-L77)
    
    ## What changed
    
    - Switched `SessionTask::{run, abort}` to native RPITIT futures with
    explicit `Send` bounds.
    - Added a private `AnySessionTask` adapter that boxes those futures only
    at the `Arc<dyn ...>` storage boundary.
    - Updated `RunningTask` to store `Arc<dyn AnySessionTask>` and removed
    `#[async_trait]` from the concrete task impls plus test-only
    `SessionTask` impls.
    
    ## Timing
    
    Benchmarked package-clean `codex-core` rebuilds with dependencies left
    warm:
    
    ```shell
    cargo check -p codex-core --lib >/dev/null
    cargo clean -p codex-core >/dev/null
    /usr/bin/time -p cargo +nightly rustc -p codex-core --lib -- \
      -Z time-passes \
      -Z time-passes-format=json >/dev/null
    ```
    
    | revision | rustc `total` | process `real` | `generate_crate_metadata`
    | `MIR_borrow_checking` | `monomorphization_collector_graph_walk` |
    | --- | ---: | ---: | ---: | ---: | ---: |
    | parent `3c7f013f9735` | 67.21s | 67.71s | 24.61s | 23.43s | 22.43s |
    | this PR `2cafd783ac22` | 35.08s | 35.60s | 8.01s | 7.25s | 7.15s |
    | delta | -47.8% | -47.4% | -67.5% | -69.1% | -68.1% |
    
    For completeness, the warm touched-file benchmark stayed flat (`1.96s`
    parent vs `1.97s` this PR), which is why that benchmark should not be
    used to evaluate this refactor.
    
    ## Verification
    
    - Ran `cargo test -p codex-core`; this change compiled and task-related
    tests passed before hitting the same unrelated 5
    `config::tests::*guardian*` failures already present on the parent
    stack.
  • core: remove cross-crate re-exports from lib.rs (#16512)
    ## Why
    
    `codex-core` was re-exporting APIs owned by sibling `codex-*` crates,
    which made downstream crates depend on `codex-core` as a proxy module
    instead of the actual owner crate.
    
    Removing those forwards makes crate boundaries explicit and lets leaf
    crates drop unnecessary `codex-core` dependencies. In this PR, this
    reduces the dependency on `codex-core` to `codex-login` in the following
    files:
    
    ```
    codex-rs/backend-client/Cargo.toml
    codex-rs/mcp-server/tests/common/Cargo.toml
    ```
    
    ## What
    
    - Remove `codex-rs/core/src/lib.rs` re-exports for symbols owned by
    `codex-login`, `codex-mcp`, `codex-rollout`, `codex-analytics`,
    `codex-protocol`, `codex-shell-command`, `codex-sandboxing`,
    `codex-tools`, and `codex-utils-path`.
    - Delete the `default_client` forwarding shim in `codex-rs/core`.
    - Update in-crate and downstream callsites to import directly from the
    owning `codex-*` crate.
    - Add direct Cargo dependencies where callsites now target the owner
    crate, and remove `codex-core` from `codex-rs/backend-client`.
  • Move git utilities into a dedicated crate (#15564)
    - create `codex-git-utils` and move the shared git helpers into it with
    file moves preserved for diff readability
    - move the `GitInfo` helpers out of `core` so stacked rollout work can
    depend on the shared crate without carrying its own git info module
    
    ---------
    
    Co-authored-by: Ahmed Ibrahim <219906144+aibrahim-oai@users.noreply.github.com>
    Co-authored-by: Codex <noreply@openai.com>
  • Apply argument comment lint across codex-rs (#14652)
    ## Why
    
    Once the repo-local lint exists, `codex-rs` needs to follow the
    checked-in convention and CI needs to keep it from drifting. This commit
    applies the fallback `/*param*/` style consistently across existing
    positional literal call sites without changing those APIs.
    
    The longer-term preference is still to avoid APIs that require comments
    by choosing clearer parameter types and call shapes. This PR is
    intentionally the mechanical follow-through for the places where the
    existing signatures stay in place.
    
    After rebasing onto newer `main`, the rollout also had to cover newly
    introduced `tui_app_server` call sites. That made it clear the first cut
    of the CI job was too expensive for the common path: it was spending
    almost as much time installing `cargo-dylint` and re-testing the lint
    crate as a representative test job spends running product tests. The CI
    update keeps the full workspace enforcement but trims that extra
    overhead from ordinary `codex-rs` PRs.
    
    ## What changed
    
    - keep a dedicated `argument_comment_lint` job in `rust-ci`
    - mechanically annotate remaining opaque positional literals across
    `codex-rs` with exact `/*param*/` comments, including the rebased
    `tui_app_server` call sites that now fall under the lint
    - keep the checked-in style aligned with the lint policy by using
    `/*param*/` and leaving string and char literals uncommented
    - cache `cargo-dylint`, `dylint-link`, and the relevant Cargo
    registry/git metadata in the lint job
    - split changed-path detection so the lint crate's own `cargo test` step
    runs only when `tools/argument-comment-lint/*` or `rust-ci.yml` changes
    - continue to run the repo wrapper over the `codex-rs` workspace, so
    product-code enforcement is unchanged
    
    Most of the code changes in this commit are intentionally mechanical
    comment rewrites or insertions driven by the lint itself.
    
    ## Verification
    
    - `./tools/argument-comment-lint/run.sh --workspace`
    - `cargo test -p codex-tui-app-server -p codex-tui`
    - parsed `.github/workflows/rust-ci.yml` locally with PyYAML
    
    ---
    
    * -> #14652
    * #14651
  • chore(otel): rename OtelManager to SessionTelemetry (#13808)
    ## Summary
    This is a purely mechanical refactor of `OtelManager` ->
    `SessionTelemetry` to better convey what the struct is doing. No
    behavior change.
    
    ## Why
    
    `OtelManager` ended up sounding much broader than what this type
    actually does. It doesn't manage OTEL globally; it's the session-scoped
    telemetry surface for emitting log/trace events and recording metrics
    with consistent session metadata (`app_version`, `model`, `slug`,
    `originator`, etc.).
    
    `SessionTelemetry` is a more accurate name, and updating the call sites
    makes that boundary a lot easier to follow.
    
    ## Validation
    
    - `just fmt`
    - `cargo test -p codex-otel`
    - `cargo test -p codex-core`
  • feat(core, tracing): add a span representing a turn (#13424)
    This is PR 3 of the app-server tracing rollout.
    
    PRs https://github.com/openai/codex/pull/13285 and
    https://github.com/openai/codex/pull/13368 gave us inbound request spans
    in app-server and propagated trace context through Submission. This
    change finishes the next piece in core: when a request actually starts a
    turn, we now create a core-owned long-lived span that stays open for the
    real lifetime of the turn.
    
    What changed:
    - `Session::spawn_task` can now optionally create a long-lived turn span
    and run the spawned task inside it
    - `turn/start` uses that path, so normal turn execution stays under a
    single core-owned span after the async handoff
    - `review/start` uses the same pattern
    - added a unit test that verifies the spawned turn task inherits the
    submission dispatch trace ancestry
    
    **Why**
    The app-server request span is intentionally short-lived. Once work
    crosses into core, we still want one span that covers the actual
    execution window until completion or interruption. This keeps that
    ownership where it belongs: in the layer that owns the runtime
    lifecycle.
  • Fix compaction context reinjection and model baselines (#12252)
    ## Summary
    - move regular-turn context diff/full-context persistence into
    `run_turn` so pre-turn compaction runs before incoming context updates
    are recorded
    - after successful pre-turn compaction, rely on a cleared
    `reference_context_item` to trigger full context reinjection on the
    follow-up regular turn (manual `/compact` keeps replacement history
    summary-only and also clears the baseline)
    - preserve `<model_switch>` when full context is reinjected, and inject
    it *before* the rest of the full-context items
    - scope `reference_context_item` and `previous_model` to regular user
    turns only so standalone tasks (`/compact`, shell, review, undo) cannot
    suppress future reinjection or `<model_switch>` behavior
    - make context-diff persistence + `reference_context_item` updates
    explicit in the regular-turn path, with clearer docs/comments around the
    invariant
    - stop persisting local `/compact` `RolloutItem::TurnContext` snapshots
    (only regular turns persist `TurnContextItem` now)
    - simplify resume/fork previous-model/reference-baseline hydration by
    looking up the last surviving turn context from rollout lifecycle
    events, including rollback and compaction-crossing handling
    - remove the legacy fallback that guessed from bare `TurnContext`
    rollouts without lifecycle events
    - update compaction/remote-compaction/model-visible snapshots and
    compact test assertions (including remote compaction mock response
    shape)
    
    ## Why
    We were persisting incoming context items before spawning the regular
    turn task, which let pre-turn compaction requests accidentally include
    incoming context diffs without the new user message. Fixing that exposed
    follow-on baseline issues around `/compact`, resume/fork, and standalone
    tasks that could cause duplicate context injection or suppress
    `<model_switch>` instructions.
    
    This PR re-centers the invariants around regular turns:
    - regular turns persist model-visible context diffs/full reinjection and
    update the `reference_context_item`
    - standalone tasks do not advance those regular-turn baselines
    - compaction clears the baseline when replacement history may have
    stripped the referenced context diffs
    
    ## Follow-ups (TODOs left in code)
    - `TODO(ccunningham)`: fix rollback/backtracking baseline handling more
    comprehensively
    - `TODO(ccunningham)`: include pending incoming context items in
    pre-turn compaction threshold estimation
    - `TODO(ccunningham)`: inject updated personality spec alongside
    `<model_switch>` so some model-switch paths can avoid forced full
    reinjection
    - `TODO(ccunningham)`: review task turn lifecycle
    (`TurnStarted`/`TurnComplete`) behavior and emit task-start context
    diffs for task types that should have them (excluding `/compact`)
    
    ## Validation
    - `just fmt`
    - CI should cover the updated compaction/resume/model-visible snapshot
    expectations and rollout-hydration behavior
    - I did **not** rerun the full local test suite after the latest
    resume-lookup / rollout-persistence simplifications
  • feat: metrics capabilities (#8318)
    Add metrics capabilities to Codex. The `README.md` is up to date.
    
    This will not be merged with the metrics before this PR of course:
    https://github.com/openai/codex/pull/8350
  • chore: merge git crates (#5909)
    Merge `git-apply` and `git-tooling` into `utils/`