feat(gateway): add contextual rationale to approval prompts#27833
feat(gateway): add contextual rationale to approval prompts#27833zccyman wants to merge 1 commit into
Conversation
Include the agent's last assistant text as contextual_reason in dangerous command approval prompts across all gateway adapters. When the agent requests approval for a dangerous command, the user now sees the agent's reasoning (e.g. 'I need to free disk space') above the command preview, giving context for the approval decision. Changes: - gateway/run.py: capture last assistant text in closure, pass to adapter send_exec_approval as contextual_reason - All 7 adapters (Telegram, Feishu, Discord, Slack, Matrix, QQBot, Teams): accept contextual_reason kwarg, render in approval UI - Graceful degradation: no rationale shown when interim messages disabled or assistant produced no text before tool call Closes NousResearch#27604
Bartok9
left a comment
There was a problem hiding this comment.
Solid implementation. A few observations:
Correctness — The _last_assistant_rationale closure captures the most recent non-empty text from _interim_assistant_cb, which is the right signal. One subtle case: if the agent emits a large streaming blob and the rationale is the first segment rather than the last, only the most recent chunk is preserved. In practice this is usually fine (the final reasoning summary is typically at the end), but worth a comment.
QQBot path — The QQBot adapter sends the rationale as a separate preceding message (await self.send(...)) rather than embedding it in the approval card. This is pragmatically correct (QQBot's card format is more rigid), but it creates a minor race: if the approval card arrives before the rationale message on slow connections. Consider awaiting self.send before sending the approval request (it looks like it already does — just confirming the send order is preserved).
Telegram HTML escape — Does contextual_reason get HTML-escaped before injection into Telegram's parse_mode=HTML messages? Worth confirming the Telegram path uses html.escape(contextual_reason) for the HTML-mode render path to avoid accidental tag injection from streaming text.
Tests — 8 tests covering all 7 adapters is good coverage. Would add one edge-case: rationale that itself contains Markdown/HTML special characters (backticks, <, >) to exercise the escaping paths.
Overall this is a clean, useful feature — good work.
|
Solid implementation. A few observations: Correctness — The QQBot path — The QQBot adapter sends the rationale as a separate preceding message ( Telegram HTML escape — Confirm Tests — 8 tests covering all 7 adapters is good coverage. One useful edge-case to add: rationale containing Markdown/HTML special characters (backticks, Overall this is a clean, useful feature — good work. |
|
Thanks for the thorough review, @Bartok9! Streaming rationale ordering: Good catch — the closure captures the most recent non-empty chunk, which is indeed the final reasoning summary in the common case. I'll add a docstring comment clarifying this behavior for future maintainers. QQBot race condition: You're right that the separate Both points are non-blocking observations — appreciate the detailed analysis! |
Summary
Include the agent's last assistant text as contextual rationale in dangerous command approval prompts across all gateway adapters.
When the agent requests approval for a dangerous command (e.g.
rm -rf), the user now sees the agent's reasoning (e.g. "I need to free disk space before the build") above the command preview, giving context for the approval decision.Closes #27604
Changes
gateway/run.py: Capture last assistant text in_interim_assistant_cbclosure, pass to adaptersend_exec_approvalascontextual_reasoncontextual_reason: str = ""kwarg, render in platform-native approval UITest Plan
tests/gateway/test_approval_contextual_rationale.pyScreenshots (conceptual)
Before:
After: