Repository navigation
fix(auth): recover after synchronous refresh startup failure - #1246
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. |
Castiron custom codeEvaluated main: Trusted main policy: ✅ No new custom-code files detected. 97 mixed files remain; 0 existing customizations changed. Compared 97 existing customizations unchanged
57 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 39488998613 --repo openai/openai-java \
--name castiron-custom-code-39488998613-1 --dir /tmp/castiron-custom-code-39488998613-1
git apply --stat /tmp/castiron-custom-code-39488998613-1/custom-code.patch
cat /tmp/castiron-custom-code-39488998613-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin 1e54d93d6d1c68bbd8529b0327aa06bdea0abdb9 e64e234cca534051c2e26cd29cf3a33f571e8380
python3 scripts/castiron/custom_code_report.py report \
--base 1e54d93d6d1c68bbd8529b0327aa06bdea0abdb9 \
--head e64e234cca534051c2e26cd29cf3a33f571e8380 --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-e64e234cca53
cat /tmp/castiron-custom-code-e64e234cca53/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
410c2a5 to
b0cbd9c
Compare
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Reviewed b0cbd9c. No actionable findings. The shared refresh is settled and cleared when the provider throws synchronously, concurrent waiters receive the failure, subsequent calls can retry, and background callers retain a valid cached token. Checked regression coverage and existing async completion paths. Hosted SDK CI passed; separate OkTest run 39445509465 is still running. Static review only; no local tests executed.
markstuart-oai
left a comment
markstuart-oai
left a comment
There was a problem hiding this comment.
Reviewed b0cbd9c826bfabdc15733e46670de2e74fc21d50. No actionable findings.
The shared refresh helper clears the pending refresh and settles waiting callers when a provider throws synchronously. Background failures preserve the valid cached token, and subsequent calls can refresh again. The regression tests cover repeated failure, recovery, and concurrent waiters without changing token expiry or exchange behavior.
Validation: source review and exact-head hosted CI. The observed build, SDK tests, compatibility checks and lint passed. I did not run the candidate code locally; the focused test counts in the description are author-reported.
b0cbd9c to
a8fd55b
Compare
|
Resolved the All 24 focused authentication and HTTP-wrapper tests passed on the refreshed branch. Hosted CI is running on |
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved at a8fd55b. Re-reviewed the refreshed head. Authentication implementation and regression tests are byte-identical to the previously approved change; the additional changes come from upstream and the regenerated snapshot pointer.
Coordinator and independent static reviews found no actionable issues. Hosted repository checks passed or were intentionally skipped. External OkTest is still running and will be followed up. No candidate code or tests were run locally.
Castiron-Internal-PR: openai/openai-java-internal#194 Castiron-Source-SHA: 6e1f7349e35cac87c326879431ceec55b7f87508 Castiron-Public-Base-SHA: 1e54d93
a8fd55b to
e64e234
Compare
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved at e64e234. Authentication code/tests are unchanged; incremental differences incorporate upstream and regenerated snapshot metadata.
Coordinator and independent static reviews found no actionable issues. Current-head build and external OkTest remain pending; follow-up retained. No candidate code or tests ran locally.
3 participants
When
SubjectTokenProvider.getTokenAsync()throws before returning a future, workload-identity authentication can leave a refresh pending indefinitely. Settle the failed refresh and clear its shared state so waiting callers receive the failure and subsequent calls can refresh successfully. Background callers continue to receive their still-valid cached token.Addresses #852. Adapted from #877.
The change is limited to
WorkloadIdentityAuthand regression tests for repeated failures, foreground and background recovery, cached-token use during a pending refresh, and three concurrent waiters. Public APIs and token-expiry behavior remain unchanged.Validation before promotion: