FIX: pyrit_shell run fails for every scenario with a flag-collision error

[jsong468]

Bug Description

Running any scenario from pyrit_shell failed before argument parsing with:

Error: Scenario parameter flag '--memory-labels' collides with a built-in flag. Rename the parameter to avoid the collision.

Root cause: The shell registers each of a scenario's parameters as a CLI flag, using the full list from the API (scenario_meta.supported_parameters). That list combines the scenario's custom parameters with the framework's common parameters (memory_labels, max_concurrency, max_retries), which normalize to flags that already exist as built-in shell flags. The collision guard _validate_scenario_flag_collisions raised on the first such clash (memory_labels) instead of skipping it. Since every scenario carries the common parameters, run was broken universally.

pyrit_scan was unaffected: its _add_scenario_params_from_api silently skips any scenario flag that already exists as a built-in. The inconsistency between the two entry points was the bug.

Fix

Make the shell mirror pyrit_scan. Replaced _validate_scenario_flag_collisions with _resolve_scenario_flag_collisions in pyrit/cli/_cli_args.py, which:

• Drops scenario specs whose flag collides with a built-in flag (the built-in owns that flag), instead of raising.
• Still raises on genuine scenario-vs-scenario collisions (two declared params normalizing to the same flag)

parse_run_arguments now uses the returned filtered specs rather than relying on a validator side effect.

Testing

• Added tests/unit/cli/test_cli_args.py::test_parse_run_arguments_skips_scenario_params_colliding_with_builtins — reproduces the airt.scam case (common params present) and asserts run parses successfully while the scenario's custom --max-turns flag still works.
• Added tests/unit/cli/test_cli_args.py::test_parse_run_arguments_raises_on_scenario_vs_scenario_collision — confirms the scenario-vs-scenario check is preserved.
• New tests pass (4/4). Full test_pyrit_shell.py + test_pyrit_scan.py suites pass (168/168), no regressions.

[rlundeen2]

Rich agrees with this comment but it is copilot generated:

The docstring says this drop mirrors pyrit_scan._add_scenario_params_from_api, but it only mirrors the built-in collision case. _add_scenario_params_from_api uses a single seen_flags set and silently drops any duplicate flag (first-wins), including scenario-vs-scenario. Here we instead raise on a scenario-vs-scenario collision. Net effect: a scenario definition that runs fine under pyrit_scan would hard-fail under pyrit_shell — the two entrypoints aren't actually at parity for that case.

In practice the raise branch is close to dead code (param names are Python identifiers and the _→- normalization is injective over identifiers, so two distinct names can't collide), so severity is low. But it's worth either aligning with pyrit_scan (silently drop, first-wins) for true parity, or softening the "mirroring" wording to call out the intentional divergence and why.