Match Duo Chat tool approval badges to tool results by id
What does this MR do and why?
Fixes the Duo Chat approval card sometimes saying "Cancelled" after you clicked Approve, even though the tool ran.
The card decides between Approved and Cancelled by finding the tool result that belongs to it. Until now it guessed, using the tool name and the order of the log. The backend already gives the request and its result the same message_id (the tool call id). Now we just use that.
Why the guess broke: if one tool result never reaches the browser, or arrives out of order, the next request for the same tool grabs it and you get swapped badges. The original forward scan swapped them one way (#619249). The nearest-preceding scan from !251316 (merged) swaps them the other way when the earlier result is missing. Same bug, different order. With id matching a card can only be Approved by its own result.
| Scenario | Before | After |
|---|---|---|
| Deny tool A, then approve tool A | Approved, Cancelled | Cancelled, Approved |
| Approve tool A but its result never arrives, then approve tool A again | Approved, Cancelled | Cancelled, Approved |
| Approve the same tool twice | Approved, Approved | Approved, Approved |
What this does not fix: why a tool result sometimes goes missing at all. That looks like a service-side problem (notes below). After this change a lost result only affects its own card, and reloading the thread shows the right badge.
Where the shared id comes from, and where the result is probably getting lost
Every path in ai-assist that writes an approval request and its tool result stamps the tool call id on both as message_id:
duo_workflow_service/agents/chat_agent.py(_get_approvals) andduo_workflow_service/agents/tools_executor.py(_create_tool_ui_chat_log)duo_workflow_service/components/human_approval/tools_approval.pyduo_workflow_service/agent_platform/v1/components/agent/nodes/tool_approval_request_node.pyandnodes/tool_node.pyforagentic_chat/v1
The old transformer comment said correlation_id is always null. True, but message_id was the link all along.
On the missing result: I could not find a way for the browser to skip a frame. The suspicious spot is the incremental ui_chat_log cursor in duo_workflow_service/checkpointer/notifier.py. Its own comment says that a dropped oversized checkpoint makes an incremental client permanently miss entries. The missing session card from the report fits the same cause, since plugins/start_flow/index.js builds that card from the tool result. Worth its own issue on the service side.
Changes:
tool_denial_transformer.js: match request to result bymessage_id, ignorependingplaceholder cards. Trailing unanswered requests are left alone as before.- Transformer spec: fixtures now share one id, like the real log. New cases for deny then approve, approve then deny, a missing result, and a placeholder before its request.
tool_approval_spec.js(integration): the fixture used different ids for request and result. It now uses one, and the comment about what the service sends is corrected.
References
- Issue: #629915 (closed)
- Earlier report of the swapped badges: #619249
- Previous fix this replaces: !251316 (merged)
- !248785 (merged) documented the old Approved vs Cancelled behaviour
Screenshots or screen recordings
No visual change. Same card, only the badge it ends up with differs.
How to set up and validate locally
yarn jest ee/spec/frontend/ai/duo_agentic_chat/transformers/tool_denial_transformer_spec.jsyarn jest --config jest.config.integration.js ee/spec/frontend/integration/ai_duo_panel/duo_agentic_chat/tool_approval_spec.js- In Duo Agentic Chat, ask for two actions that need approval of the same tool (two commits, say). Deny the first, approve the second. Badges should read Cancelled, then Approved. Repeat the other way round.
- Start a flow from chat and approve it. The card should read Approved and the session card should show up.
MR acceptance checklist
Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.