Conversation
Write request IDs as soon as headers arrive, including error paths, and persist explicit null fields in rollout response metadata. Add rollout coverage for both successful and error responses. Co-authored-by: Codex <noreply@openai.com>
|
@codex review this |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb05429f8d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Handle the new rollout response metadata event in exhaustive matches outside core so downstream crates keep compiling. Co-authored-by: Codex <noreply@openai.com>
Do not reuse the websocket upgrade request ID as per-turn rollout metadata for later stream requests. Co-authored-by: Codex <noreply@openai.com>
Update resume expectations for replayed response metadata and regenerate app-server protocol schema fixtures. Co-authored-by: Codex <noreply@openai.com>
|
@codex review this |
1 similar comment
|
@codex review this |
Mark response metadata as internal-only for schema export and strip it from app-server notifications while keeping rollout persistence intact. Co-authored-by: Codex <noreply@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e080a5871
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Filter rollout-only response metadata out of resumed initial messages in protocol, so app-server does not need to scrub SessionConfigured history. Keep app-server's live event suppression so ResponseMetadata stays out of the public interface. Co-authored-by: Codex <noreply@openai.com>
Rename the response stream field that carries an early request ID for rollout persistence so its purpose is explicit. Co-authored-by: Codex <noreply@openai.com>
Rename the response stream field to request_id_for_rollout_log so its purpose is explicit at the call sites. Co-authored-by: Codex <noreply@openai.com>
Keep the cloned event binding named event when filtering response metadata from app-server notifications. Co-authored-by: Codex <noreply@openai.com>
Use rollout ResponseMetadata terminology in the websocket request-id comment to avoid confusion with the static rollout metadata header. Co-authored-by: Codex <noreply@openai.com>
Carry optional request IDs on core invalid-request errors so pre-stream HTTP 400 failures still emit rollout response metadata. Co-authored-by: Codex <noreply@openai.com>
|
@codex review this |
|
Codex Review: Didn't find any major issues. Another round soon, please! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Replace the rollout-only response metadata event with separate ResponsesApiRequestId and ResponsesApiResponseId events so persisted logs do not carry unrelated null fields. Co-authored-by: Codex <noreply@openai.com>
Summary
response.createdand write request IDs even when the request fails before streamingnullvalues and add rollout coverage for success and error casesTesting
Notes
cargo testsuite because the repo instructions say to ask first aftercoreorprotocolchanges