fix: improve CLI error messages in register and policy commands

[aryanputta]

Fixes #307

Description

Two CLI commands in packages/agent-mesh/src/agentmesh/cli/main.py had error handling problems that caused raw Python tracebacks to appear instead of useful messages.

The register command imported AgentIdentity inside the function body with no try/except. If agentmesh is not installed the user sees a full traceback instead of being told what to run. Wrapped the import in a try/except ImportError that prints a clear install instruction and returns early.

The policy command caught all exceptions as a single bare Exception and printed only the exception object. Split it into FileNotFoundError, JSONDecodeError and YAMLError, and a general fallback so each case gives a message that points to the actual problem.

Type of Change

• Bug fix (non-breaking change that fixes an issue)

Packages Affected

• agent-mesh

How Was This Tested?

• Manual Verification: Confirmed the register command now prints a helpful install message when agentmesh is not installed, and the policy command prints specific parse error messages when given a bad file.

AI Usage Disclosure

• No AI used.

[github-actions]

🤖 AI Agent: test-generator

🧪 Test Coverage Analysis

packages/agent-mesh/src/agentmesh/cli/main.py

• ✅ Existing coverage:

  • The register and policy commands are likely covered by basic CLI tests that verify their functionality under normal conditions.
  • Tests may already exist for successful agent registration and policy loading.

• ❌ Missing coverage:

  • Error handling for register command:
    • No tests for the ImportError scenario when agentmesh is not installed.
  • Error handling for policy command:
    • No tests for FileNotFoundError when the policy file does not exist.
    • No tests for JSONDecodeError or YAMLError when the policy file is malformed.
    • No tests for the fallback Exception case.

• 💡 Suggested test cases:

  1. test_register_missing_agentmesh — Simulate the absence of the agentmesh package and verify that the register command prints the correct error message and exits gracefully.
  2. test_policy_file_not_found — Provide a non-existent policy file path to the policy command and verify that the FileNotFoundError message is displayed.
  3. test_policy_invalid_json — Provide a policy file with invalid JSON content and verify that the JSONDecodeError message is displayed.
  4. test_policy_invalid_yaml — Provide a policy file with invalid YAML content and verify that the YAMLError message is displayed.
  5. test_policy_generic_exception — Simulate a generic exception (e.g., permission error or unexpected failure) during policy loading and verify that the fallback error message is displayed.

Additional Notes

• Ensure that the new test cases cover edge cases such as:
  • Oversized or corrupted policy files for JSONDecodeError and YAMLError.
  • Scenarios where the agentmesh.identity.AgentIdentity.create method raises unexpected exceptions during registration.
• Consider using mocking frameworks like unittest.mock to simulate the absence of the agentmesh package and other error conditions without modifying the environment or files.

[github-actions]

🤖 AI Agent: code-reviewer

Review Summary

This PR improves error handling in the register and policy commands of the agent-mesh package by providing more user-friendly error messages. The changes address the issue of raw Python tracebacks being displayed, which could confuse users. The fixes include wrapping imports in a try/except block and providing specific error messages for common exceptions.

The changes are generally well-implemented and improve the usability of the CLI. However, there are a few areas that require attention to ensure robustness, security, and maintainability.

---

🔴 CRITICAL

1. Unvalidated Exception Messages in policy Command
   • The policy command directly includes the exception message (e) in the output for json.JSONDecodeError and yaml.YAMLError. This could potentially expose sensitive information (e.g., file paths or debug information) to the user, which is a security risk.
   • Recommendation: Avoid directly exposing exception messages. Instead, provide a generic error message and log the detailed exception for debugging purposes. For example:

     except (json.JSONDecodeError, yaml.YAMLError):
         console.print(f"[red]Error: Failed to parse {policy_file}. Ensure the file is valid JSON or YAML.[/red]")
         logger.exception("Failed to parse policy file: %s", policy_file)

---

💡 SUGGESTION

1. Use Specific Exception Types for ImportError

   • While the try/except ImportError block in the register command is a good improvement, it could be more specific. For example, if agentmesh.identity has submodules, an unrelated ImportError in one of those submodules could trigger this block incorrectly.
   • Recommendation: Use importlib to check for the module's existence or validate the exception's context:

     try:
         from agentmesh.identity import AgentIdentity
     except ImportError as e:
         if "agentmesh.identity" in str(e):
             console.print("[red]Error: agentmesh is not installed. Run: pip install agentmesh[/red]")
             return
         raise

