FIRE-832 | Rogue-CLI | MCP Protocol support

[yuval-qf]

Description

Motivation and Context

Type of Change

• 🐛 Bug fix (non-breaking change which fixes an issue)
• ✨ New feature (non-breaking change which adds functionality)
• 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
• 📝 Documentation update
• 🎨 Code style/refactoring (no functional changes)
• 🧪 Test updates
• 🔧 Configuration/build changes

Changes Made

Screenshots/Examples (if applicable)

Checklist

• I have read the CONTRIBUTING.md guide
• My code follows the code style of this project (PEP 8, type hints, docstrings)
• I have run uv run black . to format my code
• I have run uv run flake8 . and fixed all issues
• I have run uv run mypy --config-file .mypy.ini . and addressed type checking issues
• I have run uv run bandit -c .bandit.yaml -r . for security checks
• I have added tests that prove my fix is effective or that my feature works
• I have run uv run pytest and all tests pass
• I have manually tested my changes
• I have updated the documentation accordingly
• I have added/updated type hints for new/modified functions
• My changes generate no new warnings
• I have checked my code for security issues
• Any dependent changes have been merged and published

Testing

Test Configuration:

• Python version:
• OS:
• Other relevant details:

Test Steps:
1.
2.
3.

Additional Notes

Related Issues/PRs

• Fixes #
• Related to #
• Depends on #

---

[coderabbitai]

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

sdks/python/rogue_sdk/types.py (1)

108-136: Remove type ignore and validate protocol/transport compatibility.

Current typing sets a non-optional Transport with a None default, requiring a type: ignore and skipping validation of mismatched pairs. Prefer Optional typing + a validator that defaults and enforces compatibility.

Apply:

@@
-    transport: Transport = None  # type: ignore # fixed in model_post_init
+    transport: Transport | None = None
@@
     @model_validator(mode="after")
     def check_auth_credentials(self) -> "AgentConfig":
@@
         return self
-
-    def model_post_init(self, __context: Any) -> None:
-        if self.transport is None:
-            self.transport = self.protocol.get_default_transport()
+
+    @model_validator(mode="after")
+    def ensure_transport_for_protocol(self) -> "AgentConfig":
+        # Default
+        if self.transport is None:
+            self.transport = self.protocol.get_default_transport()
+        # Validate
+        if not self.transport.is_valid_for_protocol(self.protocol):
+            raise ValueError(
+                f"Transport '{self.transport.value}' is invalid for protocol '{self.protocol.value}'"
+            )
+        return self

This removes the mypy suppression and prevents invalid combinations early. As per coding guidelines.

rogue/run_cli.py (1)

27-57: Add --workdir argument to set_cli_args().

The argument is missing despite being accessed at:

• rogue/run_cli.py:358 (config_file logic)
• rogue/run_cli.py:469 (evaluation_results_output_path)
• rogue/run_ui.py:22 (workdir assignment)
• rogue/__main__.py:252 (mkdir operation)

Add to rogue/run_cli.py in set_cli_args():

parser.add_argument("--workdir", type=Path, default=Path(".") / ".rogue", help="Working directory")

🧹 Nitpick comments (5)

.vscode/launch.json (1)

47-65: Ensure business context is supplied (or add a flag).

This launch config doesn’t pass --business-context or --business-context-file; CLI validation will fail unless ${workspaceFolder}/examples/tshirt_store_agent/.rogue/business_context.md exists. Consider adding one of those args explicitly for reliability. You may also add --transport to exercise non-default MCP transports when needed.

rogue/tests/models/test_cli_input.py (1)

27-29: Tests updated correctly; add coverage for defaults and invalid combos.

• Add a case with transport=None to assert it defaults via protocol.get_default_transport().
• Add a negative case (e.g., protocol=MCP, transport=HTTP) expecting ValidationError once validation is added.

Also applies to: 86-88

sdks/python/rogue_sdk/sdk.py (1)

257-257: Rely on AgentConfig to default (or validate here).

If you adopt AgentConfig.ensure_transport_for_protocol, you can simplify to pass transport through and let the model default/validate:

-            transport=transport or protocol.get_default_transport(),
+            transport=transport,

Otherwise, add a guard:

if transport and not transport.is_valid_for_protocol(protocol):
    raise ValueError("Invalid transport/protocol combination")

rogue/tests/test_run_cli.py (1)

26-28: Tests look good; consider MCP and invalid-transport cases.

• Add a case with protocol=MCP and transport omitted to assert default to STREAMABLE_HTTP.
• Add a case with protocol=MCP and transport=HTTP to assert early validation error (once validation is added).

Also applies to: 50-52, 73-75

rogue/run_cli.py (1)

389-414: Add timeouts to MCP client transports (if supported).

Consider passing reasonable timeouts (connect/read) to StreamableHttpTransport/SSETransport to avoid indefinite hangs during ping. If the library supports it, e.g., timeout=5.0.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2ec51dc and ee15fca.

📒 Files selected for processing (7)

• .vscode/launch.json (1 hunks)
• rogue/models/cli_input.py (4 hunks)
• rogue/run_cli.py (8 hunks)
• rogue/tests/models/test_cli_input.py (3 hunks)
• rogue/tests/test_run_cli.py (4 hunks)
• sdks/python/rogue_sdk/sdk.py (1 hunks)
• sdks/python/rogue_sdk/types.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Format Python code with Black
Ensure code passes flake8 linting
Run mypy with the repository configuration for static typing
Run Bandit security checks using .bandit.yaml configuration
Use isort import conventions for import ordering
Add type hints to all function signatures
Follow PEP 8 naming (snake_case for variables/functions, PascalCase for classes)
Use try/except around code that may raise exceptions

Files:

• sdks/python/rogue_sdk/sdk.py
• sdks/python/rogue_sdk/types.py
• rogue/tests/test_run_cli.py
• rogue/tests/models/test_cli_input.py
• rogue/run_cli.py
• rogue/models/cli_input.py

🧬 Code graph analysis (5)

sdks/python/rogue_sdk/sdk.py (1)

sdks/python/rogue_sdk/types.py (1)

• get_default_transport (74-79)

rogue/tests/test_run_cli.py (1)

sdks/python/rogue_sdk/types.py (3)

• AuthType (23-48)
• Protocol (68-79)
• Transport (82-93)

rogue/tests/models/test_cli_input.py (1)

sdks/python/rogue_sdk/types.py (2)

• Protocol (68-79)
• Transport (82-93)

rogue/run_cli.py (1)

sdks/python/rogue_sdk/types.py (6)

• AgentConfig (104-136)
• AuthType (23-48)
• Protocol (68-79)
• Scenarios (181-199)
• Transport (82-93)
• get_auth_header (31-48)

rogue/models/cli_input.py (1)

sdks/python/rogue_sdk/types.py (5)

• AuthType (23-48)
• Protocol (68-79)
• Scenarios (181-199)
• Transport (82-93)
• get_default_transport (74-79)