Skip to content

docs: add docstrings to Python adapter modules#664

Open
kanish5 wants to merge 1 commit intomicrosoft:mainfrom
kanish5:docs/add-adapter-docstrings
Open

docs: add docstrings to Python adapter modules#664
kanish5 wants to merge 1 commit intomicrosoft:mainfrom
kanish5:docs/add-adapter-docstrings

Conversation

@kanish5
Copy link
Copy Markdown
Contributor

@kanish5 kanish5 commented Apr 1, 2026

Closes #506

Summary

Added missing module-level and function-level docstrings to all Python
adapter modules as specified in the issue.

Files changed

  • crewai_adapter.py — 5 docstrings added (governed_run, governed_execute, governed_step, governed_save, governed_delegate)
  • smolagents_adapter.py — 8 docstrings added (governed_forward, _default_violation_handler, _record, _check_tool_allowed, _check_content, _check_timeout, _check_budget, _raise_violation)
  • pydantic_ai_adapter.py — 3 docstrings added (governed_fn, original, context)
  • google_adk_adapter.py — 7 docstrings added (_default_violation_handler, _record, _check_tool_allowed, _check_content, _check_timeout, _check_budget, _raise_violation)
  • guardrails_adapter.py — 10 docstrings added (to_dict x2, failed_validators, name x3, validate x3, _default_violation_handler)

Verification

All 5 files pass ast.parse() with zero missing public docstrings.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 1, 2026

Welcome to the Agent Governance Toolkit! Thanks for your first pull request.
Please ensure tests pass, code follows style (ruff check), and you have signed the CLA.
See our Contributing Guide.

@github-actions github-actions bot added the size/XL Extra large PR (500+ lines) label Apr 1, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 1, 2026

🤖 AI Agent: breaking-change-detector — Summary

🔍 API Compatibility Report

Summary

This pull request adds docstrings to various Python adapter modules in the microsoft/agent-governance-toolkit repository. The changes are purely additive and do not modify any existing functionality, signatures, or behavior. No breaking changes were introduced.

Findings

Severity Package Change Impact
🔵 agent-os Added docstrings to public functions and methods across multiple adapter modules Improves documentation, no impact on API compatibility

Migration Guide

✅ No migration steps are required as there are no breaking changes.

Notes

  • The added docstrings enhance code readability and provide valuable context for developers using the library.
  • These changes are purely additive and do not alter the runtime behavior of the code.

No breaking changes found.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 1, 2026

🤖 AI Agent: docs-sync-checker — Issues Found

📝 Documentation Sync Report

Issues Found

  • ⚠️ CHANGELOG.md — no entry for this change.

Suggestions

  • 💡 Add an entry to CHANGELOG.md summarizing the addition of docstrings to the Python adapter modules. For example:
    - Added comprehensive docstrings to all public functions and methods in the following adapter modules:
      - `crewai_adapter.py`
      - `smolagents_adapter.py`
      - `pydantic_ai_adapter.py`
      - `google_adk_adapter.py`
      - `guardrails_adapter.py`
    

Notes

  1. New public APIs without docstrings: No issues found. All public functions, classes, and methods in the modified files now have appropriate docstrings.
  2. README sections out of date: No changes to functionality were introduced in this PR, so no updates to the README files are necessary.
  3. Example code outdated: No API signatures were changed, so no updates to example code are required.
  4. Type hints: All public APIs in the modified files include complete type annotations.

✅ Documentation is almost in sync, except for the missing CHANGELOG.md entry.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 1, 2026

🤖 AI Agent: security-scanner — Security Review of Pull Request: `docs: add docstrings to Python adapter modules`

Security Review of Pull Request: docs: add docstrings to Python adapter modules

This pull request primarily adds docstrings to various Python adapter modules in the microsoft/agent-governance-toolkit repository. The changes are documentation-focused and do not introduce new functionality or modify existing logic. However, given the critical nature of this repository, a thorough review is warranted to ensure that no security vulnerabilities are introduced inadvertently.


Analysis of Potential Security Issues

1. Prompt Injection Defense Bypass

  • Risk: No new code has been introduced that could directly impact prompt injection defenses. The changes are limited to docstrings and do not modify the logic of existing functions.
  • Rating: 🔵 LOW
  • Recommendation: No action required.

2. Policy Engine Circumvention

  • Risk: The docstrings describe the behavior of policy enforcement mechanisms (e.g., governed_run, governed_execute, _check_tool_allowed, _check_content, etc.). While the documentation does not alter the implementation, it is important to ensure that the described behavior matches the actual implementation.
  • Rating: 🔵 LOW
  • Recommendation: Perform a separate review of the implementation to confirm that the described behavior aligns with the actual policy enforcement logic.

