Skip to content

[codex] Add response.processed websocket request#21284

Open
pakrym-oai wants to merge 2 commits intomainfrom
pakrym/response-processed-ws
Open

[codex] Add response.processed websocket request#21284
pakrym-oai wants to merge 2 commits intomainfrom
pakrym/response-processed-ws

Conversation

@pakrym-oai
Copy link
Copy Markdown
Collaborator

Summary

  • Add a response.processed websocket request payload and sender for Responses API websockets.
  • Send response.processed from try_run_sampling_request after a response completes, local turn processing succeeds, and the session-owned feature flag is enabled.
  • Add websocket coverage for both enabled and disabled feature-flag behavior.

Validation

  • just fmt
  • cargo test -p codex-core response_processed
  • cargo test -p codex-api responses_websocket
  • cargo test -p codex-features responses_websocket_response_processed_is_under_development
  • git diff --check
  • just fix -p codex-api -p codex-core -p codex-features
  • git diff --check origin/main...HEAD

@pakrym-oai pakrym-oai force-pushed the pakrym/response-processed-ws branch from 74bd628 to 6b2d486 Compare May 6, 2026 01:40
@pakrym-oai pakrym-oai marked this pull request as ready for review May 6, 2026 01:43
@pakrym-oai pakrym-oai requested a review from a team as a code owner May 6, 2026 01:43
Copy link
Copy Markdown
Collaborator

@jif-oai jif-oai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after my nits

&& outcome.is_ok()
&& let Some(response_id) = completed_response_id.as_deref()
{
client_session.send_response_processed(response_id).await;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks a little early.. we send response.processed before the in-flight work is drained and before the cancellation check below. This sounds race-prone to me

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw can we also keep this off the critical path if it's best effort only? Right now a stuck ws write can hold up a finished or cancelled turn until idle_timeout

));
};

send_websocket_request(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also clears failed ws here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants