Skip to content

Commit 13eeb3e

Browse files
committed
fix(beta): adapt ToolPolicyMiddleware to new ToolResult signature
ToolResult no longer accepts 'content' keyword; the constructor is now: ToolResult(*inputs, parts=(), final=False, metadata=None) _make_tool_error simplified to one line via ToolErrorEvent.from_call: return ToolErrorEvent.from_call(event, PermissionError(reason)) Tests updated: .content assertions replaced with isinstance(error_event.error, PermissionError) + str(error_event.error) checks, since ToolErrorEvent no longer carries a .content property. Removed unused ToolResult import from tool_policy.py. Addresses marklysze's CI-staleness review on #2533.
1 parent a555023 commit 13eeb3e

2 files changed

Lines changed: 8 additions & 16 deletions

File tree

autogen/beta/middleware/builtin/tool_policy.py

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
from autogen.beta.annotations import Context
1212
from autogen.beta.events import BaseEvent, ToolCallEvent, ToolErrorEvent
1313
from autogen.beta.middleware.base import BaseMiddleware, MiddlewareFactory, ToolExecution, ToolResultType
14-
from autogen.beta.tools import ToolResult
1514

1615
OnBlockedCallback = Callable[[ToolCallEvent, str], None]
1716

@@ -38,16 +37,8 @@ def __post_init__(self) -> None:
3837

3938

4039
def _make_tool_error(event: ToolCallEvent, reason: str) -> ToolErrorEvent:
41-
"""Build a ToolErrorEvent with an explicit content string."""
42-
err = ToolErrorEvent(
43-
parent_id=event.id,
44-
name=event.name,
45-
result=ToolResult(content=None),
46-
error=PermissionError(reason),
47-
)
48-
# Override content so it shows the reason, not a traceback.
49-
err.content = reason
50-
return err
40+
"""Build a ToolErrorEvent that carries the denial reason as a PermissionError."""
41+
return ToolErrorEvent.from_call(event, PermissionError(reason))
5142

5243

5344
class _ToolPolicy:

test/beta/middleware/builtins/test_tool_policy_middleware.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -150,17 +150,17 @@ def test_blocklist_overrides_allowlist(self) -> None:
150150
assert not allowed
151151
assert "blocked" in reason
152152

153-
def test_tool_error_content_matches_reason(self) -> None:
153+
def test_tool_error_reason_carried_as_permission_error(self) -> None:
154154
# Given a tool call that will be blocked
155155
event = _make_tool_call(name="delete_all")
156156
reason = "tool 'delete_all' is blocked"
157157

158158
# When building the error event
159159
error_event = _make_tool_error(event, reason)
160160

161-
# Then the content equals the reason string, not "NoneType: None"
162-
assert error_event.content == reason
163-
assert "NoneType" not in error_event.content
161+
# Then the error is a PermissionError whose message equals the denial reason
162+
assert isinstance(error_event.error, PermissionError)
163+
assert str(error_event.error) == reason
164164

165165

166166
# ---------------------------------------------------------------------------
@@ -236,7 +236,8 @@ async def call_next(event: ToolCallEvent, context: mock.MagicMock) -> ToolResult
236236

237237
# Then enforcement still returns the policy denial
238238
assert isinstance(result, ToolErrorEvent)
239-
assert result.content == "tool 'bad_tool' is blocked"
239+
assert isinstance(result.error, PermissionError)
240+
assert str(result.error) == "tool 'bad_tool' is blocked"
240241

241242
@pytest.mark.asyncio()
242243
async def test_on_blocked_not_called_when_allowed(self) -> None:

0 commit comments

Comments
 (0)