Commit Graph

6 Commits

  • 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
  • Add auth 401 observability to client bug reports (#14611)
    CXC-392
    
      [With
      401](https://openai.sentry.io/issues/7333870443/?project=4510195390611458&query=019ce8f8-560c-7f10-a00a-c59553740674&referrer=issue-stream)
      <img width="1909" height="555" alt="401 auth tags in Sentry"
      src="https://github.com/user-attachments/assets/412ea950-61c4-4780-9697-15c270971ee3"
      />
    
    
      - auth_401_*: preserved facts from the latest unauthorized response snapshot
      - auth_*: latest auth-related facts from the latest request attempt
      - auth_recovery_*: unauthorized recovery state and follow-up result
    
    
      Without 401
      <img width="1917" height="522" alt="happy-path auth tags in Sentry"
      src="https://github.com/user-attachments/assets/3381ed28-8022-43b0-b6c0-623a630e679f"
      />
    
      ###### Summary
      - Add client-visible 401 diagnostics for auth attachment, upstream auth classification, and 401 request id / cf-ray correlation.
      - Record unauthorized recovery mode, phase, outcome, and retry/follow-up status without changing auth behavior.
      - Surface the highest-signal auth and recovery fields on uploaded client bug reports so they are usable in Sentry.
      - Preserve original unauthorized evidence under `auth_401_*` while keeping follow-up result tags separate.
    
      ###### Rationale (from spec findings)
      - The dominant bucket needed proof of whether the client attached auth before send or upstream still classified the request as missing auth.
      - Client uploads needed to show whether unauthorized recovery ran and what the client tried next.
      - Request id and cf-ray needed to be preserved on the unauthorized response so server-side correlation is immediate.
      - The bug-report path needed the same auth evidence as the request telemetry path, otherwise the observability would not be operationally useful.
    
      ###### Scope
      - Add auth 401 and unauthorized-recovery observability in `codex-rs/core`, `codex-rs/codex-api`, and `codex-rs/otel`, including feedback-tag surfacing.
      - Keep auth semantics, refresh behavior, retry behavior, endpoint classification, and geo-denial follow-up work out of this PR.
    
      ###### Trade-offs
      - This exports only safe auth evidence: header presence/name, upstream auth classification, request ids, and recovery state. It does not export token values or raw upstream bodies.
      - This keeps websocket connection reuse as a transport clue because it can help distinguish stale reused sessions from fresh reconnects.
      - Misroute/base-url classification and geo-denial are intentionally deferred to a separate follow-up PR so this review stays focused on the dominant auth 401 bucket.
    
      ###### Client follow-up
      - PR 2 will add misroute/provider and geo-denial observability plus the matching feedback-tag surfacing.
      - A separate host/app-server PR should log auth-decision inputs so pre-send host auth state can be correlated with client request evidence.
      - `device_id` remains intentionally separate until there is a safe existing source on the feedback upload path.
    
      ###### Testing
      - `cargo test -p codex-core refresh_available_models_sorts_by_priority`
      - `cargo test -p codex-core emit_feedback_request_tags_`
      - `cargo test -p codex-core emit_feedback_auth_recovery_tags_`
      - `cargo test -p codex-core auth_request_telemetry_context_tracks_attached_auth_and_retry_phase`
      - `cargo test -p codex-core extract_response_debug_context_decodes_identity_headers`
      - `cargo test -p codex-core identity_auth_details`
      - `cargo test -p codex-core telemetry_error_messages_preserve_non_http_details`
      - `cargo test -p codex-core --all-features --no-run`
      - `cargo test -p codex-otel otel_export_routing_policy_routes_api_request_auth_observability`
      - `cargo test -p codex-otel otel_export_routing_policy_routes_websocket_connect_auth_observability`
      - `cargo test -p codex-otel otel_export_routing_policy_routes_websocket_request_transport_observability`
  • feat: search_tool migrate to bring you own tool of Responses API (#14274)
    ## Why
    
    to support a new bring your own search tool in Responses
    API(https://developers.openai.com/api/docs/guides/tools-tool-search#client-executed-tool-search)
    we migrating our bm25 search tool to use official way to execute search
    on client and communicate additional tools to the model.
    
    ## What
    - replace the legacy `search_tool_bm25` flow with client-executed
    `tool_search`
    - add protocol, SSE, history, and normalization support for
    `tool_search_call` and `tool_search_output`
    - return namespaced Codex Apps search results and wire namespaced
    follow-up tool calls back into MCP dispatch
  • feat(otel): Centralize OTEL metric names and shared tag builders (#14117)
    This cleans up a bunch of metric plumbing that had started to drift.
    
    The main change is making `codex-otel` the canonical home for shared
    metric definitions and metric tag helpers. I moved the `turn/thread`
    metric names that were still duplicated into the OTEL metric registry,
    added a shared `metrics::tags` module for common tag keys and session
    tag construction, and updated `SessionTelemetry` to build its metadata
    tags through that shared path.
    
    On the codex-core side, TTFT/TTFM now use the shared metric-name
    constants instead of local string definitions. I also switched the
    obvious remaining turn/thread metric callsites over to the shared
    constants, and added a small helper so TTFT/TTFM can attach an optional
    sanitized client.name tag from TurnContext.
    
    This should make follow-on telemetry work less ad hoc:
    - one canonical place for metric names
    - one canonical place for common metric tag keys/builders
    - less duplication between `codex-core` and `codex-otel`
  • chore(otel): rename OtelManager to SessionTelemetry (#13808)
    ## Summary
    This is a purely mechanical refactor of `OtelManager` ->
    `SessionTelemetry` to better convey what the struct is doing. No
    behavior change.
    
    ## Why
    
    `OtelManager` ended up sounding much broader than what this type
    actually does. It doesn't manage OTEL globally; it's the session-scoped
    telemetry surface for emitting log/trace events and recording metrics
    with consistent session metadata (`app_version`, `model`, `slug`,
    `originator`, etc.).
    
    `SessionTelemetry` is a more accurate name, and updating the call sites
    makes that boundary a lot easier to follow.
    
    ## Validation
    
    - `just fmt`
    - `cargo test -p codex-otel`
    - `cargo test -p codex-core`
  • chore(otel): reorganize codex-otel crate (#13800)
    ## Summary
    This is a structural cleanup of `codex-otel` to make the ownership
    boundaries a lot clearer.
    
    For example, previously it was quite confusing that `OtelManager` which
    emits log + trace event telemetry lived under
    `codex-rs/otel/src/traces/`. Also, there were two places that defined
    methods on OtelManager via `impl OtelManager` (`lib.rs` and
    `otel_manager.rs`).
    
    What changed:
    - move the `OtelProvider` implementation into `src/provider.rs`
    - move `OtelManager` and session-scoped event emission into
    `src/events/otel_manager.rs`
    - collapse the shared log/trace event helpers into
    `src/events/shared.rs`
    - pull target classification into `src/targets.rs`
    - move `traceparent_context_from_env()` into `src/trace_context.rs`
    - keep `src/otel_provider.rs` as a compatibility shim for existing
    imports
    - update the `codex-otel` README to reflect the new layout
    
    ## Why
    `lib.rs` and `otel_provider.rs` were doing too many different jobs at
    once: provider setup, export routing, trace-context helpers, and session
    event emission all lived together.
    
    This refactor separates those concerns without trying to change the
    behavior of the crate. The goal is to make future OTEL work easier to
    reason about and easier to review.
    
    ## Notes
    - no intended behavior change
    - `OtelManager` remains the session-scoped event emitter in this PR
    - the `otel_provider` shim keeps downstream churn low while the
    internals move around
    
    ## Validation
    - `just fmt`
    - `cargo test -p codex-otel`
    - `just fix -p codex-otel`