3. Trust Chain Weaknesses

  • Risk: No changes to trust chain mechanisms (e.g., SPIFFE/SVID validation, certificate pinning) are present in this PR.
  • Rating: 🔵 LOW
  • Recommendation: No action required.

4. Credential Exposure

  • Risk: No sensitive information (e.g., credentials, secrets) is exposed in the added docstrings.
  • Rating: 🔵 LOW
  • Recommendation: No action required.

5. Sandbox Escape

  • Risk: No changes to sandboxing or process isolation mechanisms are introduced in this PR.
  • Rating: 🔵 LOW
  • Recommendation: No action required.

6. Deserialization Attacks

  • Risk: No deserialization logic is introduced or modified in this PR.
  • Rating: 🔵 LOW
  • Recommendation: No action required.

7. Race Conditions

  • Risk: No changes to concurrency or time-of-check-to-time-of-use (TOCTOU) mechanisms are introduced in this PR.
  • Rating: 🔵 LOW
  • Recommendation: No action required.

8. Supply Chain

  • Risk: No new dependencies are introduced in this PR, so there is no risk of dependency confusion or typosquatting.
  • Rating: 🔵 LOW
  • Recommendation: No action required.

General Observations

  1. Accuracy of Docstrings: The added docstrings appear to accurately describe the behavior of the functions they document. However, it is recommended to verify that the implementation matches the described behavior in a separate review.
  2. Improved Documentation: The added docstrings improve the readability and maintainability of the codebase, which is beneficial for future development and security reviews.

Conclusion

This pull request does not introduce any new security vulnerabilities. The changes are limited to documentation and do not modify the underlying logic or introduce new functionality. The overall risk is LOW.

Recommendations

  1. Approve and merge this pull request as it improves the documentation quality without introducing security risks.
  2. Conduct a separate review of the implementation to ensure that the described behavior in the docstrings matches the actual logic, particularly for policy enforcement functions.

If you have any additional questions or concerns, feel free to ask!

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 AI Agent: code-reviewer

Pull Request Review for microsoft/agent-governance-toolkit


Summary

This pull request primarily adds docstrings to Python adapter modules and introduces three new Jupyter notebooks for interactive demonstrations of the toolkit's capabilities. While the docstring additions improve code documentation, the notebooks provide hands-on examples for users to understand policy enforcement, MCP security, and multi-agent governance.


Key Areas of Review

1. Policy Engine Correctness

  • The notebooks demonstrate the policy engine's ability to block violations (e.g., SQL injection, destructive shell commands, and PII leakage). However, the implementation in the notebooks does not appear to validate the policy engine's behavior against edge cases or complex scenarios.
  • 🔴 CRITICAL: The GovernancePolicy in 01_policy_enforcement_101.ipynb uses simplistic regex patterns for blocking actions. Regex-based filtering is prone to bypasses, especially with obfuscated inputs or slight variations in syntax. For example:
    • DROP /* comment */ TABLE might bypass the DROP TABLE regex.
    • Unicode homoglyphs (e.g., DROP TABLE) could evade detection.
    • Suggestion: Use a more robust parsing mechanism (e.g., an SQL parser for SQL commands) instead of regex for critical policy enforcement.

2. Trust/Identity: Cryptographic Operations, Credential Handling, SPIFFE/SVID

  • The notebooks do not directly handle cryptographic operations or credentials. However, the MCP security scanner in 02_mcp_security_proxy.ipynb introduces a trust gate to block malicious tool definitions.
  • 💡 SUGGESTION: The trust_gate function could benefit from logging the findings to a secure audit trail for post-incident analysis. This would align with best practices for trust and identity management.

3. Sandbox Escape Vectors

  • The MCP tool poisoning examples in 02_mcp_security_proxy.ipynb highlight potential sandbox escape vectors (e.g., exec('/bin/sh'), sudo, and eval). The detection patterns are a good start but may not cover all possible escape vectors.
  • 💡 SUGGESTION: Expand the detection patterns to include additional sandbox escape techniques, such as:
    • File descriptor manipulation.
    • Environment variable poisoning.
    • Path traversal attacks.

4. Thread Safety in Concurrent Agent Execution

  • The notebooks do not demonstrate multi-threaded or concurrent execution of agents. However, the 03_multi_agent_governance.ipynb introduces metrics tracking for multiple agents.
  • 💡 SUGGESTION: If the toolkit supports concurrent agent execution, ensure that shared state (e.g., AgentMetrics) is thread-safe. Consider using thread-safe data structures or locks to avoid race conditions.

