Expands session affinity ID extraction sources#2899
Expands session affinity ID extraction sources#2899can1357 wants to merge 1 commit intorouter-for-me:devfrom
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6229351321
ℹ️ 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".
| if headers != nil { | ||
| if sid := headers.Get("X-Session-ID"); sid != "" { | ||
| return "header:" + sid, "" | ||
| for _, h := range []string{"X-Session-ID", "X-Client-Request-Id", "Session-Id", "Conversation-Id"} { |
There was a problem hiding this comment.
Session_id header for session affinity
extractSessionIDs now looks for Session-Id/Conversation-Id, but other Codex-facing code in this repo uses underscore-style headers (for example Session_id in sdk/api/handlers/openai/openai_responses_websocket_toolcall_repair.go:150). In Go, header lookup is case-insensitive but does not normalize _ to -, so requests carrying Session_id will still miss this new header path and fall back to body/hash extraction, which can break sticky auth selection for requests without stable body session fields.
Useful? React with 👍 / 👎.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request expands the session ID extraction logic to support additional headers (X-Client-Request-Id, Session-Id, Conversation-Id) and the prompt_cache_key body field. Feedback was provided to align documentation comments with the actual kebab-case header names and to unroll the header check loop to prevent unnecessary allocations in high-traffic request paths.
| // Priority for session ID extraction: | ||
| // 1. metadata.user_id (Claude Code format) - highest priority | ||
| // 2. X-Session-ID header | ||
| // 2. X-Session-ID / X-Client-Request-Id / session_id / conversation_id headers |
There was a problem hiding this comment.
The comment uses snake_case for session_id and conversation_id, but the implementation (and standard HTTP header naming) uses kebab-case (Session-Id, Conversation-Id). This can be misleading for developers trying to understand which headers are actually supported.
| // 2. X-Session-ID / X-Client-Request-Id / session_id / conversation_id headers | |
| // 2. X-Session-ID / X-Client-Request-Id / Session-Id / Conversation-Id headers |
| // Priority order: | ||
| // 1. metadata.user_id (Claude Code format with _session_{uuid}) - highest priority for Claude Code clients | ||
| // 2. X-Session-ID header | ||
| // 2. X-Session-ID / X-Client-Request-Id / session_id / conversation_id headers |
There was a problem hiding this comment.
The comment uses snake_case for session_id and conversation_id, but the implementation uses kebab-case. Updating this to match the actual header keys improves clarity.
| // 2. X-Session-ID / X-Client-Request-Id / session_id / conversation_id headers | |
| // 2. X-Session-ID / X-Client-Request-Id / Session-Id / Conversation-Id headers |
| // 2. X-Session-ID / X-Client-Request-ID / session_id / conversation_id headers | ||
| if headers != nil { | ||
| if sid := headers.Get("X-Session-ID"); sid != "" { | ||
| return "header:" + sid, "" | ||
| for _, h := range []string{"X-Session-ID", "X-Client-Request-Id", "Session-Id", "Conversation-Id"} { | ||
| if sid := headers.Get(h); sid != "" { | ||
| return "header:" + sid, "" | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Creating a new slice of strings on every call to extractSessionIDs causes unnecessary allocations in a high-traffic request path. Unrolling the header checks is more efficient and avoids these allocations. Additionally, the comment has been updated to match the kebab-case naming used in the code.
| // 2. X-Session-ID / X-Client-Request-ID / session_id / conversation_id headers | |
| if headers != nil { | |
| if sid := headers.Get("X-Session-ID"); sid != "" { | |
| return "header:" + sid, "" | |
| for _, h := range []string{"X-Session-ID", "X-Client-Request-Id", "Session-Id", "Conversation-Id"} { | |
| if sid := headers.Get(h); sid != "" { | |
| return "header:" + sid, "" | |
| } | |
| } | |
| } | |
| // 2. X-Session-ID / X-Client-Request-Id / Session-Id / Conversation-Id headers | |
| if headers != nil { | |
| if sid := headers.Get("X-Session-ID"); sid != "" { | |
| return "header:" + sid, "" | |
| } | |
| if sid := headers.Get("X-Client-Request-Id"); sid != "" { | |
| return "header:" + sid, "" | |
| } | |
| if sid := headers.Get("Session-Id"); sid != "" { | |
| return "header:" + sid, "" | |
| } | |
| if sid := headers.Get("Conversation-Id"); sid != "" { | |
| return "header:" + sid, "" | |
| } | |
| } |
- Updated session ID extraction to consider additional request headers, including X-Client-Request-Id, Session-Id, and Conversation-Id. - Added fallback parsing for the prompt_cache_key body field as a conversation/session identifier source. - Adjusted the in-code priority comments to reflect the expanded header and body-based identifier sources.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d78530a402
ℹ️ 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".
| if headers != nil { | ||
| if sid := headers.Get("X-Session-ID"); sid != "" { | ||
| return "header:" + sid, "" | ||
| for _, h := range []string{"X-Session-ID", "X-Client-Request-Id", "Session-Id", "Conversation-Id"} { |
There was a problem hiding this comment.
Conversation_id header alias
The new header extraction list includes Conversation-Id but not Conversation_id, while the Codex websocket path sets Conversation_id (internal/runtime/executor/codex_websockets_executor.go:805). In Go, _ and - are not interchangeable for header lookup, so these requests will still miss the intended header-based affinity path and fall back to body/hash IDs, which keeps session stickiness inconsistent for this flow.
Useful? React with 👍 / 👎.
1 participant
Broadens the set of identifiers used to maintain session affinity, improving compatibility with a wider range of clients (e.g., OpenAI Codex).
X-Client-Request-Id,Session-Id,Conversation-Id) alongsideX-Session-IDwhen extracting a session identifierprompt_cache_keybody field as a session source, which Codex stamps with a session ID