Commit Graph

12 Commits

  • Move skills watcher to app-server (#21287)
    ## Why
    
    Skills update notifications are app-server API behavior, but the watcher
    lived in `codex-core` and surfaced through
    `EventMsg::SkillsUpdateAvailable`. Moving the watcher out keeps core
    focused on thread execution and lets app-server own both cache
    invalidation and the `skills/changed` notification.
    
    ## What changed
    
    - Added an app-server-owned skills watcher that watches local skill
    roots, clears the shared skills cache, and emits `skills/changed`
    directly.
    - Registers skill watches from the common app-server thread listener
    attach path, including direct starts, resumes, and app-server-observed
    child or forked threads.
    - Stores the `WatchRegistration` on `ThreadState`, so listener
    replacement, thread teardown, idle unload, and app-server shutdown
    deregister by dropping the RAII guard.
    - Removed `EventMsg::SkillsUpdateAvailable`, the core watcher, and the
    old core live-reload test.
    - Extended the app-server skills change test to verify a cached skills
    list is refreshed after a filesystem change without forcing reload.
    
    ## Validation
    
    - `cargo check -p codex-core -p codex-app-server -p codex-mcp-server -p
    codex-rollout -p codex-rollout-trace`
    - `cargo test -p codex-app-server
    skills_changed_notification_is_emitted_after_skill_change`
  • [codex-analytics] emit tool item events from item lifecycle (#17090)
    ## Why
    
    After the tool-item schemas are in place, analytics needs to emit them
    from the app-server item lifecycle rather than requiring bespoke
    tracking at each callsite. The reducer should also reuse the shared
    thread analytics context introduced below it in the stack so later event
    families do not repeat the same reducer joins or missing-state ladder.
    
    ## What changed
    
    - Tracks tool-item completion notifications and emits the matching tool
    analytics event when a terminal item arrives.
    - Derives event-specific payload details for command execution, file
    changes, MCP calls, dynamic tools, collaboration tools, web search, and
    image generation.
    - Denormalizes thread, app-server client, runtime, and subagent
    provenance metadata through the shared thread analytics context.
    - Adds reducer coverage for item lifecycle emission and subagent
    metadata inheritance.
    
    ## Duration semantics
    
    `duration_ms` is computed from the app-server item lifecycle timestamps:
    `completed_at_ms - started_at_ms`. That makes it the duration of the
    lifecycle Codex observed locally, not necessarily the upstream
    provider's full execution time.
    
    - Web search usually has a meaningful observed lifecycle because
    Responses can send `response.output_item.added` before
    `response.output_item.done`; in that case `started_at_ms` comes from the
    added event and `completed_at_ms` comes from the done event.
    - Image generation can be much less precise. In the current observed
    stream, image generation often arrives only as a completed
    `response.output_item.done`; when there is no earlier added event, Codex
    synthesizes the started item immediately before completion, so
    `duration_ms` can be `0` even though upstream image generation took
    longer.
    - Standalone web search and standalone image generation work is expected
    to land after this stack. Those paths may introduce more direct
    lifecycle events or timing points, so the current
    web-search/image-generation duration semantics should be treated as the
    best available item-lifecycle approximation, not the final latency
    contract for those tool families.
    - `execution_duration_ms` is populated only where the completed item
    already carries a native execution duration; otherwise it remains `null`
    while `duration_ms` still reflects the local lifecycle interval.
    
    ## Currently placeholder / partial fields
    
    Some fields are included in the schema for the intended steady-state
    contract, but this PR does not yet populate them from real
    approval/review state:
    
    - `review_count`, `guardian_review_count`, and `user_review_count`
    currently default to `0`.
    - `final_approval_outcome` currently defaults to `unknown`.
    - `requested_additional_permissions` and `requested_network_access`
    currently default to `false`.
    
    ## Verification
    
    - `cargo test -p codex-analytics`
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/17090).
    * #18748
    * #18747
    * __->__ #17090
    * #17089
    * #20514
  • 2- Use string service tiers in session protocol (#20971)
    ## Summary
    - break service tier session/op/app-server protocol fields from the
    closed enum to string tier ids
    - send the service tier string directly through model requests, prewarm,
    compaction, memories, and TUI/app-server turn starts
    - regenerate app-server protocol JSON/TypeScript schemas, removing the
    standalone ServiceTier TS enum
    
    ## Verification
    - just fmt
    - cargo check -p codex-core -p codex-app-server -p codex-tui
    - just write-app-server-schema
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • 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-analytics] rework thread_source for thread analytics (#20949)
    ## Summary
    - make `thread_source` an explicit optional thread-level field on
    `thread/start`, `thread/fork`, and returned thread payloads
    - persist `thread_source` in rollout/session metadata so resumed live
    threads retain the original value
    - replace the old best-effort `session_source` -> `thread_source`
    mapping with an explicit caller-supplied analytics classification
    
    ## Why
    Before this change, analytics `thread_source` was populated by a
    best-effort mapping from `session_source`. `session_source` describes
    the runtime/client surface, not the actual thread-level origin, so that
    projection was not accurate enough to distinguish cases such as `user`,
    `subagent`, `memory_consolidation`, and future thread origins reliably.
    
    Making `thread_source` explicit keeps one thread-level analytics field
    while letting callers provide the real classification directly instead
    of recovering it indirectly from `session_source`.
    
    ## Impact
    For new analytics events, `thread_source` now reflects the explicit
    thread-level classification supplied by the caller rather than an
    inferred value derived from `session_source`. Existing protocol fields
    remain optional; callers that omit `threadSource` now produce `null`
    instead of a best-effort inferred value.
    
    ## Validation
    - `just write-app-server-schema`
    - `cargo test -p codex-analytics -p codex-core -p
    codex-app-server-protocol --no-run`
    - `cargo test -p codex-app-server-protocol
    generated_ts_optional_nullable_fields_only_in_params`
    - `cargo test -p codex-analytics
    thread_initialized_event_serializes_expected_shape`
    - `cargo test -p codex-core
    resume_stopped_thread_from_rollout_preserves_thread_source`
  • Auto-deny MCP elicitations for Xcode 26.4 clients (#21113)
    ## Summary
    
    Xcode 26.4 was built against app-server behavior from before MCP
    elicitation requests became client-visible in CLI 0.120.0 via #17043.
    That client line does not expect the new events/messages, so this PR
    restores the old behavior for exactly that client/version combination.
    
    The compatibility handling stays in the app-server layer: when the
    initialized client is `Xcode` and its version starts with `26.4`, the
    app server marks the live Codex thread so MCP elicitations are
    auto-denied. The flag is applied on thread start/resume/fork/turn
    attachment, carried through `Codex`/`CodexThread`, and stored on
    `McpConnectionManager` so refreshed MCP managers preserve the behavior.
    
    ## Notes
    
    This is intentionally narrow and includes a TODO to remove the
    compatibility path once Xcode 26.4 ages out.
  • add turn items view to app-server turns (#21063)
    ## Why
    
    `Turn.items` currently overloads an empty array to mean either that no
    items exist or that the server intentionally did not load them for this
    response. That ambiguity blocks future lazy-loading work where clients
    need to distinguish unloaded, summary, and fully hydrated turn payloads.
    
    ## What changed
    
    - add a new `TurnItemsView` enum with `notLoaded`, `summary`, and `full`
    variants
    - add required `itemsView` metadata to app-server `Turn` payloads
    - mark reconstructed persisted history as `full` and live shell-style
    turn payloads as `notLoaded`
    - keep current `thread/turns/list` behavior unchanged and document that
    it still returns `full` turns today
    - regenerate the JSON and TypeScript protocol fixtures
    
    ## Verification
    
    - `just write-app-server-schema`
    - `cargo test -p codex-app-server-protocol`
    - `cargo test -p codex-app-server thread_read_can_include_turns`
    - `cargo test -p codex-app-server
    thread_turns_list_can_page_backward_and_forward`
    - `cargo test -p codex-app-server
    thread_resume_rejects_history_when_thread_is_running`
    - `just fix -p codex-app-server-protocol`
    - `just fix -p codex-app-server`
    - `just fmt`
  • [codex] Use shared app-server JSON-RPC error helpers (#21221)
    ## Why
    
    App-server had repeated hand-built JSON-RPC error objects for standard
    error shapes. Using the shared helpers keeps the common
    `invalid_request`, `invalid_params`, and `internal_error` construction
    in one place and reduces the chance of new call sites drifting from the
    common error payload shape.
    
    ## What changed
    
    - Replaced manual standard JSON-RPC error object creation with
    `internal_error(...)`, `invalid_request(...)`, and `invalid_params(...)`
    across app-server request processors and runtime paths.
    - Removed local duplicate helper definitions from search and review
    request handling.
    - Preserved existing structured `data` payloads by creating the shared
    helper error first and then attaching the existing metadata.
    - Left custom non-standard errors and raw error-code assertions intact.
    
    ## Validation
    
    - `cargo test -p codex-app-server`
  • codex: migrate (more) app-server thread history reads to ThreadStore (#20575)
    Migrate token usage replay, rollback responses, and detached review
    setup (a special case of forking) to be served from ThreadStore reads
    rather direct rollout files.
    
    - replay restored token usage from already-loaded `RolloutItem` history
    instead of reopening `Thread.path`
    - rebuild rollback responses from loaded `ThreadStore` snapshots and
    history
    - start detached reviews from store-backed parent history and stored
    review-thread metadata
    - remove obsolete app-server rollout-summary helper code that became
    dead after the store-backed migration
    - preserve response/notification ordering for resume, fork, rollback,
    and detached review flows
    - add integration test coverage for the affected paths
  • state: pass state db handles through consumers (#20561)
    ## Why
    
    SQLite state was still being opened from consumer paths, including lazy
    `OnceCell`-backed thread-store call sites. That let one process
    construct multiple state DB connections for the same Codex home, which
    makes SQLite lock contention and `database is locked` failures much
    easier to hit.
    
    State DB lifetime should be chosen by main-like entrypoints and tests,
    then passed through explicitly. Consumers should use the supplied
    `Option<StateDbHandle>` or `StateDbHandle` and keep their existing
    filesystem fallback or error behavior when no handle is available.
    
    The startup path also needs to keep the rollout crate in charge of
    SQLite state initialization. Opening `codex_state::StateRuntime`
    directly bypasses rollout metadata backfill, so entrypoints should
    initialize through `codex_rollout::state_db` and receive a handle only
    after required rollout backfills have completed.
    
    ## What Changed
    
    - Initialize the state DB in main-like entrypoints for CLI, TUI,
    app-server, exec, MCP server, and the thread-manager sample.
    - Pass `Option<StateDbHandle>` through `ThreadManager`,
    `LocalThreadStore`, app-server processors, TUI app wiring, rollout
    listing/recording, personality migration, shell snapshot cleanup,
    session-name lookup, and memory/device-key consumers.
    - Remove the lazy local state DB wrapper from the thread store so
    non-test consumers use only the supplied handle or their existing
    fallback path.
    - Make `codex_rollout::state_db::init` the local state startup path: it
    opens/migrates SQLite, runs rollout metadata backfill when needed, waits
    for concurrent backfill workers up to a bounded timeout, verifies
    completion, and then returns the initialized handle.
    - Keep optional/non-owning SQLite helpers, such as remote TUI local
    reads, as open-only paths that do not run startup backfill.
    - Switch app-server startup from direct
    `codex_state::StateRuntime::init` to the rollout state initializer so
    app-server cannot skip rollout backfill.
    - Collapse split rollout lookup/list APIs so callers use the normal
    methods with an optional state handle instead of `_with_state_db`
    variants.
    - Restore `getConversationSummary(ThreadId)` to delegate through
    `ThreadStore::read_thread` instead of a LocalThreadStore-specific
    rollout path special case.
    - Keep DB-backed rollout path lookup keyed on the DB row and file
    existence, without imposing the filesystem filename convention on
    existing DB rows.
    - Verify readable DB-backed rollout paths against `session_meta.id`
    before returning them, so a stale SQLite row that points at another
    thread's JSONL falls back to filesystem search and read-repairs the DB
    row.
    - Keep `debug prompt-input` filesystem-only so a one-off debug command
    does not initialize or backfill SQLite state just to print prompt input.
    - Keep goal-session test Codex homes alive only in the goal-specific
    helper, rather than leaking tempdirs from the shared session test
    helper.
    - Update tests and call sites to pass explicit state handles where DB
    behavior is expected and explicit `None` where filesystem-only behavior
    is intended.
    
    ## Validation
    
    - `CARGO_TARGET_DIR=/tmp/codex-target-state-db cargo check -p
    codex-rollout -p codex-thread-store -p codex-app-server -p codex-core -p
    codex-tui -p codex-exec -p codex-cli --tests`
    - `CARGO_TARGET_DIR=/tmp/codex-target-state-db cargo test -p
    codex-rollout state_db_`
    - `CARGO_TARGET_DIR=/tmp/codex-target-state-db cargo test -p
    codex-rollout find_thread_path`
    - `CARGO_TARGET_DIR=/tmp/codex-target-state-db cargo test -p
    codex-rollout find_thread_path -- --nocapture`
    - `CARGO_TARGET_DIR=/tmp/codex-target-state-db cargo test -p
    codex-rollout try_init_ -- --nocapture`
    - `CARGO_TARGET_DIR=/tmp/codex-target-state-db cargo test -p
    codex-rollout`
    - `CARGO_TARGET_DIR=/tmp/codex-target-state-db cargo clippy -p
    codex-rollout --lib -- -D warnings`
    - `CARGO_TARGET_DIR=/tmp/codex-target-state-db cargo test -p
    codex-thread-store
    read_thread_falls_back_when_sqlite_path_points_to_another_thread --
    --nocapture`
    - `CARGO_TARGET_DIR=/tmp/codex-target-state-db cargo test -p
    codex-thread-store`
    - `CARGO_TARGET_DIR=/tmp/codex-target-state-db cargo test -p codex-core
    shell_snapshot`
    - `CARGO_TARGET_DIR=/tmp/codex-target-state-db cargo test -p codex-core
    --test all personality_migration`
    - `CARGO_TARGET_DIR=/tmp/codex-target-state-db cargo test -p codex-core
    --test all rollout_list_find`
    - `RUST_MIN_STACK=8388608 CODEX_SKIP_VENDORED_BWRAP=1
    CARGO_TARGET_DIR=/tmp/codex-target-state-db cargo test -p codex-core
    --test all rollout_list_find::find_prefers_sqlite_path_by_id --
    --nocapture`
    - `RUST_MIN_STACK=8388608 CODEX_SKIP_VENDORED_BWRAP=1
    CARGO_TARGET_DIR=/tmp/codex-target-state-db cargo test -p codex-core
    --test all rollout_list_find -- --nocapture`
    - `CARGO_TARGET_DIR=/tmp/codex-target-state-db cargo test -p codex-core
    interrupt_accounts_active_goal_before_pausing`
    - `CARGO_TARGET_DIR=/tmp/codex-target-state-db cargo test -p
    codex-app-server get_auth_status -- --test-threads=1`
    - `CODEX_SKIP_VENDORED_BWRAP=1
    CARGO_TARGET_DIR=/tmp/codex-target-state-db cargo test -p
    codex-app-server --lib`
    - `CODEX_SKIP_VENDORED_BWRAP=1
    CARGO_TARGET_DIR=/tmp/codex-target-state-db cargo check -p codex-rollout
    -p codex-app-server --tests`
    - `CARGO_TARGET_DIR=/tmp/codex-target-state-db just fix -p codex-rollout
    -p codex-thread-store -p codex-core -p codex-app-server -p codex-tui -p
    codex-exec -p codex-cli`
    - `CODEX_SKIP_VENDORED_BWRAP=1
    CARGO_TARGET_DIR=/tmp/codex-target-state-db just fix -p codex-rollout -p
    codex-app-server`
    - `CARGO_TARGET_DIR=/tmp/codex-target-state-db just fix -p
    codex-rollout`
    - `CODEX_SKIP_VENDORED_BWRAP=1
    CARGO_TARGET_DIR=/tmp/codex-target-state-db just fix -p codex-core`
    - `just argument-comment-lint -p codex-core`
    - `just argument-comment-lint -p codex-rollout`
    
    Focused coverage added in `codex-rollout`:
    
    - `recorder::tests::state_db_init_backfills_before_returning` verifies
    the rollout metadata row exists before startup init returns.
    - `state_db::tests::try_init_waits_for_concurrent_startup_backfill`
    verifies startup waits for another worker to finish backfill instead of
    disabling the handle for the process.
    -
    `state_db::tests::try_init_times_out_waiting_for_stuck_startup_backfill`
    verifies startup does not hang indefinitely on a stuck backfill lease.
    -
    `tests::find_thread_path_accepts_existing_state_db_path_without_canonical_filename`
    verifies DB-backed lookup accepts valid existing rollout paths even when
    the filename does not include the thread UUID.
    -
    `tests::find_thread_path_falls_back_when_db_path_points_to_another_thread`
    verifies DB-backed lookup ignores a stale row whose existing path
    belongs to another thread and read-repairs the row after filesystem
    fallback.
    
    Focused coverage updated in `codex-core`:
    
    - `rollout_list_find::find_prefers_sqlite_path_by_id` now uses a
    DB-preferred rollout file with matching `session_meta.id`, so it still
    verifies that valid SQLite paths win without depending on stale/empty
    rollout contents.
    
    `cargo test -p codex-app-server thread_list_respects_search_term_filter
    -- --test-threads=1 --nocapture` was attempted locally but timed out
    waiting for the app-server test harness `initialize` response before
    reaching the changed thread-list code path.
    
    `bazel test //codex-rs/thread-store:thread-store-unit-tests
    --test_output=errors` was attempted locally after the thread-store fix,
    but this container failed before target analysis while fetching `v8+`
    through BuildBuddy/direct GitHub. The equivalent local crate coverage,
    including `cargo test -p codex-thread-store`, passes.
    
    A plain local `cargo check -p codex-rollout -p codex-app-server --tests`
    also requires system `libcap.pc` for `codex-linux-sandbox`; the
    follow-up app-server check above used `CODEX_SKIP_VENDORED_BWRAP=1` in
    this container.
  • [codex] Split app-server request processors (#20940)
    ## Why
    
    The app-server request path had grown around a large
    `CodexMessageProcessor` plus separate API wrapper/helper modules. That
    made the dependency graph hard to see and forced unrelated request
    families to share broad processor state.
    
    This PR makes the split mechanical and command-prefix oriented so
    request families own only the dependencies they use.
    
    ## What changed
    
    - Replaced `CodexMessageProcessor` with command-prefix request
    processors under `app-server/src/request_processors/`.
    - Removed the old config, device-key, external-agent-config, and fs API
    wrapper files by moving their API handling into processors.
    - Split apps, plugins, marketplace, catalog, account, MCP, command exec,
    fs, git, feedback, thread, turn, thread goals, and Windows sandbox
    handling into dedicated processors.
    - Kept shared lifecycle, summary conversion, token usage replay, and
    shared error mapping only where multiple processors use them; single-use
    helpers were inlined into their owning processor.
    - Removed the fallback processor path and moved processor tests to
    `_tests` files.
    
    ## Validation
    
    - `cargo test -p codex-app-server`
    - `cargo check -p codex-app-server`
    - `just fix -p codex-app-server`