fix(ai): restore ToolLoopAgent onFinish callbacks#14336
fix(ai): restore ToolLoopAgent onFinish callbacks#14336paulelliotco wants to merge 1 commit intovercel:release-v6.0from
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 365a242b63
ℹ️ 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".
| /** | ||
| * Callback that is called when all steps are finished and the response is complete. | ||
| */ | ||
| onFinish?: ToolLoopAgentOnFinishCallback<TOOLS>; |
There was a problem hiding this comment.
Exclude onFinish from prepareCall option type
Adding onFinish to AgentCallParameters also widens ToolLoopAgentSettings.prepareCall (it is typed as Omit<AgentCallParameters, 'onStepFinish'>), but ToolLoopAgent.generate()/stream() immediately destructure onFinish and call prepareCall(options) without it. This creates a type/runtime mismatch where prepareCall appears able to read per-call onFinish but will always see it as absent, which can silently break user code that branches on this callback. Please either omit onFinish from the prepareCall input type as well or pass it through consistently.
Useful? React with 👍 / 👎.
365a242 to
0574616
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 057461691b
ℹ️ 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".
| /** | ||
| * Callback that is called when all steps are finished and the response is complete. | ||
| */ | ||
| onFinish?: ToolLoopAgentOnFinishCallback<TOOLS>; |
There was a problem hiding this comment.
Remove onFinish from prepareCall input type
Adding onFinish to AgentCallParameters widens ToolLoopAgentSettings.prepareCall (it is defined as Omit<AgentCallParameters<...>, 'onStepFinish'>), but ToolLoopAgent.generate() and stream() destructure onFinish and call prepareCall(options) without it, so prepareCall can never observe the callback at runtime. This creates a type/runtime mismatch that can silently break prepareCall logic that branches on onFinish; either omit onFinish from the prepareCall input type as well or pass it through consistently.
Useful? React with 👍 / 👎.
Summary
This fixes a v6 regression where
ToolLoopAgentstopped forwardingonFinishtogenerateText()/streamText().As a result:
onFinishcallbacks inToolLoopAgentSettingsno longer firedagent.generate({ onFinish })/agent.stream({ onFinish })were no longer accepted in the agent call typesThis PR:
onFinishtoAgentCallParametersandAgentStreamParametersonFinishforwarding inToolLoopAgent.generate()andstream()onFinishcallbacks, invoking the constructor callback first to matchonStepFinishFixes #14330.
Manual Verification
pnpm --filter ai test:node src/agent/tool-loop-agent.test.tspnpm --filter ai test:node src/agent/tool-loop-agent.test-d.tspnpm type-check:fullpnpm check