2. Add Unit Tests for Error Scenarios

   • The PR mentions manual verification, but automated tests should be added to ensure these error-handling paths are covered. This will prevent regressions in the future.
   • Recommendation: Use pytest to mock scenarios such as:
     • ImportError for the register command.
     • FileNotFoundError, JSONDecodeError, and YAMLError for the policy command.

     def test_register_import_error(mocker):
         mocker.patch("agentmesh.cli.main.AgentIdentity", side_effect=ImportError)
         result = runner.invoke(app, ["register", "test-agent"])
         assert "Error: agentmesh is not installed" in result.output
     
     def test_policy_file_not_found(mocker):
         result = runner.invoke(app, ["policy", "nonexistent-file"])
         assert "Error: Policy file not found" in result.output

3. Consider Logging Errors

   • While the CLI provides user-friendly error messages, it would be beneficial to log detailed error information for debugging purposes.
   • Recommendation: Use Python's logging module to log exceptions at an appropriate level (e.g., logger.error or logger.exception).

4. Backward Compatibility

   • While this PR does not introduce breaking changes to the public API, the improved error messages may change the CLI's output format. If any downstream automation or scripts parse these messages, they could break.
   • Recommendation: Document this change in the release notes and ensure that any dependent systems are updated accordingly.

---

🟡 WARNING

1. Potential Breaking Change in CLI Output
   • The updated error messages in the policy command change the format of the output. If users or systems rely on the previous output format, this could cause issues.
   • Recommendation: Clearly document the change in CLI behavior in the release notes and consider providing a transition period or a compatibility flag if necessary.

---

💡 Additional Suggestions

1. Use Constants for Error Messages

   • To improve maintainability and consistency, consider defining error messages as constants at the top of the file or in a separate module. This will make it easier to manage and update messages in the future.

2. Add Type Annotations

   • The functions register and policy lack type annotations for their parameters and return types. Adding type hints improves code readability and helps with static analysis.
   • Example:

     def register(agent_dir: str, name: Optional[str] = None) -> None:
     def policy(policy_file: str, validate: bool) -> None:

3. Consider Using a Custom Exception for Policy Parsing

   • Instead of handling json.JSONDecodeError and yaml.YAMLError separately, consider wrapping them in a custom exception (e.g., PolicyParsingError). This would simplify error handling and make the code more readable.

---

Final Assessment

• The PR addresses the issue effectively and improves the user experience of the CLI.
• However, there are critical security concerns regarding unvalidated exception messages and potential breaking changes in CLI output.
• With the suggested improvements, this PR can be merged safely.

Action Items

1. Address the critical security issue by sanitizing exception messages.
2. Add automated tests for the new error-handling paths.
3. Consider logging detailed error information.
4. Document the changes in CLI output to mitigate potential breaking changes.

[github-actions]

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]

🤖 AI Agent: breaking-change-detector

🔍 API Compatibility Report

Summary

This pull request modifies CLI error handling in the agent-mesh package to provide more user-friendly error messages. The changes involve wrapping imports in try/except blocks and refining exception handling for better diagnostics. No breaking changes were introduced to the public API.

Findings

Severity	Package	Change	Impact
✅	agent-mesh	Improved error handling in CLI commands	No breaking changes

Migration Guide

No migration steps are required as this pull request does not introduce any breaking changes. Existing code and users of the CLI commands will benefit from clearer error messages without requiring modifications.

Notes

• The changes enhance usability and debugging for CLI users, particularly in scenarios where dependencies are missing or input files are invalid.
• No public functions, classes, or methods were removed, renamed, or had their signatures changed.
• Exception handling changes are internal to the CLI commands and do not alter the public API.

✅ No breaking changes detected.

[github-actions]

🤖 AI Agent: docs-sync-checker

📝 Documentation Sync Report

Issues Found

• ❌ register(agent_dir: str, name: str = None) in packages/agent-mesh/src/agentmesh/cli/main.py — missing docstring
• ❌ policy(policy_file: str, validate: bool) in packages/agent-mesh/src/agentmesh/cli/main.py — missing docstring
• ⚠️ packages/agent-mesh/README.md — no mention of updated error handling for register and policy commands
• ⚠️ CHANGELOG.md — no entry for improved CLI error messages in register and policy commands

Suggestions

• 💡 Add docstring for register(agent_dir: str, name: str = None) explaining its purpose, parameters, and behavior when agentmesh is not installed.
• 💡 Add docstring for policy(policy_file: str, validate: bool) explaining its purpose, parameters, and behavior for different error cases (e.g., file not found, parsing errors).
• 💡 Update packages/agent-mesh/README.md to include a section or note about the improved error handling for register and policy commands.
• 💡 Add an entry to CHANGELOG.md under the "Fixed" section, describing the improved CLI error messages for register and policy commands.

