feat(agents): run local tools during session creation - #1141
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. |
|
@codex review pls |
Castiron custom codeEvaluated main: ✅ No new custom-code files detected. 91 mixed files remain; 4 existing customizations changed. Compared
87 existing customizations unchanged
47 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 37341145298 --repo openai/openai-java \
--name castiron-custom-code-37341145298-1 --dir /tmp/castiron-custom-code-37341145298-1
git apply --stat /tmp/castiron-custom-code-37341145298-1/custom-code.patch
cat /tmp/castiron-custom-code-37341145298-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin c50d94b56e76317888b7018fd51c6704c89865b5 41a3ef1c74f409da74a33e362d30ecf9e3c357dc
python3 scripts/castiron/custom_code_report.py report \
--base c50d94b56e76317888b7018fd51c6704c89865b5 \
--head 41a3ef1c74f409da74a33e362d30ecf9e3c357dc --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-41a3ef1c74f4
cat /tmp/castiron-custom-code-41a3ef1c74f4/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1a4492569
ℹ️ 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".
markstuart-oai
left a comment
There was a problem hiding this comment.
Reviewed b1a4492. The existing finding about creation without input remains valid. A self-hosted session can send created/idle events without starting a turn. The new overload then throws at EOF in both blocking and async consumption, even when the caller requests raw events only. Preserve whether creation has input and allow idle to finish that raw stream, while retaining the follow-up stream's turn check.
The shared dispatcher, result collection, retry keys, and response cleanup otherwise look consistent. I found no additional distinct issue. Hosted tests, lint, build, API/Jackson compatibility, Java 8/25 runtime checks, and required CI passed for this head. Source review only; I did not run local tests or live smoke tests.
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Reviewed b1a4492. The existing P2 no-input creation finding remains valid: self-hosted creation may omit input, but the new handler overload loses that distinction and only treats idle as terminal after a turn has ended. Both blocking raw iteration and async subscription then report unexpected EOF for a valid created/idle stream—even with an empty handler registry. Preserve input-less creation completion separately from the follow-up helper's completed-turn requirement. The final-result helper's initial-input requirement should not make raw event consumption fail.
Reviewed all ten changed files and supporting stream/result collection, request-option, retry and response-cleanup paths. No additional distinct finding. Exact-head hosted tests, lint, build, API/Jackson compatibility, Java 8/25 runtime, version matrix, Castiron and required CI passed; git diff --check passed. No local SDK tests or live API calls. Referencing the existing inline thread rather than duplicating it.
|
@codex review pls |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94cfa2268a
ℹ️ 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".
|
@codex review pls |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
| /** Local function handlers for a beta Agents creation stream. Never sent to the API. */ | ||
| class AgentToolHandlers | ||
| private constructor( | ||
| internal val handlers: Map<String, Function<Map<String, Any?>, CompletionStage<*>>> |
There was a problem hiding this comment.
[Medium] Preserve tool-error observation for creation handlers
openai-java-core/src/main/kotlin/com/openai/models/beta/agents/AgentToolHandlers.kt:10
Creation builds AgentSessionStreamSupport with params == null, so its onToolError is always null, and this handler container provides no way to supply the observer supported by follow-up streams. If initial-turn arguments are malformed, a handler fails, or its output cannot be serialized, the model gets the sanitized Tool handler failed. result but the application cannot log or monitor the real cause.
Suggested fix: carry an optional Consumer<AgentToolError> in AgentToolHandlers, pass it into creation stream support, and cover argument, execution, and output failures (including observer exceptions) in the blocking and async creation tests.
|
@codex review pls |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
markstuart-oai
left a comment
There was a problem hiding this comment.
Re-reviewed 41a3ef1. The prior input-free creation finding is fixed. Raw blocking and async streams can end at idle without a turn, while finalResult still rejects an incomplete turn. Shared dispatch preserves callback cleanup, tool-result idempotency, and the existing follow-up stream behavior.
The open suggestion to expose a creation tool-error observer remains an API follow-up. Source review only. Hosted build, lint, tests, API compatibility, Jackson, Java 8/25, required checks, and Castiron passed. Live smoke tests are author-reported; Java OkTest was still running in the request. No local workloads were run.
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Re-reviewed the full updated diff at 41a3ef1. The prior no-input creation-stream blocker is fixed for both blocking and async streams: raw created/idle iteration can complete without a turn, while final-result collection still reports an incomplete stream. Deriving input presence from raw JSON also preserves future input shapes. The parameterized regressions cover both stream styles and collection semantics. No additional actionable findings. Exact-head hosted checks passed (optional jobs skipped); source/hosted-CI review only, no local tests or live API calls.
Summary
Run local function tools while streaming a session's initial prompt, including sessions with
environment: none. Previously, local handlers required creating an idle session and then submitting a separate follow-up prompt.Before:
After:
The async service accepts the same overload and completion-stage handlers. Callbacks remain local; both creation and follow-up streams reuse the existing dispatcher, retry behavior, and opt-in result collection.