fix: #1104 preserve incomplete status for rejected function tool outputs#1110
fix: #1104 preserve incomplete status for rejected function tool outputs#1110
Conversation
🦋 Changeset detectedLatest commit: a45d3f6 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 |
|
Quick update on the investigation so far:
So at this point, the client-side SDK can represent |
|
This PR is stale because it has been open for 10 days with no activity. |
wsk-builds
left a comment
There was a problem hiding this comment.
Thanks for writing this up and for leaving the investigation note about managed-history behavior. I did a cross-PR pass against #1104 and #1191, and also ran the targeted tests locally:
pnpm -F @openai/agents-core test runner/toolExecution.test.tspnpm -F @openai/agents-openai test openaiResponsesModel.helpers.test.ts hitlOpenAIConversationsSession.test.tspnpm -F @openai/agents-realtime test openaiRealtimeBase.test.ts realtimeSession.test.ts
All passed.
My read: using status: "incomplete" for a human-rejected approval is consistent with #1104. The important distinction is that the tool did not actually run, so completed gives the model a structurally successful signal that conflicts with the rejection text. The core and Realtime changes both align with that interpretation, and the helper tests cover preserving the value through our local conversion layer.
I do think it is worth keeping the server-managed-history caveat visible before merge. The note in this PR says Responses/Conversations managed history may normalize incomplete back to completed, so this PR improves the SDK-local / request-side representation but should not be presented as a guaranteed persisted signal when OpenAI owns the transcript. A short comment in the PR description, release note, or a nearby test name would help avoid future confusion.
For #1191: I do see a semantic tension, but not necessarily a direct conflict if we frame them separately. #1110 is about a deliberate human rejection, where incomplete is the more honest model-visible state. #1191 is trying to rebalance an already-aborted managed conversation by synthesizing an output for a provider-persisted call; if that PR keeps completed for compatibility, it should explicitly justify that as an abort-recovery/server-transcript constraint rather than treating aborts and rejected approvals as the same status category.
Changeset coverage looks reasonable for the packages whose behavior/API surface changes, and the unit coverage is good for the SDK-local behavior. I would not block this PR on broader live API probes, but I would keep the managed-history limitation documented because the current tests cannot prove that server-side history preserves incomplete.
This pull request resolves #1104. It fixes inconsistent approval-rejection handling so rejected function tool outputs are no longer persisted or projected as if they completed successfully across
agents-core, Conversations-backed OpenAI sessions, and Realtime transport/history flows.Behaviorally, this change:
status: 'incomplete'while keeping successful executions oncompletedfunction_call_outputpayloads and localitem_updateprojections with the same rejection status