Add buffered Responses for native agents - #5589

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1aca01e66c

Reviewed the full diff with two independent static reviewers. No actionable findings. Buffered ingress preserves ordered Core tool/history/policy handling, validates all output before effects, retains terminal metadata while withholding rejected output, and keeps body ownership/cancellation/idle timeout/retry behavior on the existing mapper. Reviewed provider selection and unsupported-mode guards, restore selection, observation transport, source manifests, and benchmark actual-wire receipts/full-owner accounting. Benchmark claims are limited to the documented local HTTP workloads; they do not establish live-streaming or short-response parity. The existing field-order comment is nonblocking: the published v0.24.0-alpha.6 provider has no auth field, and this constructor is keyword-only. Hosted CI: 4 passed, 2 skipped, 1 neutral, 4 running; qualification retained. No candidate code/tests executed locally.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1369f2ee01

Re-reviewed the three-file update with an independent static reviewer. Moving response_mode after auth follows the append convention; the matching API snapshot and explicit asdict key-order assertion are consistent. No runtime behavior change or new actionable findings. The existing field-order feedback is addressed. Hosted CI: 3 passed, 2 skipped, 1 neutral, 5 running; follow-up retained. No candidate code or tests executed locally.

Reassessed the existing body-bound feedback and am superseding my earlier approval. The finding at #5589 (comment) is actionable: response_stream appends every chunk to an unbounded Vec, and a peer that keeps sending within the per-chunk idle timeout can exhaust the SDK process before JSON validation. Enforce a cumulative response-body limit before extending the buffer, and make the oversize failure non-retryable; cover both declared and chunked/unknown-length responses. This is the existing finding, not a duplicate inline comment. Independent static review confirms the issue. Hosted CI remains pending. No candidate code or tests executed locally.

🛡️ Codex Security Review · Automatically triggered

Here are some automated security review suggestions for this pull request.

Reviewed commit: a006551934

Re-reviewed the body-limit fix with an independent static reviewer. The JSON request now sets the existing 64 MiB transport limit; declared and cumulative sizes are checked before delivery to the buffer. ResponseTooLarge stays typed through the JSON mapper and becomes non-retryable InvalidRequest in Core. Added declared/chunked regressions assert one request and cleanup; exact-once custom-tool coverage is consistent with the existing consumer. Patch digest matches the manifest. This resolves my request-changes decision. Hosted CI remains pending. No candidate code or tests executed locally.

Re-reviewed the rebased head with an independent static reviewer. Of 14 changed blobs, 13 exactly match the current base; the remaining API-contract fixture integrates the base metadata. The buffered implementation is byte-identical to approved a006551, including the body bound and non-retryable oversize handling. The new JSON-allocation feedback identifies real parsing overhead, but the documented limit is on body bytes, not total heap; no unbounded-body bypass is demonstrated. Complexity limits are nonblocking hardening. Hosted CI: 4 passed, 1 neutral, 4 running. Static review only; no candidate code or tests executed locally. Hosted CI follow-up retained.

Re-reviewed 678ec3a with an independent static reviewer. No new blockers. The pre-deserialization scan bounds dense JSON structures while correctly excluding escaped string payloads; serde retains syntax/depth validation and the existing 64 MiB transport bound remains. Structural-limit errors are non-retryable. Tests cover dense objects/scalars, escaped large text and one-request native failure. Benchmark workers and peers consistently disable bytecode writes, record and validate runtime cache settings, and document the separate requirement to inventory valid caches. Patch SHA-256 matches the series manifest. Hosted CI: 3 passed, 1 neutral, 5 running; platform qualification follow-up retained. No candidate code or tests executed locally.

Follow-up to my approval: new hosted CI confirms a release-contract integration failure.

[P2] Refresh the reviewed API snapshot against the integrated base. packages/codex/tests/fixtures/released_api_contract.json:2028 records only three SandboxArchiveLimits fields. The tested merge 1716714 contains five additional fields from base 2a09c45: max_tar_metadata_bytes, max_tar_stream_bytes, max_zip_directory_bytes, max_tar_lzma_memory_bytes, and max_zip_lzma_dictionary_bytes, with their referenced defaults. Five completed Linux/Windows jobs fail the same four released API contract assertions. Regenerate and review the contract against the integrated source, preserving response_mode and the intended public API, then rerun the matrix without weakening contract checks.

Hosted evidence: https://github.com/openai/openai-agents-python/actions/runs/42412090275/job/128529419971 and https://github.com/openai/openai-agents-python/actions/runs/42412090275/job/128528865983 . This is stale snapshot integration, not evidence of a buffered-response behavior regression. Independent static inspection confirmed the missing fields. This decision supersedes my approval based on new CI evidence; remaining hosted checks are still pending. No candidate code or tests executed locally.

Re-reviewed a5a93fb with an independent static reviewer. The contract finding is addressed: the snapshot now records all eight SandboxArchiveLimits fields and the four added default constants, matching the integrated source. Deep comparison shows all other public API entries, including response_mode, are unchanged. The five archive implementation/test files match the merged base; buffered-response implementation, byte limits and structural limits are unchanged from the reviewed commit. No remaining actionable findings.

Hosted CI has restarted and remains pending; successful current-head qualification is still required before merge and is tracked. No candidate code or tests executed locally.

Re-reviewed ab01182 with an independent static reviewer. The three-file delta applies the structural complexity limit to non-2xx buffered HTTP bodies before retry/error parsing. ResponseTooComplex propagates as a typed non-retryable transport error and maps to InvalidRequest in Core; ordinary HTTP errors keep their existing retry behavior. Tests cover dense payloads at 200/400/403/429/503 and a normal 429 followed by a successful retry. The existing byte bound and successful-response validation remain intact; patch checksum matches the manifest. No actionable findings.

Hosted CI has restarted and remains pending; follow-up retained. No candidate code or tests executed locally.

Re-reviewed 93c0065 with an independent static reviewer. This update incorporates base-branch changes: 47 of the 49 changed files match the current base exactly; session.py retains the buffered-mode additions while adopting the base checkpoint-policy check, and the API snapshot incorporates base declarations while retaining response_mode/http_json. The previously reviewed buffered transport, size/complexity bounds, provider and regression tests are unchanged. No new actionable findings. Hosted CI has restarted on this head and remains pending; the earlier green result applied to ab01182. No candidate code or tests executed locally.

Two static reviews of the current integration found no new actionable correctness/security issues. Ordinary client construction retains the bounded public-certificate wrapper cache; pooled clients use freshly read and parsed CA bytes and a digest, retire changed roots before building, and validate certificate wrappers on construction. Invalid-DER retirement is covered. The buffered patch is unchanged apart from its series number; MCP/voice integration and the provider override regression preserve the feature boundaries. Approval is a code decision, not a green-CI claim. Python 3.10 hosted CI failed in test_rpc_timeout_exception_chain_does_not_expose_bearer[idle]: its 50 ms timeout elapsed with no request received by the fixture. Logs suggest pre-request timing, but do not prove flakiness. This check still needs resolution: https://github.com/openai/openai-agents-python/actions/runs/42428914157/job/128580220536 . No candidate code or tests run locally.

Re-reviewed this exact commit after the rebase. The buffered Responses patch is byte-identical after renumbering; the additional sandbox diagnostic patch matches the merged base, and benchmark documentation accurately clarifies its scope. No new actionable findings. Hosted CI is still running on this head; the earlier timeout failure is not established as resolved. Static review only; no local code or tests executed.