Skip to content

fix(WorkflowAgent): apply maxRetries and abortSignal from prepareCall - #18593

Open
edenbuilds wants to merge 1 commit into
vercel:mainfrom
edenbuilds:fix/workflow-agent-preparecall-maxretries-abortsignal-18576
Open

fix(WorkflowAgent): apply maxRetries and abortSignal from prepareCall#18593
edenbuilds wants to merge 1 commit into
vercel:mainfrom
edenbuilds:fix/workflow-agent-preparecall-maxretries-abortsignal-18576

Conversation

@edenbuilds

Copy link
Copy Markdown

Summary

  • WorkflowAgent.prepareCall can return maxRetries and abortSignal (they are on GenerationSettings / PrepareCallResult), but the field-by-field merge skipped both so they were silently dropped.
  • Copy those two fields into effectiveGenerationSettings the same way as seed / headers.
  • Add a regression test that asserts they land on streamTextIterator's generationSettings.

Fixes #18576

Test plan

  • pnpm test:node -- src/workflow-agent.test.ts in packages/workflow (includes new prepareCall coverage)
  • Confirm constructor/per-call maxRetries / abortSignal still override prepareCall via the existing merge order

Made with Cursor

prepareCall could return these GenerationSettings fields, but the
field-by-field merge skipped them so they were silently dropped (vercel#18576).
@hyamero

hyamero commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Thanks for picking this up. I filed #18576, so I verified the branch: reverting just the four source lines makes the new test fail and restoring them passes all 74, and the merge order you left unchecked does hold for both fields. The abortSignal half is correct and complete, traced through mergeAbortSignals and the loop's aborted checks down to streamModelCall, and that was the half with real user impact.

On maxRetries I owe you a correction to my own issue. I wrote that the call "proceeds with the constructor/per-stream value or the default (mergedGenerationSettings.maxRetries ?? 2)", but that line is the telemetry payload at workflow-agent.ts:1924, not the model call path. Read literally, your change is exactly what the issue asked for. The runtime picture is different though: maxRetries never reaches a retry implementation from any source. streamModelCall at do-stream-step.ts:132-154 lists its fields explicitly and omits it (abortSignal is in that list), experimental_streamLanguageModelCall has neither retry logic nor such a parameter, and prepareRetries is referenced only by generate-text.ts and stream-text.ts, never in packages/workflow. The constructor value, the per-stream value, and the prepareStep value are all equally disconnected.

So this half makes prepareCall consistent with the other sources rather than making the setting take effect, and the new test passes because it asserts on the object handed to streamTextIterator rather than on retry behavior. That may well be by design, since doStreamStep is a workflow step ('use step', line 114) and the doc comment on GenerationSettings.maxRetries says retries are handled by the step mechanism. If so, the real question is whether maxRetries belongs in PrepareCallResult at all, or should be documented as telemetry-only. Either resolution works for me, but the type currently promises something no code path delivers.

One piece of the issue is untouched: prepareStepGenerationSettingKeys (stream-text-iterator.ts:44) includes 'maxRetries' but omits abortSignal, which PrepareStepResult types via Partial<GenerationSettings>. Same silent drop, and unlike maxRetries it has real consequences, since abortSignal does work in this package. Because the PR says Fixes #18576, merging would close that out along with it, so it is worth either adding the key here or splitting it into a follow-up.

Minor and pre-existing: when both a per-stream and a prepareCall abortSignal are present, the latter is discarded rather than merged (workflow-agent.ts:1792). That matches how the constructor signal is already treated, but running both through mergeAbortSignals would be safer for cancellation sources.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WorkflowAgent: maxRetries and abortSignal returned from prepareCall are silently ignored

2 participants