fix(agents): correct chat agent example for tool approval and coercion#31278
fix(agents): correct chat agent example for tool approval and coercion#31278thomasgauvin wants to merge 1 commit into
Conversation
thomasgauvin
requested review from
a team,
GregBrimble,
MattieTK,
Maximo-Guk,
WalshyDev,
ack-cf,
aninibread,
danielegm,
elithrar,
hsaxenaCF,
irvinebroque,
jahands,
krys-cf,
ncrouch-cflare,
ngayerie,
nikitacano,
ravindra-cloudflare,
roerohan,
securitypedant,
stechedo,
vy-ton and
zeinjaber
as code owners
thomasgauvin
requested review from
MohamedH1998,
ahaywood and
colbywhite
as code owners
Review✅ No style-guide issues found in commit CommandsOnly codeowners can run commands. Post a comment with the command to trigger it.
|
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cannot merge — 2 blocking issues need fixing.
CRITICAL: This PR accidentally commits ~14,600 macOS resource fork files (._* AppleDouble files) across the repository. These must be removed before merge. Check your local Git configuration to prevent this — for example, git config --global core.precomposeunicode true and ensure your global .gitignore excludes ._* and .DS_Store.
HIGH: The branch contains an unrelated commit (7205f4fe) that adds a Workers React + Vite deployment guide. If this wasn't intentional, rebase the branch onto production and drop that commit so the PR stays scoped to the chat agent fixes.
The actual content changes to chat-agent.mdx and chat-agents.mdx look correct.
|
Done. Here's a summary of the review for PR #31278: Verdict: Changes requested — 2 blocking issues prevent merge. I applied the labels Critical Issues Flagged
Technical Assessment of Intended ChangesThe actual documentation fixes in
No inline suggestions were needed for the MDX content itself because the fixes are technically accurate and properly formatted. |
|
This pull request requires reviews from CODEOWNERS as it changes files that match the following patterns:
|
|
/bonk rereview |
There was a problem hiding this comment.
"Both blocking issues from the previous review are now resolved.\n\n- No macOS resource fork files (._*) remain in the branch.\n- The unrelated commit has been removed; the branch is scoped to the chat agent fixes only.\n\nThe content changes are all technically correct:\n\n- z.coerce.number() properly handles string values that LLMs emit for numeric tool arguments.\n- part.state === \"approval-requested\" matches the actual runtime state name.\n- getToolApproval(part).id is the correct identifier to pass to addToolApprovalResponse.\n- Removing the part.type === \"tool\" checks is appropriate since the part type is dynamic (tool-*).\n\nApproved.\n"
|
Posted an approving review on PR #31278. The previous blocking issues are both resolved:
The fixes in
|
|
/bonk rereview |
There was a problem hiding this comment.
Re-reviewed — no issues found. The fixes are all correct:
- properly handles LLM string outputs for numeric tool arguments.
- matches the actual runtime state name.
- is the correct identifier for .
- Removing the checks is appropriate since the part type is dynamic.
Approved.
|
Re-reviewed — no issues found. The fixes are all correct:
Approved. |
|
Re-reviewed PR #31278 and posted an approving review. The fixes are all technically correct and properly scoped:
No blocking issues or style-guide violations found in the current diff. |
|
Manual review. LGTM |
12 participants
Summary
The chat agent example at
/agents/examples/chat-agent/contains 4 bugs that prevent the approval flow and tool execution from working correctly.Issues Found
1. Zod number validation fails with LLM string outputs (line 133-134)
Problem:
z.number()rejects string values like"4"and"23"that LLMs output when calling tools.Fix: Use
z.coerce.number()to automatically convert strings to numbers.2. Tool part type is dynamic, not
"tool"(line 229)Problem:
part.typeis set to dynamic values like"tool-calculate", not the literal string"tool". The conditionpart.type === "tool"never matches.Fix: Check
part.state === "approval-requested"directly instead of coupling type and state checks.3. Wrong approval state name (line 230)
Problem: The state is
"approval-requested", not"approval-required". The UI never renders.Fix: Use the correct state name
"approval-requested".4. Wrong ID passed to
addToolApprovalResponse(line 241, 251)Problem:
addToolApprovalResponseexpects theapproval.id(frompart.approval.id), notpart.toolCallId. PassingtoolCallIdcauses the warning: "Could not find toolCallId for approval ID" and the server never gets notified.Fix: Use
getToolApproval(part).idas documented in the API reference.5. Also fixed same
z.number()issue in chat-agents API reference (line 1006)The main
/agents/communication-channels/chat/chat-agents/page had the same Zod schema issue in itsprocessPaymentexample.Files Changed
src/content/docs/agents/examples/chat-agent.mdx— Full example fixsrc/content/docs/agents/communication-channels/chat/chat-agents.mdx— API referencez.coerce.number()fixVerification
These changes have been tested and verified to resolve the MissingToolResultsError and make the approval UI render correctly.