5. OWASP Agentic Top 10 Compliance

  • The notebooks address several OWASP Agentic Top 10 risks, including:
    • A1: Malicious Inputs: MCP poisoning detection.
    • A2: Unsafe Outputs: Blocking exfiltration patterns.
    • A4: Trust Boundary Violations: Trust gate implementation.
  • 🔴 CRITICAL: The notebooks do not address A6: Insecure Policy Enforcement comprehensively. For example:
    • The max_tool_calls enforcement in 01_policy_enforcement_101.ipynb relies on manually setting ctx.call_count. This is error-prone and could be bypassed if the context is not properly managed.
    • Suggestion: Implement a centralized mechanism to enforce call budgets and other policies, rather than relying on manual updates.

6. Type Safety and Pydantic Model Validation

  • The notebooks do not use Pydantic models for input validation. For example:
    • The GovernancePolicy and AgentSLO definitions could benefit from Pydantic validation to ensure that inputs (e.g., blocked_patterns, max_error_rate) conform to expected types and ranges.
  • 💡 SUGGESTION: Replace plain Python classes with Pydantic models where applicable to enforce type safety and validation.

7. Backward Compatibility

  • The changes in this PR do not introduce breaking changes to the public API. The added docstrings and notebooks are non-breaking enhancements.
  • 🟡 WARNING: If the regex patterns in GovernancePolicy are updated to use more robust parsing mechanisms, ensure backward compatibility for existing users who rely on the current behavior.

Actionable Feedback

🔴 CRITICAL

  1. Regex-based Policy Enforcement:

    • Replace regex-based policy enforcement with more robust parsing mechanisms to prevent bypasses.
    • Example: Use an SQL parser for SQL commands and a shell command parser for shell inputs.
  2. Insecure Policy Enforcement:

    • Centralize policy enforcement mechanisms (e.g., call budgets) to prevent manual errors and bypasses.

🟡 WARNING

  1. Backward Compatibility:
    • If regex patterns are replaced, provide a migration guide for users relying on the current behavior.

💡 SUGGESTIONS

  1. Audit Trail for Trust Gate:

    • Log findings from the trust_gate function to a secure audit trail for post-incident analysis.
  2. Expand Detection Patterns:

    • Add more patterns to detect sandbox escape techniques and other malicious behaviors.
  3. Thread Safety:

    • Ensure that shared state (e.g., AgentMetrics) is thread-safe if concurrent execution is supported.
  4. Pydantic Validation:

    • Use Pydantic models for GovernancePolicy, AgentSLO, and other configurations to enforce type safety and validation.

Conclusion

This pull request improves documentation and provides valuable interactive examples for users. However, critical issues in policy enforcement and sandbox escape detection need to be addressed to ensure the toolkit's security and reliability.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 1, 2026

🤖 AI Agent: test-generator — `crewai_adapter.py`

🧪 Test Coverage Analysis

The changes in this pull request are limited to adding docstrings to existing methods in the specified Python adapter modules. Since no functional changes have been made to the code, the test coverage analysis will focus on whether the existing tests adequately cover the methods for which docstrings were added. Below is the analysis for each file:


crewai_adapter.py

  • Existing coverage:

    • Methods like governed_run, governed_execute, governed_step, governed_save, and governed_delegate are likely covered by tests that validate policy enforcement, audit logging, and delegation depth checks.
    • These methods are central to the governance of CrewAI tools and agents, so they are expected to have test coverage for policy violations, audit logging, and delegation limits.
  • Missing coverage:

    • Edge cases for policy violations (e.g., bypass attempts, conflicting policies).
    • Scenarios with invalid or malformed inputs to governed_* methods.
    • Concurrency issues when multiple agents/tools are governed simultaneously.
  • 💡 Suggested test cases:

    1. test_governed_run_policy_bypass_attempt — Test if governed_run correctly detects and blocks attempts to bypass policy checks.
    2. test_governed_execute_conflicting_policies — Test governed_execute when multiple conflicting policies are applied.
    3. test_governed_save_malformed_input — Test governed_save with malformed or oversized payloads.
    4. test_governed_delegate_max_depth — Test governed_delegate when the delegation depth exceeds the policy-defined limit.
    5. test_concurrent_governed_operations — Test for race conditions or deadlocks when multiple agents/tools are governed simultaneously.

