Repository navigation
Allow request preparation in native TypeScript sessions - #2160
markstuart-oai merged 59 commits into
Conversation
🦋 Changeset detectedLatest commit: 8edad04 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23304ddecd
ℹ️ 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".
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Changes requested after coordinator and independent static review. Existing finding #2160 (comment) is blocking: Agent.run(null, {state, prepareRequest}) accepts and stores the callback, but the resumed path calls session.wait() and bypasses its only reader in _run. Reject this unsupported combination before opening the session. Also avoid cloning the entire prompt for ordinary operations without an editor (existing #2160 (comment)), and document the automatic-compaction failure restriction in the public callback API (#2160 (comment)). No duplicate inline comments added.
The exact-store/generation admission, one-shot replies, canceled callback joining, detached schema presentation and original executable authority otherwise look sound. Node tests/coverage passed; native packaging remains pending. No candidate code or local tests ran.
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved after coordinator and independent static re-review. All three findings are addressed: null-input resume rejects a new editor before restoring resources, ordinary sampling borrows the original prompt, and public docs describe pre/mid/post-turn compaction restrictions. The new regression verifies rejection leaves the checkpoint usable. The author reports a separate checkpoint/executor timeout under investigation; retain that follow-up.
Hosted CI remains pending. Static review only; no candidate code or local tests ran.
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved after coordinator and independent static re-review of the base integration. Direct tree comparison shows 25 of 31 changed files equal the current base. Reviewed the six overlapping files, including TraceContext propagation through startTurn, manual continuation and native argument ordering. Prepared-request behavior and all prior fixes remain intact.
Hosted CI remains pending. Static review only; no candidate code or local tests ran.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e572447fe
ℹ️ 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".
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Changes requested after validating two existing findings; no duplicate inline comments added.
- #2160 (comment) : one-shot admission still permits unhandled manual tools because canCheckpointManual ignores prepareRequest. If the model calls the tool, sealManualTurn rejects the editor-bearing operation and maybeSealManual returns InteractionRequired without resumable state. Reject this combination before model activity unless an interaction handler is supplied.
- #2160 (comment) : the early owned-run profile omits declarations.checklist, so an inevitably unsupported run can open provider/MCP resources before the later Session check rejects it. Add the checklist preflight.
The new deferred-roster changes themselves look sound: native validation checks the selected sampling roster before invoking the editor, and beforeStep cannot exceed the operation ceiling. No hosted checks were reported for this new head at inspection; CI follow-up remains open.
Static review only; no candidate code or local tests ran.
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
The reviewed findings are addressed: edited one-shot runs with unhandled manual tools reject before model/editor activity, checklist incompatibility rejects during early preflight, and managed parameter edits retain raw schema constraints through the sampling receipt while preserving immutable identity and original execution validation. Ordinary MCP edits still require lossless projection. Reviewed base integration; no new findings. Hosted native CI is pending. Coordinator and independent static review; no candidate code or local tests executed.
…-owned-child-trace-context
|
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". |
🛡️ 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. |
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved at 4f7b49f. Request preparation and fatal propagation remain intact when composed with authentication recovery and the reviewed A2A changes. Shared tracing/child-admission changes match reviewed 1875314, and patch hashes match the manifests.
Current reported hosted checks: 7 SUCCESS, 1 IN_PROGRESS. Coordinator and independent static review found no actionable integration regressions. No candidate code or local tests executed; no CI reruns requested.
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved at f563352. Coordinator and independent static review found no actionable integration issues. All 51 files changed since 4f7b49f match the merged parent and current base exactly; all 21 feature files remain byte-identical to the prior approval. The imported changes cover documentation, example import/output adjustments, and suite-owned telemetry setup/teardown without removing test assertions. Previously withdrawn findings remain withdrawn.
Fresh hosted CI remains pending. No candidate code, local tests, builds, generators or CI reruns executed.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f56335279f
ℹ️ 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".
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved at ea8a96e. The two-file fix addresses the existing raw-schema finding: managed request editors receive original JSON Schema keywords, unchanged declarations return to Core’s original projection, and partial edits retain raw keywords through serialization receipts. Tool identity checks, executable validators and ownership remain intact. Rust and native TypeScript regressions cover unchanged, description-only and partial-schema edits. Coordinator and independent static review found no actionable issues.
Fresh hosted CI remains pending. No candidate code, local tests, builds, generators or CI reruns executed.
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved at 8edad04. The post-turn guard preserves a completed answer by skipping automatic compaction when request preparation is active. Pre-turn and mid-turn rejection remain intact; pinned upstream and the composed Fatal propagation were inspected. Edited and ordinary regressions assert one versus two provider requests and preserve the final answer. Patch checksum matches the manifest, and the previously reviewed raw-schema fix is unchanged.
Coordinator and independent static review found no actionable issues. Hosted CI remains pending. No candidate code, local tests, builds or CI reruns executed.
Builds on merged #2162. Current head:
8edad04d.Summary
Add
prepareRequestto native TypeScript sessions so callers can edit instructions, input, tool descriptions and JSON schemas before sampling. The callback receives a detached request. Execution keeps the original tool declarations and admission rules.This supports direct root Responses HTTP requests with selected eager function and MCP tools. Each new sampling request invokes the editor; HTTP retries reuse its captured edit. Unsupported selections fail before editor or provider dispatch. Pre-turn and mid-turn automatic compaction fail before hidden requests. Post-turn compaction skips the hidden request and preserves the completed answer.
Managed tools expose their original JSON schemas, so partial edits preserve raw keywords. No-op and description-only edits restore Core's captured declaration projection before validation. The public types are available through
@openai/agents-codexand the existing@openai/agentsfacade.Test plan
The new post-turn native regression reproduces the old failure with an editor; its normal-compaction control passes. Current-head hosted code/security reviews and Justin's source review are clean.
A normal development build verifies Core
1c65094c, all 135 native inputs and six independent physical products. All 50 editor/context/default native cases and two callback/parent-policy controls pass, including both post-turn cases and all raw-schema regressions. Both required independent source reviews are clean. The complete SDK verification passes: format, lint, build, types and 9,567 tests, with 116 skipped. Reviewed source and all native product hashes remain unchanged before and after verification. Platform CI status is tracked in the checks below.Before the post-turn correction, the raw-schema fix passed 48 native editor/context/default cases, four Rust schema tests and two trace controls. Earlier qualification also covered 18 source cases and installed Linux release runtime/declarations. Those results support the unchanged components; earlier products do not qualify the changed Core.
Issue number
REQ106
Checks