Merge pull request #216 from zts212653/fix/issue-111-leftover-p1-p2

fix(tool): resolve AFTER_TOOL.approved semantic pollution

Author: bouillipx

dare_framework/agent/_internal/tool_executor.py

@@ -117,7 +117,11 @@ async def run_tool_loop(
                     "attempt": attempts,
                     "success": False,
                     "error": policy_error,
-                    "approved": True,
+                    "approved": False,
+                    "policy_decision": (
+                        "hook_ask" if before_tool_dispatch.decision is HookDecision.ASK
+                        else "hook_block"
+                    ),
                     "evidence_collected": False,
                     "duration_ms": (time.perf_counter() - tool_start) * 1000.0,
                     "budget_stats": agent._budget_stats(),
@@ -347,15 +351,16 @@ async def approval_observer(payload: dict[str, Any]) -> None:
             if hasattr(result, "success") and not result.success:
                 tool_success = False
             evidence_collected = bool(getattr(result, "evidence", []))
-            denied_status = "fail"
             denied_output = getattr(result, "output", {})
-            approved = True
-            if not tool_success:
-                if isinstance(denied_output, dict):
-                    candidate_status = denied_output.get("status")
-                    if isinstance(candidate_status, str) and candidate_status:
-                        denied_status = candidate_status
-                approved = denied_status != "not_allow"
+            # Derive "approved" from both the pre-invocation policy decision
+            # AND the runtime approval outcome.  When policy_decision is
+            # "approve_required", the GovernedToolGateway may still deny at
+            # runtime (returning output.status="not_allow"), so we must also
+            # check the tool result for an explicit denial signal.
+            approved = policy_decision != PolicyDecision.DENY.value
+            if not tool_success and isinstance(denied_output, dict):
+                if denied_output.get("status") == "not_allow":
+                    approved = False
             await agent._emit_hook(
                 HookPhase.AFTER_TOOL,
                 {
@@ -374,9 +379,14 @@ async def approval_observer(payload: dict[str, Any]) -> None:
             )
 
             if not tool_success:
+                tool_fail_status = "fail"
+                if isinstance(denied_output, dict):
+                    candidate = denied_output.get("status")
+                    if isinstance(candidate, str) and candidate:
+                        tool_fail_status = candidate
                 return {
                     "success": False,
-                    "status": denied_status,
+                    "status": tool_fail_status,
                     "error": result.error or "tool failed",
                     "output": denied_output,
                     "result": result,
@@ -421,14 +431,13 @@ async def approval_observer(payload: dict[str, Any]) -> None:
                     "policy_decision": policy_decision,
                 },
             )
-            approved = True
-            try:
-                from dare_framework.tool.exceptions import HumanApprovalRequired
+            # Derive approved from the policy decision made before execution,
+            # not from the exception type.  Infra errors do not revoke approval.
+            approved = policy_decision != PolicyDecision.DENY.value
+            from dare_framework.tool.exceptions import HumanApprovalRequired
 
-                if isinstance(exc, HumanApprovalRequired):
-                    approved = False
-            except Exception:
-                approved = True
+            if isinstance(exc, HumanApprovalRequired):
+                approved = False
             await agent._emit_hook(
                 HookPhase.AFTER_TOOL,
                 {

dare_framework/agent/dare_agent.py

@@ -715,6 +715,7 @@ async def _emit_after_tool(
             error: str | None,
             approved: bool,
             evidence_collected: bool,
+            policy_decision: str | None = None,
         ) -> None:
             await self._emit_hook(
                 HookPhase.AFTER_TOOL,
@@ -726,6 +727,7 @@ async def _emit_after_tool(
                     "success": success,
                     "error": error,
                     "approved": approved,
+                    "policy_decision": policy_decision,
                     "evidence_collected": evidence_collected,
                     "duration_ms": (time.perf_counter() - tool_start) * 1000.0,
                     "budget_stats": self._budget_stats(),
@@ -807,8 +809,12 @@ async def _emit_after_tool(
             await _emit_after_tool(
                 success=False,
                 error=policy_error,
-                approved=True,
+                approved=False,
                 evidence_collected=False,
+                policy_decision=(
+                    "hook_ask" if before_tool_dispatch.decision is HookDecision.ASK
+                    else "hook_block"
+                ),
             )
             return StepResult(
                 step_id=step.step_id,
@@ -868,6 +874,7 @@ async def _emit_after_tool(
                 error=error_text,
                 approved=True,
                 evidence_collected=False,
+                policy_decision="allow",
             )
             return StepResult(
                 step_id=step.step_id,
@@ -885,6 +892,7 @@ async def _emit_after_tool(
                 error=result_error,
                 approved=True,
                 evidence_collected=bool(result.evidence),
+                policy_decision="allow",
             )
             return result
 
@@ -894,6 +902,7 @@ async def _emit_after_tool(
             error=invalid_error,
             approved=True,
             evidence_collected=False,
+            policy_decision="allow",
         )
         return StepResult(
             step_id=step.step_id,

dare_framework/observability/_internal/tracing_hook.py

@@ -371,6 +371,7 @@ async def _on_after_tool(self, payload: dict[str, Any]) -> None:
         success = bool(payload.get("success", False))
         error = payload.get("error")
         approved = payload.get("approved", True)
+        policy_decision = payload.get("policy_decision")
         evidence_collected = payload.get("evidence_collected", False)
 
         span = self._end_span(
@@ -382,6 +383,8 @@ async def _on_after_tool(self, payload: dict[str, Any]) -> None:
                 span.set_attribute("success", success)
                 span.set_attribute(DAREAttributes.TOOL_APPROVED, approved)
                 span.set_attribute(DAREAttributes.TOOL_EVIDENCE_COLLECTED, evidence_collected)
+                if policy_decision is not None:
+                    span.set_attribute("dare.tool.policy_decision", policy_decision)
                 if error:
                     span.set_attribute(GenAIAttributes.ERROR_TYPE, type(error).__name__)
                     span.set_attribute(GenAIAttributes.ERROR_MESSAGE, str(error))

dare_framework/transport/types.py

@@ -91,6 +91,12 @@ class ActionPayload(EnvelopePayload):
     code: str | None = None
     reason: str | None = None
 
+    def __post_init__(self) -> None:
+        if not isinstance(self.resource_action, str):
+            raise TypeError(
+                f"invalid resource_action type: {type(self.resource_action).__name__}"
+            )
+
 
 @dataclass(frozen=True)
 class ControlPayload(EnvelopePayload):