feat: #1097 support overrideArguments for approved tool calls#1098
feat: #1097 support overrideArguments for approved tool calls#1098
Conversation
🦋 Changeset detectedLatest commit: 3b487df The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 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 |
seratch
changed the title
|
@codex review |
![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
openai-agents-js/packages/agents-core/src/run.ts
Lines 528 to 530 in b987d9c
Detect server-managed sessions before blocking exec-only overrides
serverManagesConversation is computed only from conversationId/previousResponseId. Resumes using OpenAIConversationsSession usually have neither, so execution-only overrides (saveOverrideArguments: false) are rejected even though the transcript is server-managed and non-rewriteable. This blocks the intended fallback path for conversations-backed runs.
ℹ️ 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".
seratch
force-pushed
the
feat/approval-override-arguments
branch
from
b987d9c to
cea8656
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cea8656ac9
ℹ️ 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".
seratch
force-pushed
the
feat/approval-override-arguments
branch
from
cea8656 to
d0cb8cf
Compare
|
This PR is stale because it has been open for 10 days with no activity. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f307f0f4d7
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d6aa770df
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f68f7140bf
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f58a7af315
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b487df52f
ℹ️ 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".
| const serverManagesConversation = | ||
| Boolean(resolvedConversationId) || Boolean(resolvedPreviousResponseId); |
There was a problem hiding this comment.
Use the same chain-availability predicate as Runner.run(). Here any non-empty previousResponseId marks the stream as server-managed, so resumed runs with a tagged server-managed session (no explicit chain) skip saveStreamResultToSession. That silently drops streamed tool outputs from local session history, breaking later fresh turns that rely on session replay.
Useful? React with 👍 / 👎.
wsk-builds
left a comment
wsk-builds
left a comment
There was a problem hiding this comment.
I reviewed the approval override path, local session history rewrite, conversationId/previousResponseId handling, serialized RunState resume, and the updated examples. The targeted tests I ran passed: pnpm -F @openai/agents-core test runState.test.ts, pnpm -F @openai/agents-core test runner/sessionPersistence.test.ts, and pnpm -F @openai/agents-openai test hitlOpenAIConversationsSession.test.ts.
One docs/example issue to fix or clarify:
examples/agent-patterns/human-in-the-loop.ts:93-99: the sample says the serialized state could be resumed on a different thread/process, but afterRunState.fromString(...)it still iteratesresult.interruptionsfrom the original in-memory result. In the actual cross-process case that object is not available, and users should approve/reject the interruptions from the restored state, e.g.for (const interruption of state.getInterruptions()). This matters more now that the example demonstratesoverrideArguments, because the approval is supposed to be applied to the rehydratedRunStateand the sample should show the recoverable source of pending approvals.
I did not find a blocker in the core override execution, local rewrite application, server-managed conversation restrictions, or serialized resume behavior in the covered paths.
2 participants
This pull request resolves #1097.
It adds
overrideArgumentssupport toRunState.approve(...)so human approval flows can correct a pendingfunction_callbefore execution and resume with the corrected arguments. The runner andRunStatenow propagate those overrides through resumed execution, replay input, tracing, and serialized state so the approved tool call behaves consistently across interruption and resume boundaries.To support durable local replay, this change also adds session-history rewrite support for local sessions, including a new
OpenAIResponsesHistoryRewriteSessionand shared rewrite-aware session interfaces/exports. That allows corrected function-call arguments to become the canonical persisted history for local memory/file-backed flows, whilesaveOverrideArgumentsremains an opt-out for execution-only overrides in server-managed conversation paths where persisted history cannot be rewritten. Examples and tests were updated to cover local persistence, compaction ordering, public exports, and the conversation/previous-response edge cases around override handling.