Commit Graph

11 Commits

  • core: cache turn diff rendering (#27489)
    ## Summary
    
    Turn diff updates repeatedly rendered and serialized the entire
    accumulated diff after every `apply_patch`. The event path also rendered
    once before updating the tracker solely to test whether a diff existed.
    In production feedback CODEX-20PW, 2,589 patches across 72 paths
    produced 401 notifications totaling 441 MB, with the hottest paths
    patched 518 and 495 times.
    
    This change:
    
    - replaces the pre-update render with a cheap cached-state check
    - caches each rendered file diff by path and content revision, so an
    update only invokes Myers for affected paths
    - caches the deterministic aggregate diff so event emission and turn
    completion reuse it without recomputation
    - preserves invalidation and net-zero clear notifications
    - applies a 100 ms per-file `similar` timeout; ordinary files complete
    far below this threshold, while pathological rewrites fall back to a
    coarse unified hunk that still represents the exact final contents
    
    The 100 ms deadline bounds synchronous tool-completion latency while
    leaving substantial headroom for normal diffs. The regression test
    applies the fallback diff through the repository's patch parser and
    verifies byte-for-byte final contents.
    
    ## Validation
    
    - `cargo test -p codex-core turn_diff_tracker::tests` (14 passed)
    - `cargo test -p codex-core tools::events::tests` (4 passed)
    - `just fix -p codex-core`
    - `just fmt`
    
    Focused coverage verifies that 42 updates across two files perform 42
    file renders rather than repeatedly rendering the accumulated set,
    unchanged paths are not re-diffed, clear events remain correct, and a
    48,000-line near-total rewrite returns promptly and applies to the exact
    expected result. The full `codex-core` suite was not used as the final
    gate because an unrelated existing multi-agent test hit a stack overflow
    when run during investigation.
    
    ## Bug context
    
    - Sentry feedback: CODEX-20PW
    - Correlation IDs: `019eb2a9-13d2-74e0-b690-27ee224ffb6d`,
    `019e9ad7-09c3-7cb2-b728-ee3acba103ab`
  • Make turn diff tracker multi-env aware (#26433)
    ## Why
    
    Turn diffs were tracked as one flat set of absolute paths. In
    multi-environment turns, local and remote environments can report the
    same path while representing different filesystems, so a single path key
    can collapse distinct changes or attribute them to the wrong
    environment.
    
    The environment name is **NOT** included in the generated unified diff.
    This can come later.
  • fix: preserve exact turn diffs after partial apply_patch failures (#21518)
    ## Why
    
    Follow-up to #21180: turn diffs are operation-backed now, but a failed
    `apply_patch` can still leave exact filesystem mutations behind. For
    example, a move can write the destination file before failing to remove
    the source. Treating the whole call as unknowable then drops a change
    that Codex actually knows happened, so the emitted turn diff can drift
    from the workspace.
    
    ## What changed
    
    -
    [`apply-patch`](https://github.com/openai/codex/blob/f55724e0276a9b3213170daf2701ccfa0ce22646/codex-rs/apply-patch/src/lib.rs#L248-L345)
    now returns `ApplyPatchFailure` with the exact committed prefix
    accumulated before an error. If a write failure may already have mutated
    the target, the delta is marked inexact instead of being reused blindly.
    - Move handling now records the destination write before attempting
    source removal, so a partially failed move can still report the
    destination file that definitely landed
    ([code](https://github.com/openai/codex/blob/f55724e0276a9b3213170daf2701ccfa0ce22646/codex-rs/apply-patch/src/lib.rs#L463-L521)).
    -
    [`ApplyPatchRuntime`](https://github.com/openai/codex/blob/f55724e0276a9b3213170daf2701ccfa0ce22646/codex-rs/core/src/tools/runtimes/apply_patch.rs#L49-L67)
    now accumulates committed deltas across attempts and forwards them even
    when the visible tool result is failed or sandbox-denied ([runtime
    path](https://github.com/openai/codex/blob/f55724e0276a9b3213170daf2701ccfa0ce22646/codex-rs/core/src/tools/runtimes/apply_patch.rs#L223-L250),
    [event
    path](https://github.com/openai/codex/blob/f55724e0276a9b3213170daf2701ccfa0ce22646/codex-rs/core/src/tools/events.rs#L215-L225)).
    - `TurnDiffTracker` now consumes committed exact deltas rather than only
    fully successful patches; exact-empty failures leave the aggregate
    unchanged, while inexact deltas still invalidate it.
    
    ## Verification
    
    - Added a regression test covering a failed move that still emits the
    committed destination diff:
    [`apply_patch_failed_move_preserves_committed_destination_diff`](https://github.com/openai/codex/blob/f55724e0276a9b3213170daf2701ccfa0ce22646/codex-rs/core/tests/suite/apply_patch_cli.rs#L1517-L1586).
    - Kept explicit coverage that an inexact delta clears the aggregate
    instead of publishing a guessed diff:
    [`apply_patch_clears_aggregated_diff_after_inexact_delta`](https://github.com/openai/codex/blob/f55724e0276a9b3213170daf2701ccfa0ce22646/codex-rs/core/tests/suite/apply_patch_cli.rs#L1589-L1655).
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • Make turn diff tracking operation backed (#21180)
    ## Summary
    - replace filesystem-based turn diff tracking with an operation-backed
    accumulator
    - preserve enough verified apply_patch state to render move-overwrite
    cases correctly
    - keep the turn/diff/updated contract intact while removing remote-only
    turn-diff test skips
    
    This takes the assumption that no 3P services rely on the output format
    of `apply_patch`
    
    ## Why
    For the CCA file system isolation push
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • 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`.
  • fix: move inline codex-rs/core unit tests into sibling files (#14444)
    ## Why
    PR #13783 moved the `codex.rs` unit tests into `codex_tests.rs`. This
    applies the same extraction pattern across the rest of `codex-rs/core`
    so the production modules stay focused on runtime code instead of large
    inline test blocks.
    
    Keeping the tests in sibling files also makes follow-up edits easier to
    review because product changes no longer have to share a file with
    hundreds or thousands of lines of test scaffolding.
    
    ## What changed
    - replaced each inline `mod tests { ... }` in `codex-rs/core/src/**`
    with a path-based module declaration
    - moved each extracted unit test module into a sibling `*_tests.rs`
    file, using `mod_tests.rs` for `mod.rs` modules
    - preserved the existing `cfg(...)` guards and module-local structure so
    the refactor remains structural rather than behavioral
    
    ## Testing
    - `cargo test -p codex-core --lib` (`1653 passed; 0 failed; 5 ignored`)
    - `just fix -p codex-core`
    - `cargo fmt --check`
    - `cargo shear`
  • chore: more clippy rules 2 (#4057)
    The only file to watch is the cargo.toml
    All the others come from just fix + a few manual small fix
    
    The set of rules have been taken from the list of clippy rules
    arbitrarily while trying to optimise the learning and style of the code
    while limiting the loss of productivity
  • chore: enable clippy::redundant_clone (#3489)
    Created this PR by:
    
    - adding `redundant_clone` to `[workspace.lints.clippy]` in
    `cargo-rs/Cargol.toml`
    - running `cargo clippy --tests --fix`
    - running `just fmt`
    
    Though I had to clean up one instance of the following that resulted:
    
    ```rust
    let codex = codex;
    ```
  • Added allow-expect-in-tests / allow-unwrap-in-tests (#2328)
    This PR:
    * Added the clippy.toml to configure allowable expect / unwrap usage in
    tests
    * Removed as many expect/allow lines as possible from tests
    * moved a bunch of allows to expects where possible
    
    Note: in integration tests, non `#[test]` helper functions are not
    covered by this so we had to leave a few lingering `expect(expect_used`
    checks around
  • Add a TurnDiffTracker to create a unified diff for an entire turn (#1770)
    This lets us show an accumulating diff across all patches in a turn.
    Refer to the docs for TurnDiffTracker for implementation details.
    
    There are multiple ways this could have been done and this felt like the
    right tradeoff between reliability and completeness:
    *Pros*
    * It will pick up all changes to files that the model touched including
    if they prettier or another command that updates them.
    * It will not pick up changes made by the user or other agents to files
    it didn't modify.
    
    *Cons*
    * It will pick up changes that the user made to a file that the model
    also touched
    * It will not pick up changes to codegen or files that were not modified
    with apply_patch