Commit Graph

17 Commits

  • Avoid noisy OTEL diagnostics in codex exec (#21107)
    `codex exec` should not print OpenTelemetry exporter self-diagnostics to
    stderr by default. Suppress the SDK and OTLP exporter targets unless
    callers
    explicitly opt in with `RUST_LOG`.
    
    Also stop defaulting the trace exporter to the log exporter, since OTLP
    HTTP
    endpoints are signal-specific and a logs endpoint is not valid for
    spans.
    
    Co-authored-by: Codex <noreply@openai.com>
    
    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.
  • 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>
  • [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`
  • 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`
  • permissions: remove legacy read-only access modes (#19449)
    ## Why
    
    `ReadOnlyAccess` was a transitional legacy shape on `SandboxPolicy`:
    `FullAccess` meant the historical read-only/workspace-write modes could
    read the full filesystem, while `Restricted` tried to carry partial
    readable roots. The partial-read model now belongs in
    `FileSystemSandboxPolicy` and `PermissionProfile`, so keeping it on
    `SandboxPolicy` makes every legacy projection reintroduce lossy
    read-root bookkeeping and creates unnecessary noise in the rest of the
    permissions migration.
    
    This PR makes the legacy policy model narrower and explicit:
    `SandboxPolicy::ReadOnly` and `SandboxPolicy::WorkspaceWrite` represent
    the old full-read sandbox modes only. Split readable roots, deny-read
    globs, and platform-default/minimal read behavior stay in the runtime
    permissions model.
    
    ## What changed
    
    - Removes `ReadOnlyAccess` from
    `codex_protocol::protocol::SandboxPolicy`, including the generated
    `access` and `readOnlyAccess` API fields.
    - Updates legacy policy/profile conversions so restricted filesystem
    reads are represented only by `FileSystemSandboxPolicy` /
    `PermissionProfile` entries.
    - Keeps app-server v2 compatible with legacy `fullAccess` read-access
    payloads by accepting and ignoring that no-op shape, while rejecting
    legacy `restricted` read-access payloads instead of silently widening
    them to full-read legacy policies.
    - Carries Windows sandbox platform-default read behavior with an
    explicit override flag instead of depending on
    `ReadOnlyAccess::Restricted`.
    - Refreshes generated app-server schema/types and updates tests/docs for
    the simplified legacy policy shape.
    
    ## Verification
    
    - `cargo check -p codex-app-server-protocol --tests`
    - `cargo check -p codex-windows-sandbox --tests`
    - `cargo test -p codex-app-server-protocol sandbox_policy_`
    
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/19449).
    * #19395
    * #19394
    * #19393
    * #19392
    * #19391
    * __->__ #19449
  • permissions: make legacy profile conversion cwd-free (#19414)
    ## Why
    
    The profile conversion path still required a `cwd` even when it was only
    translating a legacy `SandboxPolicy` into a `PermissionProfile`. That
    made profile producers invent an ambient `cwd`, which is exactly the
    anchoring we are trying to remove from permission-profile data. A legacy
    workspace-write policy can be represented symbolically instead: `:cwd =
    write` plus read-only `:project_roots` metadata subpaths.
    
    This PR creates that cwd-free base so the rest of the stack can stop
    threading cwd through profile construction. Callers that actually need a
    concrete runtime filesystem policy for a specific cwd still have an
    explicitly named cwd-bound conversion.
    
    ## What Changed
    
    - `PermissionProfile::from_legacy_sandbox_policy` now takes only
    `&SandboxPolicy`.
    - `FileSystemSandboxPolicy::from_legacy_sandbox_policy` is now the
    symbolic, cwd-free projection for profiles.
    - The old concrete projection is retained as
    `FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd` for
    runtime/boundary code that must materialize legacy cwd behavior.
    - Workspace-write profiles preserve `CurrentWorkingDirectory` and
    `ProjectRoots` special entries instead of materializing cwd into
    absolute paths.
    
    ## Verification
    
    - `cargo check -p codex-protocol -p codex-core -p
    codex-app-server-protocol -p codex-app-server -p codex-exec -p
    codex-exec-server -p codex-tui -p codex-sandboxing -p
    codex-linux-sandbox -p codex-analytics --tests`
    - `just fix -p codex-protocol -p codex-core -p codex-app-server-protocol
    -p codex-app-server -p codex-exec -p codex-exec-server -p codex-tui -p
    codex-sandboxing -p codex-linux-sandbox -p codex-analytics`
    
    
    
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/19414).
    * #19395
    * #19394
    * #19393
    * #19392
    * #19391
    * __->__ #19414
  • Rename approvals reviewer variant to auto-review (#19056)
    ## Why
    
    `approvals_reviewer` now uses `auto_review` as the canonical config/API
    value after #18504, but the Rust enum variant and nearby helper/test
    names still used `GuardianSubagent` / guardian approval wording. That
    made follow-up code and reviews confusing even though the external value
    had already moved to Auto-review.
    
    ## What changed
    
    - Renamed `ApprovalsReviewer::GuardianSubagent` to
    `ApprovalsReviewer::AutoReview`.
    - Updated protocol, app-server, config, core, TUI, exec, and analytics
    test callsites.
    - Renamed nearby helper/test names from guardian approval wording to
    Auto-review wording where they refer to the approvals reviewer mode.
    - Preserved wire compatibility:
      - `auto_review` remains the canonical serialized value.
      - `guardian_subagent` remains accepted as a legacy alias.
    
    This intentionally does not rename the `[features].guardian_approval`
    key, `Feature::GuardianApproval`, `core/src/guardian`, analytics event
    names, or app-server Guardian review event types.
    
    ## Verification
    
    - `cargo test -p codex-protocol
    approvals_reviewer_serializes_auto_review_and_accepts_legacy_guardian_subagent`
    - `cargo test -p codex-app-server-protocol
    approvals_reviewer_serializes_auto_review_and_accepts_legacy_guardian_subagent`
    - `cargo test -p codex-config approvals_reviewer`
    - `cargo test -p codex-tui update_feature_flags`
    - `cargo test -p codex-core permissions_instructions`
    - `cargo test -p codex-tui permissions_selection`
  • clients: send permission profiles to app-server (#18280)
    ## Why
    
    After app-server can accept `PermissionProfile`, first-party clients
    should stop preferring legacy sandbox fields when canonical permission
    information is available. This keeps the migration moving without
    removing legacy compatibility yet.
    
    The client side still has mixed surfaces during the stack: embedded
    thread start/resume/fork and exec initial turns can derive a profile
    directly from local config, while TUI remote sessions and some
    turn-start paths only have a legacy/server-context-safe sandbox
    projection. Those paths keep sending legacy sandbox fields rather than
    synthesizing or sending lossy/local-only profiles.
    
    ## What changed
    
    - Sends `permissionProfile` from exec and embedded TUI thread
    start/resume/fork requests when config has a representable profile.
    - Keeps legacy sandbox fallback for external sandbox policies, TUI
    remote thread lifecycle requests, and TUI turn-start requests that do
    not yet carry the active profile.
    - Sends the actual config-derived `permissionProfile` for exec initial
    turns instead of rebuilding one from the legacy sandbox projection.
    - Stores response `permissionProfile` as optional in TUI session state
    so external sandbox responses and compatibility payloads preserve
    `null`.
    - Updates tests for request construction and response mapping.
    
    ## Verification
    
    - `cargo check --tests -p codex-tui -p codex-exec`
    - `cargo test -p codex-tui app_server_session -- --nocapture`
    - `cargo test -p codex-exec thread_start_params -- --nocapture`
    - `cargo test -p codex-tui
    app_server_session::tests::thread_lifecycle_params -- --nocapture`
    - `just fix -p codex-tui -p codex-exec`
    - `just fix -p codex-tui`
    
    
    
    
    
    
    
    
    
    
    
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/18280).
    * #18288
    * #18287
    * #18286
    * #18285
    * #18284
    * #18283
    * #18282
    * #18281
    * __->__ #18280
  • app-server: expose thread permission profiles (#18278)
    ## Why
    
    The `PermissionProfile` migration needs app-server clients to see the
    same constrained permission model that core is using at runtime. Before
    this PR, thread lifecycle responses only exposed the legacy
    `SandboxPolicy` shape, so clients still had to infer active permissions
    from sandbox fields. That makes downstream resume, fork, and override
    flows harder to make `PermissionProfile`-first.
    
    External sandbox policies are intentionally excluded from this canonical
    view. External enforcement cannot be round-tripped as a
    `PermissionProfile`, and exposing a lossy root-write profile would let
    clients accidentally change sandbox semantics if they echo the profile
    back later.
    
    ## What changed
    
    - Adds the app-server v2 `PermissionProfile` wire shape, including
    filesystem permissions and glob scan depth metadata.
    - Adds `PermissionProfileNetworkPermissions` so the profile response
    does not expose active network state through the older
    additional-permissions naming.
    - Returns `permissionProfile` from thread start, resume, and fork
    responses when the active sandbox can be represented as a
    `PermissionProfile`.
    - Keeps legacy `sandbox` in those responses for compatibility and
    documents `permissionProfile` as canonical when present.
    - Makes lifecycle `permissionProfile` nullable and returns `null` for
    `ExternalSandbox` to avoid exposing a lossy profile.
    - Regenerates the app-server JSON schema and TypeScript fixtures.
    
    ## Verification
    
    - `cargo test -p codex-app-server-protocol`
    - `cargo test -p codex-app-server
    thread_response_permission_profile_omits_external_sandbox --
    --nocapture`
    - `cargo check --tests -p codex-analytics -p codex-exec -p codex-tui`
    - `just fix -p codex-app-server-protocol -p codex-app-server -p
    codex-analytics -p codex-exec -p codex-tui`
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/18278).
    * #18279
    * __->__ #18278
  • Spread AbsolutePathBuf (#17792)
    Mechanical change to promote absolute paths through code.
  • Expose instruction sources (AGENTS.md) via app server (#17506)
    Addresses #17498
    
    Problem: The TUI derived /status instruction source paths from the local
    client environment, which could show stale <none> output or incorrect
    paths when connected to a remote app server.
    
    Solution: Add an app-server v2 instructionSources snapshot to thread
    start/resume/fork responses, default it to an empty list when older
    servers omit it, and render TUI /status from that server-provided
    session data.
    
    Additional context: The app-server field is intentionally named
    instructionSources rather than AGENTS.md-specific terminology because
    the loaded instruction sources can include global instructions, project
    AGENTS.md files, AGENTS.override.md, user-defined instruction files, and
    future dynamic sources.
  • [codex-analytics] add protocol-native turn timestamps (#16638)
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/16638).
    * #16870
    * #16706
    * #16659
    * #16641
    * #16640
    * __->__ #16638
  • [regression] Fix ephemeral turn backfill in exec (#16795)
    Addresses #16781
    
    Problem: `codex exec --ephemeral` backfilled empty `turn/completed`
    items with `thread/read(includeTurns=true)`, which app-server rejects
    for ephemeral threads.
    
    This is a regression introduced in the recent conversion of "exec" to
    use app server rather than call the core directly.
    
    Solution: Skip turn-item backfill for ephemeral exec threads while
    preserving the existing recovery path for non-ephemeral sessions.
  • Fix fork source display in /status (expose forked_from_id in app server) (#16596)
    Addresses #16560
    
    Problem: `/status` stopped showing the source thread id in forked TUI
    sessions after the app-server migration.
    
    Solution: Carry fork source ids through app-server v2 thread data and
    the TUI session adapter, and update TUI fixtures so `/status` matches
    the old TUI behavior.
  • chore: move codex-exec unit tests into sibling files (#16581)
    ## Why
    
    `codex-rs/exec/src/lib.rs` already keeps unit tests in a sibling
    `lib_tests.rs` module so the implementation stays top-heavy and easier
    to read. This applies that same layout to the rest of
    `codex-rs/exec/src` so each production file keeps its entry points and
    helpers ahead of test code.
    
    ## What
    
    - Move inline unit tests out of `cli.rs`, `main.rs`,
    `event_processor_with_human_output.rs`, and
    `event_processor_with_jsonl_output.rs` into sibling `*_tests.rs` files.
    - Keep test modules wired through `#[cfg(test)]` plus `#[path = "..."]
    mod tests;`, matching the `lib.rs` pattern.
    - Preserve the existing test coverage and assertions while making this a
    source-layout-only refactor.
    
    ## Verification
    
    - `cargo test -p codex-exec`