fix(approval): surface pending-approval state with explicit marker vi…#28000
fix(approval): surface pending-approval state with explicit marker vi…#28000LifeJiggy wants to merge 1 commit into
Conversation
…sible to LLM When a tool call requires user approval in the non-blocking gateway path, the LLM previously received a result that was indistinguishable from a failed tool call (exit_code=-1, error=message). The LLM could not tell whether the tool was pending approval, had returned empty results, or had failed silently — causing it to burn context on wrong hypotheses. Fix changes the result format to include: - status: pending_approval (clear state name) - approval_pending: True (explicit boolean for LLMs to detect) - error: cleared to empty string (removes misleading error signal) This lets the LLM reason about approval latency vs actual errors, short-circuiting the previous silent failure mode. Fixes NousResearch#14806
|
BoardJames triage: this looks shared/systemic rather than branch-local. All non-test checks are green (including both Docker builds); the only blocker is the full |
outsourc-e
left a comment
There was a problem hiding this comment.
This is not merge-ready as submitted.
The new pending_approval status breaks an existing fallback contract: tests/gateway/test_approve_deny_commands.py::TestFallbackNoCallback::test_no_callback_returns_approval_required now fails because the no-callback path used to return approval_required and now returns pending_approval.
Local repro:
scripts/run_tests.sh tests/gateway/test_approve_deny_commands.py -k "approval_required or callback" tests/tools/test_cron_approval_mode.py- Fails on
assert result.get("status") == "approval_required"
So either the compatibility path needs to stay approval_required, or the broader gateway/tests/callers need to be updated in a coordinated change.
What does this PR do?
When a tool requires approval (e.g., write_file, patch on sensitive paths), the approval prompt fires asynchronously but the tool result returned to the LLM is indistinguishable from an empty success result. The LLM proceeds as if the action completed, leading to incorrect assumptions about filesystem state.
Add an explicit pending-approval marker in the tool result so the LLM can clearly see that the operation is awaiting user confirmation and has not yet taken effect.
Related Issue
Fixes #14806
Type of Change
Changes Made
How to Test