test: harden Agents async timeouts and synchronization - #994
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 code✅ No new custom-code files detected. 70 mixed files remain; 0 existing customizations changed. Compared 70 existing customizations unchanged
30 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 34532821986 --repo openai/openai-java \
--name castiron-custom-code-34532821986-1 --dir /tmp/castiron-custom-code-34532821986-1
git apply --stat /tmp/castiron-custom-code-34532821986-1/custom-code.patch
cat /tmp/castiron-custom-code-34532821986-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin e2ac1691d0b313582de19ca688822ef0e1ee31b0 1e3f18fc63518839079cb093939c883d7cd0f4a7
python3 scripts/castiron/custom_code_report.py report \
--base e2ac1691d0b313582de19ca688822ef0e1ee31b0 \
--head 1e3f18fc63518839079cb093939c883d7cd0f4a7 --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-1e3f18fc6351
cat /tmp/castiron-custom-code-1e3f18fc6351/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 837cd16f03
ℹ️ 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".
jbeckwith-oai
left a comment
There was a problem hiding this comment.
One in-scope blocking issue remains on this exact head: the blanket timeout expansion changes the deliberately distinct stream.close() nonblocking assertion from 1 second to 30 seconds. That allows a severe responsiveness regression to pass despite the test contract (closeDoesNotWaitForClaimedToolPostOrStartAnother). Please keep a short bound for that assertion while retaining the larger scheduling/completion budgets elsewhere. The existing inline P2 comment points to the exact line. I also verified the cited native-image failure was the consume(...).get(10s) timeout and confirmed the originally flaky bounded-cache case passes with the new completion budget.
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Re-reviewed the complete test-only diff at exact head 59f40cdd70f4025f667129c7f99bc23ccb8e356b. The prior blocking issue is fixed: the nonblocking stream.close() assertion again has its dedicated 1-second bound, while the larger 30-second budget is confined to load-sensitive scheduling and completion waits. The new latch/future synchronization removes polling races, response/client/executor cleanup is explicit, git diff --check passes, and CI lint/build/compatibility jobs are green (the full test job is still running). No unresolved in-scope blocking findings.
59f40cd to
1e3f18f
Compare
Summary
Testing
./gradlew :openai-java-core:test --tests "com.openai.services.beta.agents.AgentSessionStreamTest" --tests "com.openai.services.blocking.beta.agents.environments.EnvironmentTokenPaginationTest" :openai-java-core:lintKotlinThe original release attempt and its retry both timed out at
AgentSessionStreamTest.consumeunder the GraalVM native-image agent:https://github.com/openai/openai-java/actions/runs/34522715468