FIX: List Scenarios Bug (microsoft#1216)

Author: rlundeen2

doc/code/front_end/2_pyrit_shell.md

@@ -1,4 +1,4 @@
-# PyRIT Shell - Interactive Command Line
+# 2. PyRIT Shell - Interactive Command Line
 
 PyRIT Shell provides an interactive REPL (Read-Eval-Print Loop) for running AI red teaming scenarios with fast execution and session-based result tracking.
 

pyrit/cli/scenario_registry.py

@@ -94,7 +94,12 @@ def discover_modules(base_path: Path, base_module: str) -> None:
                     if module_name.startswith("_"):
                         continue
 
-                    full_module_name = f"pyrit.scenario.scenarios.{base_module}.{module_name}"
+                    # Build the full module name correctly
+                    if base_module:
+                        full_module_name = f"{base_module}.{module_name}"
+                    else:
+                        full_module_name = f"pyrit.scenario.scenarios.{module_name}"
+
                     try:
                         # Import the module
                         module = importlib.import_module(full_module_name)
@@ -119,7 +124,7 @@ def discover_modules(base_path: Path, base_module: str) -> None:
                         logger.warning(f"Failed to load scenario module {full_module_name}: {e}")
 
             # Start discovery from the scenarios package root
-            discover_modules(package_path, "pyrit.scenarios.scenarios")
+            discover_modules(package_path, "")
 
         except Exception as e:
             logger.error(f"Failed to discover built-in scenarios: {e}")

tests/unit/cli/test_scenario_registry.py

@@ -59,6 +59,92 @@ def test_discover_builtin_scenarios(self):
             assert scenario_class is not None
             assert issubclass(scenario_class, Scenario)
 
+    def test_discover_builtin_scenarios_correct_module_paths(self):
+        """Test that builtin scenario discovery uses correct module paths without duplication.
+
+        This is a regression test for a bug where module paths were incorrectly constructed
+        as 'pyrit.scenario.scenarios.pyrit.scenarios.scenarios.xxx' instead of
+        'pyrit.scenario.scenarios.xxx'.
+        """
+        with patch("pyrit.cli.scenario_registry.importlib.import_module") as mock_import:
+            with patch("pyrit.cli.scenario_registry.pkgutil.iter_modules") as mock_iter:
+                with patch("pyrit.cli.scenario_registry.inspect.getmembers") as mock_getmembers:
+                    # Mock the scenarios package structure
+                    def iter_modules_side_effect(paths):
+                        path_str = str(paths[0]) if paths else ""
+                        if "airt" in path_str:
+                            # Mock the airt subpackage
+                            return [
+                                (None, "content_harms_scenario", False),
+                                (None, "cyber_scenario", False),
+                            ]
+                        else:
+                            # Mock the main scenarios package
+                            return [
+                                (None, "encoding_scenario", False),  # A file module
+                                (None, "foundry_scenario", False),  # Another file module
+                                (None, "airt", True),  # A package (subdirectory)
+                            ]
+
+                    mock_iter.side_effect = iter_modules_side_effect
+
+                    # Create mock module with Scenario subclass
+                    mock_scenario_class = type(
+                        "TestScenario",
+                        (Scenario,),
+                        {
+                            "_get_atomic_attacks_async": lambda self: [],
+                            "get_strategy_class": classmethod(lambda cls: MockStrategy),
+                            "get_default_strategy": classmethod(lambda cls: MockStrategy.ALL),
+                        },
+                    )
+
+                    # Mock getmembers to return our test scenario
+                    mock_getmembers.return_value = [("TestScenario", mock_scenario_class)]
+
+                    # Mock import_module to return a simple module object
+                    def import_side_effect(module_name):
+                        mock_mod = MagicMock()
+                        mock_mod.__name__ = module_name
+                        return mock_mod
+
+                    mock_import.side_effect = import_side_effect
+
+                    registry = ScenarioRegistry()
+                    registry._discover_builtin_scenarios()
+
+                    # Verify the correct module paths were attempted
+                    import_calls = [call[0][0] for call in mock_import.call_args_list]
+
+                    # Filter to only scenario module imports (not the base package imports)
+                    # We expect imports like:
+                    # - pyrit.scenario.scenarios.encoding_scenario
+                    # - pyrit.scenario.scenarios.foundry_scenario
+                    # - pyrit.scenario.scenarios.airt.content_harms_scenario
+                    # - pyrit.scenario.scenarios.airt.cyber_scenario
+                    scenario_module_imports = [
+                        call
+                        for call in import_calls
+                        if call.startswith("pyrit.scenario.scenarios.") and call != "pyrit.scenario.scenarios"
+                    ]
+
+                    # Should NOT see duplicated paths like:
+                    # - pyrit.scenario.scenarios.pyrit.scenarios.scenarios.xxx
+
+                    assert len(scenario_module_imports) > 0, "No scenario modules were imported"
+
+                    for call_path in scenario_module_imports:
+                        # Verify no path duplication
+                        assert (
+                            "pyrit.scenario.scenarios.pyrit" not in call_path
+                        ), f"Module path has duplication: {call_path}"
+                        # Verify 'scenarios' appears exactly once (not duplicated)
+                        assert call_path.count("scenarios") == 1, f"Module path has 'scenarios' duplicated: {call_path}"
+                        # Verify correct base path
+                        assert call_path.startswith(
+                            "pyrit.scenario.scenarios."
+                        ), f"Module path doesn't start with correct base: {call_path}"
+
     def test_get_scenario_existing(self):
         """Test getting an existing scenario."""
         registry = ScenarioRegistry()