google_adk_adapter.py

  • Existing coverage:

    • Methods like _default_violation_handler, _record, _check_tool_allowed, _check_content, _check_timeout, _check_budget, and _raise_violation are likely covered by tests validating policy enforcement, audit logging, and budget/time constraints.
  • Missing coverage:

    • Boundary conditions for _check_budget (e.g., exactly at the budget limit).
    • Timeout edge cases in _check_timeout (e.g., near-expiry and expired scenarios).
    • Handling of invalid or malformed inputs in _check_content.
  • 💡 Suggested test cases:

    1. test_check_budget_boundary — Test _check_budget with a cost that exactly matches the remaining budget.
    2. test_check_timeout_near_expiry — Test _check_timeout when the timeout is about to expire.
    3. test_check_content_injection — Test _check_content with inputs containing injection attempts or unexpected characters.
    4. test_raise_violation_multiple_errors — Test _raise_violation when multiple violations occur in quick succession.

guardrails_adapter.py

  • Existing coverage:

    • Methods like to_dict, failed_validators, name, validate, and _default_violation_handler are likely covered by tests for validation outcomes, serialization, and violation handling.
  • Missing coverage:

    • Validation edge cases for validate methods (e.g., empty strings, extremely long strings, or strings with special characters).
    • Scenarios with multiple validators failing simultaneously.
    • Concurrency issues when multiple validations are performed in parallel.
  • 💡 Suggested test cases:

    1. test_regex_validator_empty_string — Test regex validator with an empty string.
    2. test_length_validator_oversized_input — Test length validator with a string that exceeds the maximum length by a large margin.
    3. test_keyword_validator_special_characters — Test keyword validator with inputs containing special characters.
    4. test_multiple_validators_failure — Test a scenario where multiple validators fail for the same input.
    5. test_concurrent_validations — Test for race conditions when multiple validations are performed simultaneously.

pydantic_ai_adapter.py

  • Existing coverage:

    • Methods like governed_fn, original, and context are likely covered by tests for PydanticAI tool governance and execution context tracking.
  • Missing coverage:

    • Edge cases for governed_fn (e.g., invalid arguments, policy violations).
    • Scenarios with nested or recursive tool calls.
  • 💡 Suggested test cases:

    1. test_governed_fn_invalid_arguments — Test governed_fn with invalid or malformed arguments.
    2. test_governed_fn_policy_violation — Test governed_fn when a policy violation occurs.
    3. test_governed_fn_nested_calls — Test governed_fn with nested or recursive tool calls.

smolagents_adapter.py

  • Existing coverage:

    • Methods like governed_forward, _default_violation_handler, _record, _check_tool_allowed, _check_content, _check_timeout, _check_budget, and _raise_violation are likely covered by tests for policy enforcement, audit logging, and budget/time constraints.
  • Missing coverage:

    • Edge cases for _check_tool_allowed (e.g., tools with similar names).
    • Boundary conditions for _check_budget and _check_timeout.
    • Handling of invalid or malformed inputs in _check_content.
  • 💡 Suggested test cases:

    1. test_check_tool_allowed_similar_names — Test _check_tool_allowed with tools that have similar names (e.g., "tool1" vs. "tool1a").
    2. test_check_budget_exact_limit — Test _check_budget with a cost that exactly matches the remaining budget.
    3. test_check_timeout_boundary — Test _check_timeout when the timeout is very close to expiring.
    4. test_check_content_injection — Test _check_content with inputs containing injection attempts or unexpected characters.
    5. test_governed_forward_policy_violation — Test governed_forward when a policy violation occurs.

Summary

While the changes in this PR are limited to docstrings, the analysis reveals potential gaps in test coverage for edge cases and concurrency scenarios across all five files. Implementing the suggested test cases would improve the robustness of the test suite and ensure comprehensive coverage of critical functionalities.

@kanish5 kanish5 force-pushed the docs/add-adapter-docstrings branch from b491d98 to 59967cb Compare April 1, 2026 00:49
@github-actions github-actions bot added the size/L Large PR (< 500 lines) label Apr 1, 2026
Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 AI Agent: code-reviewer

Review of Pull Request: docs: add docstrings to Python adapter modules

This pull request adds comprehensive docstrings to several Python adapter modules, improving code documentation and readability. While the changes are primarily documentation-related, they touch on critical areas such as policy enforcement, security, and governance mechanisms. Below is my review based on the focus areas:


