From 76702572efe34d92a4ee7280a207ca7bfc848599 Mon Sep 17 00:00:00 2001
From: Aditya Singh <60082699+adityasingh2400@users.noreply.github.com>
Date: Fri, 8 May 2026 10:37:35 -0700
Subject: [PATCH] fix: skip needs_approval_checker when status already resolved
 (#3229)

---
 src/agents/run_internal/tool_planning.py | 17 +++----
 tests/test_hitl_error_scenarios.py       | 58 ++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 8 deletions(-)

diff --git a/src/agents/run_internal/tool_planning.py b/src/agents/run_internal/tool_planning.py
index 56a0654a..9d2acc41 100644
--- a/src/agents/run_internal/tool_planning.py
+++ b/src/agents/run_internal/tool_planning.py
@@ -514,8 +514,6 @@ async def _select_function_tool_runs_for_resume(
             existing_pending=approval_items_by_call_id.get(call_id),
         )
 
-        requires_approval = await needs_approval_checker(run)
-
         if approval_status is False:
             await record_rejection(call_id, run.tool_call, run.function_tool)
             continue
@@ -524,16 +522,19 @@ async def _select_function_tool_runs_for_resume(
             selected.append(run)
             continue
 
+        # Only invoke needs_approval_checker when the approval state is unresolved;
+        # for explicit approve/reject decisions the checker's result is unused, and
+        # invoking it eagerly risks user-side effects (or exceptions that swallow
+        # rejections) on calls whose outcome is already determined.
+        requires_approval = await needs_approval_checker(run)
+
         if not requires_approval:
             selected.append(run)
             continue
 
-        if approval_status is None:
-            pending_interruption_adder(
-                approval_items_by_call_id.get(run.tool_call.call_id) or pending_item_builder(run)
-            )
-            continue
-        selected.append(run)
+        pending_interruption_adder(
+            approval_items_by_call_id.get(run.tool_call.call_id) or pending_item_builder(run)
+        )
 
     return selected
 
diff --git a/tests/test_hitl_error_scenarios.py b/tests/test_hitl_error_scenarios.py
index f049c61f..8b92f0fc 100644
--- a/tests/test_hitl_error_scenarios.py
+++ b/tests/test_hitl_error_scenarios.py
@@ -1247,6 +1247,64 @@ async def test_resume_honors_permanent_namespaced_function_approval_with_new_cal
     assert rejections == []
 
 
+@pytest.mark.asyncio
+async def test_resume_skips_needs_approval_checker_when_status_resolved() -> None:
+    """Resolved approve/reject decisions must short-circuit needs_approval_checker.
+
+    A user-supplied checker may have side effects (telemetry, network, exceptions).
+    When the approval status is already True or False, we must not invoke it.
+    """
+
+    @function_tool(needs_approval=True)
+    async def approve_me(value: str) -> str:
+        return value
+
+    approved_call = make_function_tool_call(
+        approve_me.name, call_id="approved-call", arguments='{"value":"a"}'
+    )
+    rejected_call = make_function_tool_call(
+        approve_me.name, call_id="rejected-call", arguments='{"value":"b"}'
+    )
+    agent = Agent(name="agent")
+    context_wrapper = make_context_wrapper()
+    context_wrapper.approve_tool(ToolApprovalItem(agent=agent, raw_item=approved_call))
+    context_wrapper.reject_tool(ToolApprovalItem(agent=agent, raw_item=rejected_call))
+
+    runs = [
+        ToolRunFunction(tool_call=approved_call, function_tool=approve_me),
+        ToolRunFunction(tool_call=rejected_call, function_tool=approve_me),
+    ]
+    checker_calls: list[str] = []
+
+    async def _needs_approval_checker(run: ToolRunFunction) -> bool:
+        checker_calls.append(run.tool_call.call_id)
+        raise AssertionError("checker must not run for resolved approvals")
+
+    rejections: list[str | None] = []
+
+    async def _record_rejection(
+        call_id: str | None,
+        _tool_call: ResponseFunctionToolCall,
+        _tool: Any,
+    ) -> None:
+        rejections.append(call_id)
+
+    selected = await _select_function_tool_runs_for_resume(
+        runs,
+        approval_items_by_call_id={},
+        context_wrapper=context_wrapper,
+        needs_approval_checker=_needs_approval_checker,
+        output_exists_checker=lambda _run: False,
+        record_rejection=_record_rejection,
+        pending_interruption_adder=lambda _item: None,
+        pending_item_builder=lambda run: ToolApprovalItem(agent=agent, raw_item=run.tool_call),
+    )
+
+    assert checker_calls == []
+    assert [run.tool_call.call_id for run in selected] == ["approved-call"]
+    assert rejections == ["rejected-call"]
+
+
 @pytest.mark.asyncio
 async def test_resume_rebuilds_function_runs_from_object_approvals() -> None:
     """Rebuild should handle ResponseFunctionToolCall approval items."""