Skip to content

Comments

fix(ai-chat): emit output-denied state for rejected tool approvals#956

Merged
threepointone merged 5 commits intomainfrom
tool-approval-fix
Feb 21, 2026
Merged

fix(ai-chat): emit output-denied state for rejected tool approvals#956
threepointone merged 5 commits intomainfrom
tool-approval-fix

Conversation

@whoiskatrin
Copy link
Contributor

@whoiskatrin whoiskatrin commented Feb 20, 2026

Summary

Fixes #955.

When CF_AGENT_TOOL_APPROVAL is sent with approved: false, AIChatAgent now transitions tool parts to output-denied (instead of approval-responded).

This aligns with AI SDK expectations so convertToModelMessages can emit a tool_result for denied approvals (required by Anthropic tool-use sequencing).

What changed

  • packages/ai-chat/src/index.ts
    • _applyToolApproval() now sets:
      • approved: true -> approval-responded
      • approved: false -> output-denied
    • Keeps merging existing approval object to preserve approval.id.
  • packages/ai-chat/src/tests/client-tool-duplicate-message.test.ts
    • Updated existing rejection assertions to expect output-denied.
    • Added regression test: rejection from approval-requested state also transitions to output-denied and preserves approval.id.
  • Added changeset for @cloudflare/ai-chat patch release.

Reviewer guide (quick pass)

  1. Start with _applyToolApproval in packages/ai-chat/src/index.ts.
  2. Check rejection test updates around:
    • CF_AGENT_TOOL_APPROVAL handles rejection (approved: false)
    • CF_AGENT_TOOL_APPROVAL with approved: false and autoContinue does NOT continue
  3. Check the new test:
    • CF_AGENT_TOOL_APPROVAL rejection from approval-requested sets output-denied

Validation

  • Ran targeted workers tests for rejection flows:
    • npm run test:workers --workspace @cloudflare/ai-chat -- src/tests/client-tool-duplicate-message.test.ts -t "rejection|approved: false"

@changeset-bot
Copy link

changeset-bot bot commented Feb 20, 2026

🦋 Changeset detected

Latest commit: 21c702c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@cloudflare/ai-chat Patch

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

@pkg-pr-new
Copy link

pkg-pr-new bot commented Feb 20, 2026

Open in StackBlitz

npm i https://pkg.pr.new/cloudflare/agents@956
npm i https://pkg.pr.new/cloudflare/agents/@cloudflare/ai-chat@956
npm i https://pkg.pr.new/cloudflare/agents/@cloudflare/codemode@956
npm i https://pkg.pr.new/cloudflare/agents/hono-agents@956

commit: 21c702c

@whoiskatrin
Copy link
Contributor Author

/bonk review this PR

Co-authored-by: whoiskatrin <whoiskatrin@users.noreply.github.com>
@ask-bonk
Copy link
Contributor

ask-bonk bot commented Feb 20, 2026

All 173 tests pass, including all 22 test files. Here's my review:


PR Review: fix(ai-chat): emit output-denied state for rejected tool approvals

Verdict: Approve -- this is a clean, well-scoped bug fix with good test coverage.

Summary

The core change is a one-line fix at packages/ai-chat/src/index.ts:1818: rejected tool approvals (approved: false) now transition to output-denied instead of approval-responded. This aligns with the AI SDK's convertToModelMessages expectations, which needs output-denied to emit a proper tool_result for providers like Anthropic that require tool-use sequencing.

Code review

packages/ai-chat/src/index.ts (+5/-2)

The fix itself is minimal and correct:

state: approved ? "approval-responded" : "output-denied",
  • The ternary is clear and matches the existing pattern used by message-builder.ts:254 which already handles tool-output-denied -> output-denied transitions for streaming chunks.
  • The approval object merge is preserved for both paths, so approval.id survives (needed by convertToModelMessages for the approvalId field).
  • The matchStates array (["input-available", "approval-requested"]) is unchanged and correct for both approval and rejection.
  • The auto-continue guard at line 621 (if (applied && approved && autoContinue)) already prevents continuation on rejection, so the output-denied state won't trigger an unintended LLM call.
  • The JSDoc update accurately describes the new behavior.

packages/ai-chat/src/tests/client-tool-duplicate-message.test.ts (+64/-2)

  • Two existing assertions updated from "approval-responded" to "output-denied" for the rejection cases (lines 678, 1131). Correct.
  • New test "CF_AGENT_TOOL_APPROVAL rejection from approval-requested sets output-denied" (line 749) covers the case where a tool is already in approval-requested state and gets rejected. It verifies both state: "output-denied" and that approval.id is preserved through the merge. Good regression coverage.
  • Test structure follows the existing patterns in the file.

.changeset/ten-llamas-exist.md (+9/-0)

Changeset is well-written and scoped to @cloudflare/ai-chat patch.

examples/playground/testing.md (+1/-1)

Escapes * in X-Agent-\* for markdown rendering. Unrelated but harmless housekeeping.

Minor observations (non-blocking)

  1. The _resolveMessageForToolMerge method (line 1043) checks for output-available, approval-responded, and approval-requested states but does not check for output-denied. If a message with an output-denied tool part gets re-persisted through a different code path, it won't get the toolCallId-based dedup. This seems fine in practice since denied tools are terminal state and won't be re-sent, but worth noting.

  2. All 173 ai-chat workers tests pass locally.

Verdict

Clean fix, correct logic, good tests. Ship it.

github run

Adds a new Vitest integration test that verifies when a tool approval is denied, convertToModelMessages produces a tool message containing a tool-approval-response (approved: false) and a tool-result with the expected toolCallId and toolName. The test sets up a worker/agent stub, persists a user and assistant tool approval-request message, sends a cf_agent_tool_approval with approved: false, converts persisted messages to model messages, and asserts the expected tool content. Also imports convertToModelMessages in the test file.
Copy link
Contributor

@threepointone threepointone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very good pr, I added an extra paranoid e2e test for my sake, and merged from main. approved, will land when ci passes

@threepointone threepointone merged commit ab401a0 into main Feb 21, 2026
4 checks passed
@threepointone threepointone deleted the tool-approval-fix branch February 21, 2026 16:08
@github-actions github-actions bot mentioned this pull request Feb 21, 2026
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.

@cloudflare/ai-chat: Tool approval denial does not produce a tool_result

2 participants