12 Commits

  • test: use automatic environments in app-server integration tests (#29789)
    ## Why
    
    Topology-neutral app-server integration tests should exercise automatic
    environment selection so the same setup covers local and remote
    executors.
    
    ## What
    
    Migrate eligible tests to `TestAppServer::new_with_auto_env()` and
    `send_thread_start_request_with_auto_env()`. Leave explicit-topology
    tests unchanged, and skip the request-permissions case on Windows with a
    TODO for cross-platform tool routing.
    
    ## Validation
    
    - `just test -p codex-app-server`
    - `bazel test //codex-rs/app-server:app-server-all-wine-exec-test
    --test_output=errors`
    
    Stacked on #29788.
  • fix: rename McpServer to TestAppServer (#25701)
    This PR brought to you via VS Code rather than Codex...
    
    - opened `codex-rs/app-server/tests/common/mcp_process.rs`
    - put the cursor on `McpServer`
    - hit `F2` and renamed the symbol to `TestAppServer`
    - went to the file tree
    - hit enter and renamed `mcp_process.rs` to `test_app_server.rs`
    - ran **Save All Files** from the Command Palette
    - ran `just fmt`
    
    The End
    
    (Admittedly, most of the local variables for `TestAppServer` are still
    named `mcp`, though.)
  • feat(app-server): move v2 sessionId onto Thread (#21336)
    ## Why
    
    `session_id` and `thread_id` are separate identities after #20437, but
    app-server only surfaced `sessionId` on the `thread/start`,
    `thread/resume`, and `thread/fork` response envelopes. Other
    thread-bearing surfaces such as `thread/list`, `thread/read`,
    `thread/started`, `thread/rollback`, `thread/metadata/update`, and
    `thread/unarchive` either lacked the grouping key or forced clients to
    special-case those three responses.
    
    Making `sessionId` part of the reusable `Thread` payload gives every v2
    API surface one place to expose session-tree identity.
    
    ## Mental model
      1. thread.sessionId lives on `Thread`
    2. It is a view/runtime identity for the current live session tree, not
    durable stored lineage metadata
    3. When app-server has a live loaded thread, it copies the real value
    from core’s session_configured.session_id
    4. When it only has stored/unloaded data, it falls back to
    thread.sessionId = thread.id
    
    ## What changed
    
    - Added `sessionId` to the v2
    [`Thread`](https://github.com/openai/codex/blob/8fc9e9b4cf81b6f61d432e71f1eb266f6f104b63/codex-rs/app-server-protocol/src/protocol/v2/thread_data.rs#L105-L109).
    - Removed the duplicate top-level `sessionId` fields from
    `thread/start`, `thread/resume`, and `thread/fork`; clients should now
    read `response.thread.sessionId`.
    - Populated `thread.sessionId` when building live thread responses,
    replaying loaded threads, and returning stored-thread summaries so the
    field is present across start, resume, fork, list, read, rollback,
    metadata-update, unarchive, and `thread/started` paths. See
    [`load_thread_from_resume_source_or_send_internal`](https://github.com/openai/codex/blob/8fc9e9b4cf81b6f61d432e71f1eb266f6f104b63/codex-rs/app-server/src/request_processors/thread_processor.rs#L2824-L2918)
    and
    [`thread_from_stored_thread`](https://github.com/openai/codex/blob/8fc9e9b4cf81b6f61d432e71f1eb266f6f104b63/codex-rs/app-server/src/request_processors/thread_processor.rs#L3671-L3719).
    - Preserved the stored-thread fallback: if a thread has not been loaded
    into a live session tree yet, `thread.sessionId` falls back to
    `thread.id`; once the thread is live again, the field reports the active
    session tree root.
    - Regenerated the JSON/TypeScript schemas and updated the app-server
    README examples to show
    [`thread.sessionId`](https://github.com/openai/codex/blob/8fc9e9b4cf81b6f61d432e71f1eb266f6f104b63/codex-rs/app-server/README.md#L306-L310)
    on the thread object.
  • codex: route metadata updates through ThreadStore (#20576)
    - Route `thread/metadata/update` through
    `ThreadStore::update_thread_metadata`.
    - Add `LocalThreadStore` git metadata patch support for set, partial
    update, and clear semantics.
    - Add some unit tests for the new thread store code
    - Remove a lot of dead code/tests!
  • Add sorting/backwardsCursor to thread/list and new thread/turns/list api (#17305)
    To improve performance of UI loads from the app, add two main
    improvements:
    1. The `thread/list` api now gets a `sortDirection` request field and a
    `backwardsCursor` to the response, which lets you paginate forwards and
    backwards from a window. This lets you fetch the first few items to
    display immediately while you paginate to fill in history, then can
    paginate "backwards" on future loads to catch up with any changes since
    the last UI load without a full reload of the entire data set.
    2. Added a new `thread/turns/list` api which also has sortDirection and
    backwardsCursor for the same behavior as `thread/list`, allowing you the
    same small-fetch for immediate display followed by background fill-in
    and resync catchup.
  • [codex-backend] Make thread metadata updates tolerate pending backfill (#16877)
    ### Summary
    Fix `thread/metadata/update` so it can still patch stored thread
    metadata when the list/backfill-gated `get_state_db(...)` path is
    unavailable.
    
    What was happening:
    - The app logs showed `thread/metadata/update` failing with `sqlite
    state db unavailable for thread ...`.
    - This was not isolated to one bad thread. Once the failure started for
    a user, branch metadata updates failed 100% of the time for that user.
    - Reports were staggered across users, which points at local app-server
    / local SQLite state rather than one global server-side failure.
    - Turns could still start immediately after the metadata update failed,
    which suggests the thread itself was valid and the failure was in the
    metadata endpoint DB-handle path.
    
    The fix:
    - Keep using the loaded thread state DB and the normal
    `get_state_db(...)` fallback first.
    - If that still returns `None`, open `StateRuntime::init(...)` directly
    for this targeted metadata update path.
    - Log the direct state runtime init error if that final fallback also
    fails, so future reports have the real DB-open cause instead of only the
    generic unavailable error.
    - Add a regression test where the DB exists but backfill is not
    complete, and verify `thread/metadata/update` can still repair the
    stored rollout thread and patch `gitInfo`.
    
    Relevant context / suspect PRs:
    - #16434 changed state DB startup to run auto-vacuum / incremental
    vacuum. This is the most suspicious timing match for per-user, staggered
    local SQLite availability failures.
    - #16433 dropped the old log table from the state DB, also near the
    timing window.
    - #13280 introduced this endpoint and made it rely on SQLite for git
    metadata without resuming the thread.
    - #14859 and #14888 added/consumed persisted model + reasoning effort
    metadata. I checked these because of the new thread metadata fields, but
    this failure happens before the endpoint reaches thread-row update/load
    logic, so they seem less likely as the direct cause.
    
    ### Testing
    - `cargo fmt -- --config imports_granularity=Item` completed; local
    stable rustfmt emitted warnings that `imports_granularity` is unstable
    - `cargo test -p codex-app-server thread_metadata_update`
    - `git diff --check`
  • 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`.
  • 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.
  • 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>
  • Add thread metadata update endpoint to app server (#13280)
    ## Summary
    - add the v2 `thread/metadata/update` API, including
    protocol/schema/TypeScript exports and app-server docs
    - patch stored thread `gitInfo` in sqlite without resuming the thread,
    with validation plus support for explicit `null` clears
    - repair missing sqlite thread rows from rollout data before patching,
    and make those repairs safe by inserting only when absent and updating
    only git columns so newer metadata is not clobbered
    - keep sqlite authoritative for mutable thread git metadata by
    preserving existing sqlite git fields during reconcile/backfill and only
    using rollout `SessionMeta` git fields to fill gaps
    - add regression coverage for the endpoint, repair paths, concurrent
    sqlite writes, clearing git fields, and rollout/backfill reconciliation
    - fix the login server shutdown race so cancelling before the waiter
    starts still terminates `block_until_done()` correctly
    
    ## Testing
    - `cargo test -p codex-state
    apply_rollout_items_preserves_existing_git_branch_and_fills_missing_git_fields`
    - `cargo test -p codex-state
    update_thread_git_info_preserves_newer_non_git_metadata`
    - `cargo test -p codex-core
    backfill_sessions_preserves_existing_git_branch_and_fills_missing_git_fields`
    - `cargo test -p codex-app-server thread_metadata_update`
    - `cargo test`
    - currently fails in existing `codex-core` grep-files tests with
    `unsupported call: grep_files`:
        - `suite::grep_files::grep_files_tool_collects_matches`
        - `suite::grep_files::grep_files_tool_reports_empty_results`