🔴 CRITICAL: Security Issues

  1. Insufficient Documentation for Security-Sensitive Methods:

    • The docstrings for methods like _check_content, _check_tool_allowed, _check_budget, and _check_timeout in multiple adapters (e.g., google_adk_adapter.py, smolagents_adapter.py) do not explicitly mention the security implications of these checks. For example:
      • What happens if a blocked tool is allowed to execute due to a bug in _check_tool_allowed?
      • What are the risks of bypassing _check_content for sensitive data like PII?
    • Action: Update the docstrings to explicitly describe the security impact of these methods and the importance of their correctness.
  2. No Mention of Input Validation for Critical Methods:

    • Methods like governed_run, governed_execute, governed_step, and governed_delegate in crewai_adapter.py do not mention whether the input arguments (*args, **kwargs) are validated before being passed to the original methods.
    • Action: Add a note in the docstrings clarifying whether input validation is performed and, if not, explicitly state that the caller is responsible for ensuring input safety.

🟡 WARNING: Potential Breaking Changes

  1. Behavioral Changes in Wrappers:
    • While this PR is focused on documentation, the docstrings describe behavior that may not have been explicitly documented before. If the behavior described in the docstrings is incorrect or incomplete, it could lead to misunderstandings for developers relying on these descriptions.
    • Action: Verify that the described behavior in the docstrings matches the actual implementation. Any discrepancies should be corrected in either the code or the documentation.

💡 Suggestions for Improvement

  1. Consistency in Docstring Format:

    • While the docstrings generally follow a consistent format, there are minor inconsistencies in phrasing and structure. For example:
      • Some docstrings use "Returns:" while others use "Returns:" followed by a description of the return value.
      • Some docstrings use "Args:" while others use "Arguments:".
    • Action: Standardize the docstring format across all modules. Consider using a tool like pydocstyle or pydocstring to enforce consistency.
  2. Add Examples in Docstrings:

    • While the docstrings are descriptive, they could benefit from usage examples, especially for complex methods like governed_run and governed_delegate.
    • Action: Add examples of how to use these methods in the docstrings to improve developer understanding.
  3. Clarify "governed" Terminology:

    • The term "governed" is used extensively in the docstrings (e.g., "Governed wrapper around..."). While this is likely clear to the development team, it might not be immediately clear to new contributors or external developers.
    • Action: Add a brief explanation of what "governed" means in the context of this project, either in the module-level docstring or as a separate section in the documentation.
  4. Add Type Annotations for Arguments and Return Values:

    • While some methods include type annotations in the code, others do not. For example, methods like _check_tool_allowed and _check_content in google_adk_adapter.py and smolagents_adapter.py could benefit from explicit type annotations in the docstrings.
    • Action: Ensure all methods have consistent type annotations in both the code and the docstrings.
  5. Document Exceptions More Thoroughly:

    • While some methods mention PolicyViolationError in their docstrings, others do not. For example, _raise_violation in smolagents_adapter.py and google_adk_adapter.py should explicitly document that it raises a PolicyViolationError.
    • Action: Ensure all exceptions raised by a method are documented in the "Raises:" section of the docstring.
  6. Add Module-Level Docstrings:

    • While function-level docstrings have been added, module-level docstrings are missing in some files. These should provide an overview of the module's purpose and its key components.
    • Action: Add module-level docstrings to all updated files.
  7. Add Tests for Docstring Accuracy:

    • While the PR mentions that all files pass ast.parse(), there is no mention of tests to ensure the accuracy of the docstrings.
    • Action: Consider adding tests using a tool like pydocstyle or interrogate to ensure that all public methods and classes have accurate and complete docstrings.

General Observations

  • The added docstrings are well-written and provide a good level of detail about the purpose and behavior of each method.
  • The PR improves the overall maintainability and readability of the codebase, which is crucial for a security-focused library like this.

Summary of Recommendations

  1. 🔴 CRITICAL: Update docstrings to explicitly describe the security implications of methods that enforce policies or handle sensitive data.
  2. 🟡 WARNING: Verify that the described behavior in the docstrings matches the actual implementation to avoid potential breaking changes.
  3. 💡 Suggestions:
    • Standardize docstring format and add usage examples.
    • Clarify "governed" terminology in the documentation.
    • Add type annotations and document exceptions more thoroughly.
    • Add module-level docstrings for better context.
    • Add tests to ensure docstring accuracy and completeness.

This PR is a valuable contribution to the project, but addressing the above points will ensure that the documentation is not only complete but also accurate and secure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L Large PR (< 500 lines) size/XL Extra large PR (500+ lines)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs: add docstrings to Python adapter modules

1 participant