Commit Graph

7 Commits

  • Reject directory rollout paths for pathless side chats (#25661)
    ## Why
    
    Fixes openai/codex#20944.
    
    Desktop side chats are intentionally ephemeral and pathless. They can
    still accept live turns while loaded, but after a reload there is no
    persisted rollout to resume. In the reported failure mode, Desktop could
    send `$CODEX_HOME` as the resume/fork path for one of these pathless
    side chats.
    
    `thread/resume` and `thread/fork` prefer an explicit `path` over
    `threadId`, and rollout path lookup only checked that a candidate
    existed. That let `$CODEX_HOME` pass as a rollout path, so the later
    rollout reader tried to open a directory and surfaced the low-level `Is
    a directory` error.
    
    ## What Changed
    
    - Reject explicit rollout paths that resolve to a directory or other
    non-file before attempting to read rollout history.
    - Make `codex_rollout::existing_rollout_path` return only plain or
    compressed rollout candidates that are actual files.
    - Add an app-server regression test that creates an ephemeral fork, runs
    a turn while the side thread is loaded, simulates reload, then verifies
    both `thread/resume` and `thread/fork` reject `$CODEX_HOME` with `path
    is a directory` instead of the OS-level directory-read error.
    - Rebase over the `TestAppServer` rename and update the remaining stale
    test harness call sites to use `TestAppServer` with `app_server` local
    variables.
    
    Relevant code:
    
    - `thread-store/src/local/read_thread.rs` validates explicit rollout
    paths before rollout reading:
    https://github.com/openai/codex/blob/25b47c8f425d351aaba4baa955a8092064a1707b/codex-rs/thread-store/src/local/read_thread.rs#L146-L165
    - `rollout/src/compression.rs` now requires file metadata for plain and
    compressed rollout candidates:
    https://github.com/openai/codex/blob/25b47c8f425d351aaba4baa955a8092064a1707b/codex-rs/rollout/src/compression.rs#L940-L950
    - The repro test covers the pathless ephemeral side-chat reload case:
    https://github.com/openai/codex/blob/25b47c8f425d351aaba4baa955a8092064a1707b/codex-rs/app-server/tests/suite/v2/thread_fork.rs#L774-L886
    
    ## Verification
    
    - `just test -p codex-app-server
    pathless_ephemeral_thread_rejects_codex_home_path_after_reload`
  • Add rollout compression histograms (#25680)
    ## Summary
    
    Stacked on #25679. Add histogram telemetry for rollout compression
    runtime, per-file compression time, byte sizes, and compression ratio.
    
    ## Changes
    
    - Emit `codex.rollout_compression.run.duration_ms` tagged by final run
    status.
    - Emit `codex.rollout_compression.file.duration_ms` tagged by file
    outcome.
    - Emit source and compressed byte histograms for compression
    candidates/results.
    - Emit `codex.rollout_compression.file.compression_ratio` for successful
    compressions, recorded as integer basis points.
    
    ## Validation
    
    - `just fmt`
    - `just test -p codex-rollout`
    - `just fix -p codex-rollout`
  • Add rollout compression counters (#25679)
    ## Summary
    
    Add counter telemetry for the local rollout compression worker so we can
    see when it runs, why it skips, and how individual file/materialization
    paths resolve.
    
    ## Changes
    
    - Emit `codex.rollout_compression.run` with statuses for start,
    completion, failure, duplicate-run skip, and missing runtime skip.
    - Emit `codex.rollout_compression.file` outcomes for scanned,
    compressed, skipped, and failed compression candidates.
    - Emit `codex.rollout_compression.temp_cleanup` and
    `codex.rollout_compression.materialize` counters for cleanup and
    decompression paths.
    
    ## Validation
    
    - `just fmt`
    - `just test -p codex-rollout`
    - `just fix -p codex-rollout`
  • Throttle repeated rollout compression runs (#25659)
    ## Why
    
    [#25089](https://github.com/openai/codex/pull/25089) introduced the
    background worker that compresses cold archived rollouts, and
    [#25654](https://github.com/openai/codex/pull/25654) made that pass
    faster once it starts. But the worker still deleted
    `rollout-compression.lock` on successful exit, so the existing six-hour
    staleness window only helped with overlapping or crashed workers. Each
    new local thread-store initialization could immediately rescan archived
    rollouts even if a full pass had just finished.
    
    This change keeps the existing marker around long enough to throttle
    redundant reruns. The worker is still best-effort, but it no longer does
    repeated startup scans when nothing new is eligible for compression.
    
    ## What Changed
    
    - Replace the drop-scoped `CompressionLock` with a
    `CompressionRunMarker` that claims the existing
    `.tmp/rollout-compression.lock` path and leaves it in place after
    success.
    - Reuse the existing six-hour staleness window to block both overlapping
    starts and immediate reruns, while still letting a stale marker be
    reclaimed.
    - Update the worker docs and debug logging to describe the new "already
    running or recently ran" behavior.
    - Extend the rollout compression tests to assert that a successful run
    leaves the marker behind and that a fresh marker suppresses a new run.
    
    ## Validation
    
    - `just test -p codex-rollout`
  • Parallelize cold rollout compression (#25654)
    ## Why
    
    [#25089](https://github.com/openai/codex/pull/25089) added the
    background worker for compressing cold archived rollouts, but the worker
    still processed files effectively one at a time: each compression job
    was sent to `spawn_blocking` and then awaited before the next file
    started. On machines with a backlog of archived rollouts, that makes
    catch-up slower than it needs to be even though the actual compression
    work already runs off the async runtime.
    
    ## What Changed
    
    - Queue rollout compression work in a `JoinSet` while directory
    traversal continues.
    - Cap the worker at two in-flight compression jobs so it can overlap
    compression without turning the background task into unbounded blocking
    work.
    - Drain pending jobs before returning, including the
    `read_dir.next_entry()` error path, so every launched job still
    contributes to the final `compressed`, `skipped`, and `failed` stats.
    - Treat task join failures the same way as compression failures in the
    worker's warning and failure accounting.
  • Compress cold local rollouts (#25089)
    ## Rollout compression stack
    
    This stack splits #24941 into reviewable steps for local rollout
    compression. The design is intentionally staged:
    
    1. Teach readers, listing, search, and lookup to understand compressed
    rollouts.
    2. Make append and resume paths materialize compressed rollouts back to
    plain JSONL before writing.
    3. Add a disabled-by-default worker that can compress cold archived
    rollouts behind `local_thread_store_compression`.
    
    The key invariant is that writers append to plain `.jsonl`. A
    `.jsonl.zst` file is a cold/read representation; if a write is needed,
    the compressed file is materialized back to plain JSONL first. Readers
    prefer plain `.jsonl` when both forms exist and can fall back to the
    compressed sibling during transitions.
    
    The worker is deliberately the last PR and remains behind an
    under-development feature flag. It currently scans only
    `archived_sessions`, not active `sessions`, because active sessions have
    the highest resume/append race risk. That means this stack does not yet
    compress most unarchived local history.
    
    ## Known race / follow-up
    
    The remaining unresolved design question is writer/compressor
    coordination. Even for archived rollouts, a resume or metadata update
    can append while the worker is replacing the plain file with
    `.jsonl.zst`; the current double-stat checks narrow but do not fully
    eliminate the window where a writer has opened the plain file before
    unlink. Do not treat the worker PR as production-ready until we either:
    
    - prevent append/resume paths from racing archived compression, or
    - introduce a shared representation/append lock or equivalent
    coordination.
    
    The first two PRs are useful independently: they make compressed
    rollouts readable and make append paths safely recover back to plain
    JSONL. The third PR isolates the worker behavior so that coordination
    issue is reviewable separately.
    
    ## Validation
    
    Focused local validation for the stack includes:
    
    - `just test -p codex-rollout`
    - `just test -p codex-thread-store` where thread-store paths were
    touched
    - `just test -p codex-features` for the feature flag slice
    - `just bazel-lock-check` after dependency graph changes
    - scoped `just fix -p ...` passes for changed crates
    
    CI is still the source of truth for the full platform matrix.
    
    ## This PR in the stack
    
    This is PR 3/3, based on #25088. It adds the under-development feature
    flag and starts the best-effort background worker when enabled. The
    worker currently compresses only cold archived rollouts, skips active
    sessions, verifies compressed output, preserves mtime and permissions,
    keeps a store-level lock heartbeat, and cleans stale temp files.
    
    Stack order:
    
    1. #25087: read compressed local rollouts.
    2. #25088: materialize compressed rollouts before append.
    3. This PR: add the disabled local compression worker.
  • Read compressed rollouts and materialize before append (#25087)
    ## Why
    
    Local rollout compression needs a cold `.jsonl.zst` representation
    without letting compressed physical paths leak into append-mode writers.
    The unsafe case is resume or metadata update code successfully reading a
    compressed rollout and then appending raw JSONL bytes to the zstd file.
    
    This PR folds the former #25088 materialization slice into the
    read-support PR so the reader changes and append-safety invariant land
    together.
    
    ## What Changed
    
    - Teach rollout readers, discovery, listing, search, and ID lookup to
    understand compressed `.jsonl.zst` rollouts.
    - Keep `.jsonl` as the logical/stored rollout path while allowing read
    paths to open either plain or compressed storage.
    - Materialize compressed rollouts back to plain `.jsonl` before
    append-mode writes, including resume and direct metadata append paths.
    - Preserve compressed-file permissions when materializing back to plain
    JSONL.
    - Refresh thread-store resolved rollout paths after compatibility
    metadata writes so reconciliation follows the materialized file.
    - Avoid treating transient compression temp files as real rollout lookup
    results.
    
    ## Remaining Stack
    
    #25089 remains the separate worker PR. It is based directly on this PR
    and stays behind the disabled `local_thread_store_compression` feature
    flag.
    
    The worker still has a broader coordination question: a resume or
    metadata update can race with background compression while a plain file
    is being replaced by `.jsonl.zst`. This PR handles the read and
    materialize-before-append primitives; it does not make the worker
    production-ready.
    
    ## Validation
    
    - `just test -p codex-rollout`
    - `just test -p codex-thread-store`
    - `just fix -p codex-rollout`
    - `just fix -p codex-thread-store`
    - `just bazel-lock-check`