Example Docstring Suggestions

def register(agent_dir: str, name: str = None):
    """
    Registers an agent with the given directory and optional name.

    Args:
        agent_dir (str): The directory containing the agent's files.
        name (str, optional): The name to assign to the agent. Defaults to None.

    Behavior:
        - If `agentmesh` is not installed, prints an error message and exits early.
        - Simulates the registration process and generates an agent identity.

    Returns:
        None
    """

def policy(policy_file: str, validate: bool):
    """
    Loads and optionally validates a policy file.

    Args:
        policy_file (str): The path to the policy file to load.
        validate (bool): Whether to validate the policy file after loading.

    Behavior:
        - Prints an error message if the file is not found.
        - Handles JSON and YAML parsing errors with specific error messages.
        - Prints a success message if policies are loaded successfully.

    Returns:
        None
    """

If these changes are addressed, the documentation will be in sync. Let us know if you need further assistance!

[github-actions]

🤖 AI Agent: contributor-guide

Hi @first-time-contributor! 👋

Welcome to the microsoft/agent-governance-toolkit community, and thank you for opening this pull request! 🎉 We're so excited to have you here, and we appreciate you taking the time to improve the project. Let's dive into your contribution together!

---

🌟 What You Did Well

1. Clear Problem Identification: You provided an excellent description of the issue and how your changes address it. This makes it easy for reviewers to understand the context and purpose of your PR.
2. Thoughtful Error Handling: Wrapping the AgentIdentity import in a try/except block and providing a helpful install message is a great improvement for user experience. Similarly, breaking down the policy command's error handling into specific cases (e.g., FileNotFoundError, JSONDecodeError, YAMLError) is a thoughtful touch that will make debugging much easier for users.
3. Manual Testing: It's great to see that you manually verified the changes. This demonstrates your commitment to ensuring the code works as intended. 👏
4. Commit Message: Your commit message follows the Conventional Commits format (fix:), which is exactly what we use in this project. Nice work!

---

🛠 Suggestions for Improvement

While your PR is already in great shape, here are a few suggestions to align it even more closely with our project conventions:

1. Linting with Ruff:

   • We use Ruff for linting, specifically with the E, F, and W rules enabled. Before merging, could you run Ruff on your changes to ensure they meet our linting standards? You can do this by running:

     ruff check .

     If there are any issues, Ruff will provide suggestions for fixes.

2. Adding Tests:

   • While manual testing is great, we also require automated tests for changes. Could you add tests for the new error-handling logic? Tests should go in the packages/agent-mesh/tests/ directory. For example:
     • A test for the register command when agentmesh is not installed.
     • Tests for the policy command with invalid files (e.g., missing file, malformed JSON, malformed YAML).
   • This ensures that future changes won't accidentally break the behavior you've implemented.

3. Security Considerations:

   • Since CLI tools often interact with user-provided input (e.g., file paths, JSON/YAML content), we give extra scrutiny to security-sensitive code. Your changes look safe, but it's always worth double-checking:
     • Are there any edge cases where the error messages might leak sensitive information?
     • Could the policy command's exception handling be exploited in any way (e.g., by crafting a malicious file)?
   • If you're confident everything is secure, just leave a note in the PR to confirm you've considered this.

---

📚 Helpful Resources

Here are some resources to help you with the suggestions above:

• CONTRIBUTING.md: Our guide for contributing to the project.
• QUICKSTART.md: A quick guide to getting started with the project.
• Ruff Documentation: Learn more about Ruff and its linting rules.

---

✅ Next Steps

1. Run Ruff to check for linting issues and fix any that come up.
2. Add automated tests for the new error-handling logic in packages/agent-mesh/tests/.
3. Confirm that you've considered security implications and leave a note in the PR if everything looks good.
4. Once you've made these updates, push your changes to this branch. The CI/CD pipeline will automatically re-run, and we'll review your updates promptly!

If you have any questions or need help with anything, don't hesitate to ask. We're here to support you! 😊

Thank you again for your contribution — you're helping make this project better for everyone. 🚀

Looking forward to your updates! 🙌

[github-actions]

🤖 AI Agent: security-scanner

Security Review of PR

This PR improves error handling in the register and policy commands of the agent-mesh CLI. While the changes are primarily focused on user experience, they touch critical areas of the codebase that could have security implications. Below is a detailed security analysis:

---

1. Prompt Injection Defense Bypass

