feat(adapters): native hooks for Anthropic, Semantic Kernel, smolagents, PydanticAI#1593
feat(adapters): native hooks for Anthropic, Semantic Kernel, smolagents, PydanticAI#1593miyannishar wants to merge 1 commit into
Conversation
…ticAI
Complete the native-hooks migration for all four remaining adapters:
Anthropic:
- Add GovernanceMessageHook + as_message_hook() factory
- Pre-execution: content scanning, tool allowlist, token limits
- Post-execution: tool_use validation, token tracking, audit
- Deprecate wrap() and wrap_client() with migration guidance
Semantic Kernel:
- Add GovernanceFunctionFilter + as_filter() factory
- Uses SK's native add_filter('auto_function_invocation', ...) system
- Validates function names, blocked patterns, call counts
- Deprecate wrap() and wrap_kernel() with migration guidance
Smolagents:
- Add GovernanceStepCallback + as_step_callback() factory
- Implements step_callbacks protocol: __call__(step, agent)
- Validates tool names, blocked patterns, observations
- Deprecate wrap() with migration guidance
PydanticAI:
- Add GovernanceCapability + as_capability() factory
- Lifecycle hooks: before/after_run, before/after_tool_execute
- Pre-execution policy gating, post-execution drift detection
- Deprecate wrap() with migration guidance
Package exports:
- Export AnthropicGovernanceHook, SKGovernanceFilter,
SmolagentsGovernanceCallback, PydanticAIGovernanceCapability
from integrations __init__.py
Tests:
- test_anthropic_hooks.py: 12 tests
- test_semantic_kernel_hooks.py: 10 tests
- test_smolagents_hooks.py: 14 tests
- test_pydantic_ai_hooks.py: 16 tests
Part of: microsoft#1571
🤖 AI Agent: security-scanner — View detailsNo security issues found. |
🤖 AI Agent: docs-sync-checker — Docs SyncDocs Sync
|
🤖 AI Agent: breaking-change-detector — API CompatibilityAPI Compatibility
|
🤖 AI Agent: test-generator — `anthropic_adapter.py`Test Coverage Analysis
|
🤖 AI Agent: code-reviewer — Feedback on Pull Request: feat(adapters): native hooks for Anthropic, Semantic Kernel, smolagents, PydanticAIFeedback on Pull Request: feat(adapters): native hooks for Anthropic, Semantic Kernel, smolagents, PydanticAICRITICAL Issues
WARNING Issues
SUGGESTIONS
Summary of Recommendations
This PR introduces a significant improvement in governance integration by adopting native hooks, but careful attention to the identified issues will ensure robustness, security, and user adoption. |
PR Review Summary
Verdict: ❌ Changes needed |
imran-siddique
left a comment
There was a problem hiding this comment.
Code Review: Anthropic, Semantic Kernel, smolagents, PydanticAI native hooks
Impressive scope covering 4 adapters in one PR, @miyannishar. Architecture is solid across all four. Issues found:
Blocking
1. contextlib.suppress(Exception) silently swallows ALL errors
In both semantic_kernel_adapter.py (wrap_kernel()) and pydantic_ai_adapter.py (wrap()), the pattern:
\\python
with contextlib.suppress(Exception), warnings.catch_warnings():
warnings.simplefilter('ignore', DeprecationWarning)
return wrapper.wrap(kernel)
\
catches all exceptions, not just deprecation warnings. If wrap() raises a real error (invalid kernel, policy validation), it's silently eaten and returns None. Remove contextlib.suppress(Exception) and keep only warnings.catch_warnings().
2. Anthropic wrap_client() double-emits deprecation
Unlike SK and PydanticAI which suppress the nested warning, Anthropic doesn't. Users see two warnings per call.
Security/Correctness
3. Session ID collisions
GovernanceMessageHook and GovernanceFunctionFilter use int(time.time()) for session IDs. Two hooks created in the same second share a session ID, corrupting audit trails. Use uuid.uuid4().hex[:12] instead.
4. Repeated as_filter()/as_message_hook() calls overwrite context entries
GovernanceFunctionFilter hardcodes wrapper._contexts['sk-filter']. Calling as_filter() twice (as shown in the docstring example for both filter types) overwrites the first filter's context.
5. Smolagents test will fail at runtime
test_blocks_pattern_in_observation passes a step with observation='Result: rm -rf / completed' to a callback with blocked_patterns=['rm -rf']. But GovernanceStepCallback scans observations unconditionally, so this will raise PolicyViolationError before reaching the assertion. Fix either the test expectation or the implementation.
Warnings
6. Post-execution token limit check is detective, not preventive
In Anthropic GovernanceMessageHook.create(), the token check runs after client.messages.create() returns. Tokens are already billed. Document this limitation.
7. Deleted # Google Gemini section comment in __init__.py
Gemini entries now appear under the Anthropic section header. Restore the comment.
Nits
- PydanticAI
GovernanceCapabilitymaintains redundant dual counters (_tool_call_countand_ctx.call_count) - SK tests use deprecated
asyncio.get_event_loop().run_until_complete()
The contextlib.suppress(Exception) issue (#1) is the main blocker since it can mask real failures silently.
imran-siddique
left a comment
There was a problem hiding this comment.
Updated review (condensed):
TL;DR: 1 blocker (error suppression), 1 test bug, rest are nice-to-haves.
| # | Sev | Issue | Where |
|---|---|---|---|
| 1 | Block | contextlib.suppress(Exception) swallows ALL errors, not just deprecation warnings |
SK wrap_kernel(), PydanticAI wrap() |
| 2 | Bug | smolagents test_blocks_pattern_in_observation will raise before reaching assertion |
test file |
| 3 | Warn | Anthropic wrap_client() double-emits deprecation (SK/PydanticAI suppress it correctly) |
wrap_client() |
| 4 | Warn | Session IDs use int(time.time()) -- collide within same second |
Anthropic, SK |
| 5 | Warn | Repeated as_filter() calls overwrite context entries |
SK GovernanceFunctionFilter |
#1: Remove contextlib.suppress(Exception), keep only warnings.catch_warnings().
#2: Either the test expectation is wrong or observation scanning should skip when no tool calls.
#3-5 are fine as follow-ups.
imran-siddique
left a comment
There was a problem hiding this comment.
Approving native hooks migration for remaining adapters.
|
Closing - rebased and re-opened as a new PR to resolve merge conflicts with recently merged native-hooks PRs. |
Blockers: - Remove contextlib.suppress(Exception) in SK wrap_kernel() and PydanticAI wrap() -- was silently swallowing real errors, not just DeprecationWarning. Use warnings.catch_warnings() only. - Fix Anthropic wrap_client() double-emitting deprecation by wrapping the inner kernel.wrap() call with warnings.catch_warnings(). - Fix smolagents test_blocks_pattern_in_observation: the fixture kernel has blocked_patterns=[rm -rf] so observation scanning fires even without a tool call. Corrected both sub-cases to expect raises. Warnings: - Replace int(time.time()) session IDs with uuid.uuid4().hex[:12] to eliminate same-second collision and audit trail corruption. - Make GovernanceFunctionFilter context key unique per instance (was hardcoded 'sk-filter'), preventing context overwrites when as_filter() is called multiple times on the same wrapper.
Summary
Complete the native-hooks migration for all four remaining agent framework adapters: Anthropic, Semantic Kernel, smolagents, and PydanticAI. Each adapter now exposes a factory method that returns a native framework hook — replacing the legacy monkey-patching
wrap()pattern with a composable, non-invasive integration path.Related to #1571
Changes
Anthropic (
anthropic_adapter.py)GovernanceMessageHook— stateless hook implementing acreate()method that wrapsclient.messages.create()tool_useblock validation, token tracking, audit trailAnthropicKernel.as_message_hook()— factory method (recommended entry point)wrap()/wrap_client()— deprecated withDeprecationWarning, directing toas_message_hook()Semantic Kernel (
semantic_kernel_adapter.py)GovernanceFunctionFilter— async callable conforming to SK'sadd_filter("auto_function_invocation", ...)protocolmax_tool_callsenforcementSemanticKernelWrapper.as_filter()— factory method (recommended entry point)wrap()/wrap_kernel()— deprecated withDeprecationWarning, directing toas_filter()Smolagents (
smolagents_adapter.py)GovernanceStepCallback— callable implementing__call__(step, agent)for smolagents' `step_callbacks``max_tool_callsenforcementSmolagentsKernel.as_step_callback()— factory method (recommended entry point)wrap()— deprecated withDeprecationWarning, directing toas_step_callback()PydanticAI (
pydantic_ai_adapter.py)GovernanceCapability— capability implementing lifecycle hooks:before_run: prompt content scanningbefore_tool_execute: tool allowlist, blocked patterns, call-count limitsafter_tool_execute: audit recordingafter_run: completion recordingPydanticAIKernel.as_capability()— factory method (recommended entry point)wrap()— deprecated withDeprecationWarning, directing toas_capability()Package exports (
__init__.py)AnthropicGovernanceHook,SKGovernanceFilter,SmolagentsGovernanceCallback,PydanticAIGovernanceCapability,SmolagentsKernelMigration Guide
Before (legacy monkey-patching):
After (native hooks — recommended):
Tests
test_anthropic_hooks.pytest_semantic_kernel_hooks.pytest_smolagents_hooks.pytest_pydantic_ai_hooks.pyDesign Decisions
wrap()methods still work but emitDeprecationWarningwith clear migration instructions.