fix(workflow): preserve provider-executed tool-approval-response on resume#14931
Open
MyPrototypeWhat wants to merge 2 commits intovercel:mainfrom
Open
Conversation
…esume Closes vercel#14292 `WorkflowAgent` previously stripped every `tool-approval-response` part from the messages when handling approval resumption, regardless of whether the underlying tool was locally or provider-executed. For provider-executed tools (e.g. MCP via the OpenAI Responses API) this silently dropped the approval before `convertToLanguageModelPrompt` could forward it, so the provider never learned of the approval and the tool was never executed. This is the inverse of vercel#14289 (which fixed approvals being forwarded twice). The fix here: - Tracks `providerExecuted` from the `tool-approval-response` part itself in `collectToolApprovalsFromMessages` — the same flag `convertToLanguageModelPrompt` already uses to decide which approval responses to forward to the model. - Skips local execution and synthetic tool-result injection for provider-executed approvals. - Preserves their `tool-approval-request` and `tool-approval-response` parts in the messages so the next conversion to `LanguageModelV4Prompt` forwards the response to the provider. Adds a regression test mirroring the structure of the existing approval resumption tests, asserting that the provider-executed response reaches the iterator and that no synthetic local result is injected.
Addresses PR review feedback on vercel#14931. Production change in `collectToolApprovalsFromMessages`: - Read `providerExecuted` from the `tool-call` part rather than the `tool-approval-response` part. The tool-call's flag is what the provider declared and is the source of truth used by `stream-text.ts` (line 1335). The previous source (`response.providerExecuted`) is metadata the UI carries and may disagree with the tool-call, which would re-introduce the original silent-drop bug for misaligned UIs. - Update the comment on the strip step to accurately describe the filter: "preserve everything not in localApprovalIds" includes both provider-executed approvals and orphan/unmatched approval-requests, not just provider-executed approvals. Test changes: - Rename `should preserve provider-executed tool-approval-response across resume` to `should forward ... to the provider` for consistency with the sister tests' phrasing. - Tighten the assertion to also pin `approved: true` so a future regression that drops the field would not pass. - Drop the `as ToolSet[string]` cast and the `as const` on `type`; contextual typing under `tools: ToolSet = { ... }` is sufficient. - Add `should preserve denied provider-executed tool-approval-response` to cover the symmetric denial path (the discriminator gate also applies to `localDeniedToolApprovals`). - Add `should partition mixed local and provider-executed approvals correctly` to exercise the per-`approvalId` `localApprovalIds` Set with one of each kind in the same resume turn.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #14292.
Summary
WorkflowAgentpreviously stripped everytool-approval-responsepart from messages when handling approval resumption, regardless of whether the underlying tool was locally or provider-executed. For provider-executed tools (e.g. MCP via the OpenAI Responses API) this silently dropped the approval beforeconvertToLanguageModelPromptcould forward it, so the provider never learned of the approval and the tool was never executed.This is the inverse of #14289, which fixed the symmetric bug where local approvals were being forwarded twice.
Fix
In
packages/workflow/src/workflow-agent.ts:collectToolApprovalsFromMessagesnow tracksproviderExecutedfrom thetool-approval-responsepart itself. This is the same flagconvertToLanguageModelPromptalready uses (line 360-361 ofconvert-to-language-model-prompt.ts) to decide which approval responses to forward to the model.localApprovedToolApprovalsandlocalDeniedToolApprovalssubsets and alocalApprovalIdsSet. Local approvals retain the existing behavior (executed locally, results synthesized, parts stripped). Provider-executed approvals are skipped from local execution and theirtool-approval-requestandtool-approval-responseparts are preserved in the messages so the next conversion forwards the response to the provider.The fix is intentionally narrow: no change to the public API, no new dependencies, mirrors the discriminator the conversion layer already uses.
Test plan
should preserve provider-executed tool-approval-response across resumetotool approval resumptiondescribe block inworkflow-agent.test.ts. The test constructs a provider-executed tool, resumes with an approved approval whose response carriesproviderExecuted: true, then asserts the response reaches the iterator and that no synthetic local tool-result was injected for it.tool approval resumption(3) still pass.pnpm check(oxlint + oxfmt) clean.pnpm type-check:fullexit 0.Follow-ups out of scope
providerExecutedtool approvals being passed to language model twice #14289 framing is consistent with this fix being apatchchangeset. NoExperimental_*API change.convertToLanguageModelPrompt) is now restored — both layers use the response'sproviderExecutedflag as the discriminator.