Commit Graph

13 Commits

  • feat: add session_id (#20437)
    ## Summary
    
    Related to
    https://openai.slack.com/archives/C095U48JNL9/p1777537279707449
    TLDR:
    We update the meaning of session ids and thread ids:
    * thread_id stays as now
    * session_id become a shared id between every thread under a /root
    thread (i.e. every sub-agent share the same session id)
    
    This PR introduces an explicit `SessionId` and threads it through the
    protocol/client boundary so `session_id` and `thread_id` can diverge
    when they need to, while preserving compatibility for older serialized
    `session_configured` events.
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • app-server: align dynamic tool identifiers with Responses API (#20724)
    ## Why
    
    Codex currently accepts dynamic tool names and namespaces that the
    upstream Responses function-tool path does not actually support. In
    practice, that means app-server can register a dynamic tool successfully
    and only discover later that the LLM-facing tool contract will reject or
    mishandle it.
    
    This PR tightens the app-server-side dynamic tool contract to match the
    Responses API before we stack dynamic tool hook support on top of it.
    
    ## What changed
    
    - validate dynamic tool `name` against the Responses function-tool
    identifier contract: `^[a-zA-Z0-9_-]+$`, length `1..128`
    - validate dynamic tool `namespace` the same way, with the Responses
    namespace length limit `1..64`
    - reject namespaces that collide with the always-reserved Responses
    runtime namespaces such as `functions`, `multi_tool_use`, `file_search`,
    `web`, `browser`, `image_gen`, `computer`, `container`, `terminal`,
    `python`, `python_user_visible`, `api_tool`, `tool_search`, and
    `submodel_delegator`
    - escape invalid identifiers in error messages so control characters do
    not spill raw into logs or client-visible error text
    - document the tightened dynamic tool identifier contract in
    `codex-rs/app-server/README.md`
    - add both unit coverage for the validator and an app-server integration
    test that rejects a `thread/start` request with Responses-incompatible
    dynamic tool identifiers
    
    ## Verification
    
    - `cargo test -p codex-app-server validate_dynamic_tools_`
    - `cargo test -p codex-app-server --test all
    thread_start_rejects_dynamic_tools_not_supported_by_responses`
  • [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`
  • [codex] Move thread naming to app server (#21260)
    ## Why
    
    Thread names are app-server metadata now, backed by the thread store and
    sqlite state database. Keeping a core `SetThreadName` op plus a rollout
    `thread_name_updated` event made rename persistence live in the wrong
    layer and required historical replay support for an event that new
    app-server flows should not write.
    
    ## What changed
    
    - Removed `Op::SetThreadName` and `EventMsg::ThreadNameUpdated` from the
    core protocol and deleted the core handler path that appended rename
    events to rollouts.
    - Updated app-server `thread/name/set` so both loaded and unloaded
    threads write through thread-store metadata and app-server emits
    `thread/name/updated` notifications.
    - Updated local thread-store name metadata updates to write sqlite title
    metadata and the legacy thread-name index without appending rollout
    events.
    - Removed state extraction and rollout handling for the deleted
    thread-name event.
    
    ## Validation
    
    - `cargo test -p codex-app-server thread_name_updated_broadcasts`
    - `cargo test -p codex-app-server
    thread_name_set_is_reflected_in_read_list_and_resume`
    - `cargo test -p codex-thread-store
    update_thread_metadata_sets_name_on_active_rollout_and_indexes_name`
    - `cargo test -p codex-state`
    - `cargo check -p codex-mcp-server -p codex-rollout-trace`
    - `just fix -p codex-app-server -p codex-thread-store -p codex-state -p
    codex-mcp-server -p codex-rollout-trace`
    
    ## Docs
    
    No external documentation update is expected for this internal ownership
    change.
  • Inject state DB, agent graph store (#20689)
    ## Why
    
    We want the agent graph store to be passed down the stack as a real
    dependency, the same way we already treat the thread store.
    
    This will let us inject the agent graph store as a real dependency and
    support implementations other than the local SQLite-backed one. Right
    now most code instantiates a state DB and an agent graph store
    just-in-time. Ideally, we would not depend on the state DB directly but
    only read through the higher-level interfaces.
    
    This change makes the dependency boundaries explicit and moves state DB
    initialization to process bootstrap instead of hiding it inside local
    store implementations.
    
    ## What changed
    
    - `ThreadManager` now requires a `StateDbHandle` and an
    `AgentGraphStore` at construction time instead of treating them as
    optional internals.
    - The local store constructors no longer lazily initialize SQLite.
    Callers now initialize the state DB once per process and use that shared
    handle to build:
      - `LocalThreadStore`
      - `LocalAgentGraphStore`
    - App bootstraps (`app-server`, `mcp-server`, `prompt_debug`, and the
    thread-manager sample) now initialize the state DB up front and inject
    the resulting handle down the stack.
    - `app-server` now consistently uses its process-scoped state DB handle
    instead of reopening SQLite or trying to recover it from loaded threads.
    - Device-key storage now reuses the shared state DB handle instead of
    maintaining its own lazy opener.
    - The thread archive / descendant traversal paths now use the injected
    `AgentGraphStore` instead of reaching through local
    thread-store-specific state.
    
    ## Verification
    
    - `cargo check -p codex-core -p codex-thread-store -p codex-app-server
    -p codex-mcp-server -p codex-thread-manager-sample --tests`
    - `cargo test -p codex-thread-store`
    - `cargo test -p codex-core
    thread_manager_accepts_separate_agent_graph_store_and_thread_store --
    --nocapture`
    - `cargo test -p codex-app-server
    thread_archive_archives_spawned_descendants -- --nocapture`
  • 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.
  • [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`
  • app-server: ignore persist_extended_history param (#21225)
    ## Why
    
    Taking a step to removing the `persistExtendedHistory` field. It's not
    scalable to be persisting so much data in the rollout file and returning
    it in the thread history.
    
    When a client explicitly sends `true`, the server now tells that client
    the parameter is deprecated and ignored so the caller has a clear
    migration signal via the `deprecationNotice` notification.
    
    ## What changed
    
    - Keep the `persist_extended_history` / `persistExtendedHistory` field
    in the v2 protocol for compatibility, but document it as deprecated and
    ignored.
    - Ignore the parameter in app-server `thread/start`, `thread/resume`,
    and `thread/fork`; those paths always use limited history persistence
    now.
    - Stop treating `persistExtendedHistory` as a running-thread resume
    override mismatch.
    - Emit a connection-scoped `deprecationNotice` when a request explicitly
    sets `persist_extended_history: true`.
    
    ## Verification
    
    - Added `thread_start_deprecates_persist_extended_history_true` to cover
    the deprecation notice.
    - `cargo test -p codex-app-server`
    - `cargo test -p codex-app-server-protocol`
  • 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
  • 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!
  • 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.
  • feat(app-server): always return limited thread history (#20682)
    ## Why
    
    Whenever we return a thread's history (turns and items) over app-server,
    always return the limited form as specified by the rollout policy
    `EventPersistenceMode::Limited`, even if the thread was previously
    started with `EventPersistenceMode::Extended`.
    
    We're finding it is quite unscalable to be returning the extended
    history, so let's apply the same filtering logic of the rollout policy
    when we load and return the thread's history.
    
    ## What Changed
    
    - Reuse the rollout persistence policy when reconstructing app-server
    `ThreadItem` history so only `EventPersistenceMode::Limited` rollout
    items are replayed into API turns.
    - Route `thread/read`, `thread/resume`, `thread/fork`,
    `thread/turns/list`, and rollback responses through the same filtered
    app-server history projection.
    - Keep live active turns intact when composing a response for a
    currently running thread.
    - Update command execution coverage so persisted extended command events
    are excluded from returned history for `thread/read`, `thread/fork`, and
    `thread/turns/list`.
    
    ## Test Plan
    
    - `cargo test -p codex-app-server limited`
    - `cargo test -p codex-app-server thread_shell_command`
    - `cargo test -p codex-app-server thread_read`
    - `cargo test -p codex-app-server thread_rollback`
    - `cargo test -p codex-app-server thread_fork`
    - `cargo test -p codex-app-server-protocol`
  • [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`