• Risk: No prompt injection risks are introduced or mitigated in this PR. The changes do not involve user input being processed into prompts for AI agents.
• Rating: 🔵 LOW
• Recommendation: No action needed.

---

2. Policy Engine Circumvention

• Risk: The policy command is responsible for loading and validating policy files. The changes improve error handling for file loading and parsing but do not alter the core logic of policy validation. There is no evidence of policy circumvention introduced by this PR.
• Rating: 🔵 LOW
• Recommendation: No action needed.

---

3. Trust Chain Weaknesses

• Risk: The register command interacts with the AgentIdentity class to generate identities. The changes wrap the import of AgentIdentity in a try/except block to handle missing dependencies gracefully. This does not affect the trust chain or identity validation mechanisms.
• Rating: 🔵 LOW
• Recommendation: No action needed.

---

4. Credential Exposure

• Risk: The changes do not introduce any new logging of sensitive information. The error messages added in this PR are generic and do not expose any secrets or sensitive data.
• Rating: 🔵 LOW
• Recommendation: No action needed.

---

5. Sandbox Escape

• Risk: The changes do not involve any code execution or system-level operations that could lead to a sandbox escape.
• Rating: 🔵 LOW
• Recommendation: No action needed.

---

6. Deserialization Attacks

• Risk: The policy command handles JSON and YAML files, which are prone to deserialization attacks if not properly sanitized. The PR improves error handling for JSONDecodeError and YAMLError, but it does not address potential risks of malicious payloads in these files.
• Rating: 🟠 HIGH
• Attack Vector: A malicious actor could craft a YAML or JSON file containing payloads designed to exploit vulnerabilities in the YAML or JSON parser. For example, YAML deserialization could execute arbitrary code if unsafe loaders like yaml.load() are used.
• Recommendation:
  • Ensure that the YAML parser uses a safe loader, such as yaml.safe_load(), to prevent execution of arbitrary code.
  • Validate the structure and content of the parsed data to ensure it conforms to expected schemas before further processing.

---

7. Race Conditions

• Risk: No race conditions are introduced or mitigated in this PR. The changes are limited to error handling and do not involve concurrent operations or time-of-check-to-time-of-use (TOCTOU) vulnerabilities.
• Rating: 🔵 LOW
• Recommendation: No action needed.

---

8. Supply Chain

• Risk: The register command now provides an error message suggesting the installation of the agentmesh package. While this is helpful for users, it could potentially mislead them into installing a malicious package if the package name is typo-squatted.
• Rating: 🟡 MEDIUM
• Attack Vector: An attacker could create a malicious package with a similar name to agentmesh (e.g., agentmeshh or agent-mesh) and trick users into installing it.
• Recommendation:
  • Include a direct link to the official package repository or documentation in the error message to guide users to the correct source.
  • Consider adding a warning about verifying the package source before installation.

---

Summary of Findings

Issue	Rating	Description	Recommendation
Prompt Injection Defense Bypass	🔵 LOW	No prompt injection risks introduced.	No action needed.
Policy Engine Circumvention	🔵 LOW	No policy circumvention risks introduced.	No action needed.
Trust Chain Weaknesses	🔵 LOW	No trust chain issues introduced.	No action needed.
Credential Exposure	🔵 LOW	No sensitive information exposed in logs or error messages.	No action needed.
Sandbox Escape	🔵 LOW	No sandbox escape risks introduced.	No action needed.
Deserialization Attacks	🟠 HIGH	Potential for unsafe YAML deserialization if yaml.load() is used.	Use yaml.safe_load() and validate parsed data against expected schemas.
Race Conditions	🔵 LOW	No race conditions introduced.	No action needed.
Supply Chain	🟡 MEDIUM	Users could be misled into installing a typo-squatted package due to the error message suggesting pip install agentmesh.	Provide a direct link to the official package or documentation and warn users to verify the package.

---

Suggested Fixes for This PR

1. Deserialization Safety:

   • Ensure that YAML files are parsed using yaml.safe_load() instead of yaml.load() (if applicable in the codebase).
   • Validate the structure and content of the parsed data against a predefined schema.

2. Supply Chain Guidance:

   • Update the error message in the register command to include a direct link to the official agentmesh package or documentation. For example:

     console.print("[red]Error: agentmesh is not installed. Run: pip install agentmesh (https://pypi.org/project/agentmesh/)[/red]")

   • Optionally, include a warning about verifying the package source before installation.

---

Final Assessment

This PR improves error handling and user experience, but it introduces a potential supply chain risk and does not address existing deserialization risks. Addressing these issues would further strengthen the security posture of the toolkit.