Commit Graph

16 Commits

  • 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`
  • [codex-analytics] ingest server requests and responses (#17088)
    ## Why
    
    Codex analytics needs a typed seam for app-server-originated
    request/response traffic so future tool-approval analytics can consume
    those facts without adding bespoke callsite tracking each time. Server
    responses arrive as JSON-RPC `id + result` payloads, so analytics has to
    reconstruct the matching typed response from the original typed request
    while that request context still exists in app-server.
    
    This also puts analytics on the app-server outbound path, which needs to
    avoid keeping the runtime alive during shutdown. The final ownership fix
    keeps the normal strong auth-manager retention in analytics and makes
    the external-auth refresh bridge hold a weak back-reference to
    `OutgoingMessageSender`, breaking the runtime cycle at the bridge
    boundary instead of exposing retention policy through the analytics
    client API.
    
    ## What changed
    
    - Adds typed `ServerRequest` and `ServerResponse` analytics facts, plus
    `AnalyticsEventsClient::track_server_request` and
    `track_server_response`.
    - Renames the existing client-side facts to `ClientRequest` and
    `ClientResponse` so reducers can distinguish client-to-server traffic
    from server-to-client traffic.
    - Adds `ServerRequest::response_from_result`, allowing a stored typed
    request to decode the matching typed server response from a raw JSON-RPC
    result payload.
    - Threads `AnalyticsEventsClient` through `OutgoingMessageSender` and
    records targeted server requests, replayed targeted requests, and
    matching targeted responses with the responding connection id needed for
    correlation.
    - Intentionally leaves broadcast server requests/responses out of
    analytics for now because the current model is per connection, while
    broadcasts fan one logical request out across multiple connections.
    - Breaks the app-server shutdown cycle by storing
    `Weak<OutgoingMessageSender>` in `ExternalAuthRefreshBridge` and
    upgrading it only when an external-auth refresh is actually requested.
    - Keeps reducer ingestion of the new server-side facts as no-ops for
    now; this PR is plumbing for later tool-approval analytics work.
    
    ## Verification
    
    - `cargo test -p codex-analytics`
    - `cargo test -p codex-app-server outgoing_message::tests::`
    - Covers typed-response reconstruction plus the targeted, replayed,
    broadcast-exclusion, and response-attribution analytics paths.
    
    ## Follow-up
    
    This PR intentionally stops at ingestion plumbing, so `ServerRequest`
    and `ServerResponse` facts are still reducer no-ops. Once a follow-up PR
    adds real downstream analytics output for those facts:
    
    - replace the temporary pre-reducer observation seam with reducer tests
    for the emitted event shape;
    - add end-to-end coverage in `app-server/tests/suite/v2/analytics.rs`
    for the real app-server workflow and captured analytics payload;
    - remove the temporary sender-level observer tests added here in favor
    of the real-output coverage above.
    
    ---
    
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/17088).
    * #18748
    * #18747
    * #17090
    * #17089
    * #20241
    * #20239
    * __->__ #17088
  • Spread AbsolutePathBuf (#17792)
    Mechanical change to promote absolute paths through code.
  • app-server: Only unload threads which were unused for some time (#17398)
    Currently app-server may unload actively running threads once the last
    connection disconnects, which is not expected.
    Instead track when was the last active turn & when there were any
    subscribers the last time, also add 30 minute idleness/no subscribers
    timer to reduce the churn.
  • 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: 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.
  • Apply argument comment lint across codex-rs (#14652)
    ## Why
    
    Once the repo-local lint exists, `codex-rs` needs to follow the
    checked-in convention and CI needs to keep it from drifting. This commit
    applies the fallback `/*param*/` style consistently across existing
    positional literal call sites without changing those APIs.
    
    The longer-term preference is still to avoid APIs that require comments
    by choosing clearer parameter types and call shapes. This PR is
    intentionally the mechanical follow-through for the places where the
    existing signatures stay in place.
    
    After rebasing onto newer `main`, the rollout also had to cover newly
    introduced `tui_app_server` call sites. That made it clear the first cut
    of the CI job was too expensive for the common path: it was spending
    almost as much time installing `cargo-dylint` and re-testing the lint
    crate as a representative test job spends running product tests. The CI
    update keeps the full workspace enforcement but trims that extra
    overhead from ordinary `codex-rs` PRs.
    
    ## What changed
    
    - keep a dedicated `argument_comment_lint` job in `rust-ci`
    - mechanically annotate remaining opaque positional literals across
    `codex-rs` with exact `/*param*/` comments, including the rebased
    `tui_app_server` call sites that now fall under the lint
    - keep the checked-in style aligned with the lint policy by using
    `/*param*/` and leaving string and char literals uncommented
    - cache `cargo-dylint`, `dylint-link`, and the relevant Cargo
    registry/git metadata in the lint job
    - split changed-path detection so the lint crate's own `cargo test` step
    runs only when `tools/argument-comment-lint/*` or `rust-ci.yml` changes
    - continue to run the repo wrapper over the `codex-rs` workspace, so
    product-code enforcement is unchanged
    
    Most of the code changes in this commit are intentionally mechanical
    comment rewrites or insertions driven by the lint itself.
    
    ## Verification
    
    - `./tools/argument-comment-lint/run.sh --workspace`
    - `cargo test -p codex-tui-app-server -p codex-tui`
    - parsed `.github/workflows/rust-ci.yml` locally with PyYAML
    
    ---
    
    * -> #14652
    * #14651
  • app-server: Silence thread status changes caused by thread being created (#13079)
    Currently we emit `thread/status/changed` with `Idle` status right
    before sending `thread/started` event (which also has `Idle` status in
    it).
    It feels that there is no point in that as client has no way to know
    prior state of the thread as it didn't exist yet, so silence these kinds
    of notifications.
  • app-server: Add ephemeral field to Thread object (#13084)
    Currently there is no alternative way to know that thread is ephemeral,
    only client which did create it has the knowledge.
  • feat(app-server): thread/unsubscribe API (#10954)
    Adds a new v2 app-server API for a client to be able to unsubscribe to a
    thread:
    - New RPC method: `thread/unsubscribe`
    - New server notification: `thread/closed`
    
    Today clients can start/resume/archive threads, but there wasn’t a way
    to explicitly unload a live thread from memory without archiving it.
    With `thread/unsubscribe`, a client can indicate it is no longer
    actively working with a live Thread. If this is the only client
    subscribed to that given thread, the thread will be automatically closed
    by app-server, at which point the server will send `thread/closed` and
    `thread/status/changed` with `status: notLoaded` notifications.
    
    This gives clients a way to prevent long-running app-server processes
    from accumulating too many thread (and related) objects in memory.
    
    Closed threads will also be removed from `thread/loaded/list`.
  • codex-rs/app-server: graceful websocket restart on Ctrl-C (#12517)
    ## Summary
    - add graceful websocket app-server restart on Ctrl-C by draining until
    no assistant turns are running
    - stop the websocket acceptor and disconnect existing connections once
    the drain condition is met
    - add a websocket integration test that verifies Ctrl-C waits for an
    in-flight turn before exit
    
    ## Verification
    - `cargo check -p codex-app-server --quiet`
    - `cargo test -p codex-app-server --test all
    suite::v2::connection_handling_websocket`
    - I (maxj) tested remote and local Codex.app
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • fix: address flakiness in thread_resume_rejoins_running_thread_even_with_override_mismatch (#12381)
    ## Why
    `thread/resume` responses for already-running threads can be reported as
    `Idle` even while a turn is still in progress. This is caused by a
    timing window where the runtime watch state has not yet observed the
    running-thread transition, so API clients can receive stale status
    information at resume time.
    
    Possibly related: https://github.com/openai/codex/pull/11786
    
    ## What
    - Add a shared status normalization helper, `resolve_thread_status`, in
    `codex-rs/app-server/src/thread_status.rs` that resolves
    `Idle`/`NotLoaded` to `Active { active_flags: [] }` when an in-progress
    turn is known.
    - Reuse this helper across thread response paths in
    `codex-rs/app-server/src/codex_message_processor.rs` (including
    `thread/start`, `thread/unarchive`, `thread/read`, `thread/resume`,
    `thread/fork`, and review/thread-started notification responses).
    - In `handle_pending_thread_resume_request`, use both the in-memory
    `active_turn_snapshot` and the resumed rollout turns to decide whether a
    turn is in progress before resolving thread status for the response.
    - Extend `thread_status` tests to validate the new status-resolution
    behavior directly.
    
    ## Verification
    - `cargo test -p codex-app-server
    suite::v2::thread_resume::thread_resume_rejoins_running_thread_even_with_override_mismatch`
  • Add field to Thread object for the latest rename set for a given thread (#12301)
    Exposes through the app server updated names set for a thread. This
    enables other surfaces to use the core as the source of truth for thread
    naming. `threadName` is gathered using the helper functions used to
    interact with `session_index.jsonl`, and is hydrated in:
    - `thread/list`
    - `thread/read`
    - `thread/resume`
    - `thread/unarchive`
    - `thread/rollback`
    
    We don't do this for `thread/start` and `thread/fork`.
  • feat: add nick name to sub-agents (#12320)
    Adding random nick name to sub-agents. Used for UX
    
    At the same time, also storing and wiring the role of the sub-agent
  • app-server: expose loaded thread status via read/list and notifications (#11786)
    Motivation
    - Today, a newly connected client has no direct way to determine the
    current runtime status of threads from read/list responses alone.
    - This forces clients to infer state from transient events, which can
    lead to stale or inconsistent UI when reconnecting or attaching late.
    
    Changes
    - Add `status` to `thread/read` responses.
    - Add `statuses` to `thread/list` responses.
    - Emit `thread/status/changed` notifications with `threadId` and the new
    status.
    - Track runtime status for all loaded threads and default unknown
    threads to `idle`.
    - Update protocol/docs/tests/schema fixtures for the revised API.
    
    Testing
    - Validated protocol API changes with automated protocol tests and
    regenerated schema/type fixtures.
    - Validated app-server behavior with unit and integration test suites,
    including status transitions and notifications.