fix(responses): refresh credentials on WebSocket reconnects - #2849
markstuart-oai wants to merge 3 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df3c37a0ac
ℹ️ 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".
df3c37a to
c5a1325
Compare
dpiet-oai
left a comment
There was a problem hiding this comment.
Requesting changes for the current-head credential-handling findings documented inline.
c5a1325 to
87028e2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87028e26d1
ℹ️ 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".
87028e2 to
c1f2db7
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1f2db736e
ℹ️ 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".
dpiet-oai
left a comment
There was a problem hiding this comment.
Requesting changes for the current-head credential transformation regression documented inline.
0e65622 to
7cf2846
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7cf2846911
ℹ️ 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".
7cf2846 to
c683ff5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c683ff5cbe
ℹ️ 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".
c683ff5 to
2d4ab82
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d4ab824aa
ℹ️ 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".
Castiron custom code✅ No new custom-code files detected. 47 mixed files remain; 5 existing customizations changed. Compared
42 existing customizations unchanged
2 more in the full report. A changed generated baseline means this report cannot reliably identify which handwritten lines changed. Inspect the custom-code diffDownload the exact patch produced by this run (requires repository access): gh run download 36895245198 --repo openai/openai-node \
--name castiron-custom-code-36895245198-1 --dir /tmp/castiron-custom-code-36895245198-1
git apply --stat /tmp/castiron-custom-code-36895245198-1/custom-code.patch
cat /tmp/castiron-custom-code-36895245198-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin 95b197b6b35c84bb7da322c3e5eb9adf1d1f79e7 5f6f94b73bee507c55a16eeee363941834b03408
python3 scripts/castiron/custom_code_report.py report \
--base 95b197b6b35c84bb7da322c3e5eb9adf1d1f79e7 \
--head 5f6f94b73bee507c55a16eeee363941834b03408 --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-5f6f94b73bee
cat /tmp/castiron-custom-code-5f6f94b73bee/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
2d4ab82 to
b3a0d59
Compare
dpiet-oai
left a comment
There was a problem hiding this comment.
Requesting changes for the current-head reconnect compatibility regressions documented inline.
b3a0d59 to
f9106a5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9106a51b4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
f9106a5 to
d77e798
Compare
Castiron-Internal-PR: openai/openai-node-internal#212 Castiron-Source-SHA: 55ae639a6df5ca90c61cea0285d9468fc0ff5a54 Castiron-Public-Base-SHA: 95b197b
d77e798 to
2ff1948
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f6f94b73b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const hookKey = | ||
| current.providerKey !== undefined && apiKey !== undefined && apiKey !== current.providerKey | ||
| ? validateCapturedAPIKey(client, apiKey) | ||
| : undefined; | ||
| const cached = current.commit(); | ||
| return hookKey === undefined ? validateCapturedAPIKey(client, cached) : hookKey; |
There was a problem hiding this comment.
When a client subclass uses the existing wrapper pattern await super._callApiKey() followed by capture?.(this.apiKey) (or derives its capture from that property), a reconnect runs super in deferred-cache mode and leaves client.apiKey at the old value until current.commit(). The wrapper therefore captures stale key A; this branch prefers it over providerKey B and sends A on the new handshake, even though it then caches B. Preserve the provider result for these post-super wrappers rather than treating that stale capture as a credential transform.
AGENTS.md reference: AGENTS.md:L52-L57
Useful? React with 👍 / 👎.
) Responses WebSockets now refresh callable API keys on every reconnect attempt that uses SDK credentials. Initial and subsequent handshakes preserve caller Authorization, Basic auth, custom credentials, and TLS settings. Closing during a slow refresh cancels promptly without replaying a command whose delivery is uncertain. Validation: real stable and beta upgrade, redirect and TLS reconnect tests; focused 583/583, lint, CJS/ESM build, and actual packed-package browser and TypeScript consumer checks. At the same commit, Node 22/24/26, Windows build, and the required Castiron custom-code budget passed in openai#2849. Supersedes openai#2849 at the same tested commit to obtain a fresh review. Previous review discussion remains at openai#2849; prior approvals are not assumed. --------- Co-authored-by: markstuart-oai <323302876+markstuart-oai@users.noreply.github.com>
Responses WebSockets now refresh callable API keys on each reconnect attempt that needs one, preserving caller-supplied Authorization, Basic auth and custom credentials on initial and subsequent handshakes. Client and socket headers are captured before an asynchronous refresh.
Closing during credential refresh ends the connection promptly without replaying an in-flight create. The synchronous constructor still requires an already-resolved key or an explicit caller credential.
Validation: real upgrade and TLS reconnect tests pass on stable and beta APIs, along with packed-package consumer, build and lint checks.