From 07de61cec7ace68fd7c8ec9b34a2a7d7f0de8bcd Mon Sep 17 00:00:00 2001 From: yxshee <84263676+yxshee@users.noreply.github.com> Date: Sun, 18 Oct 2026 00:09:24 +0530 Subject: [PATCH] fix(integrations): validate required options before switch uninstalls `specify integration switch generic` run without `--integration-options`, or with options that omit or blank out `--commands-dir`, uninstalled the current integration first and only then failed: Uninstalling current integration: claude Removed 10 file(s) Installing integration: generic Error: Failed to install integration 'generic' during switch: --commands-dir is required for the generic integration By then Phase 1 has also deleted .specify/integration.json and cleared ai and integration from init-options.json. With no other integration installed, the rollback has nothing to fall back to, so the project is left with no integration at all. switch already validates the target's options before Phase 1 so that invalid options cannot leave a project half switched, but neither option parsing nor is_skills_mode() looks at IntegrationOption.required. The only check for generic's required --commands-dir is in GenericIntegration.setup(), which switch calls in Phase 2, after the teardown. Reject a missing or blank required option before Phase 1. Two rollback tests used a bare `switch generic` to make Phase 2 fail. They now pass a commands dir outside the project, which setup() still rejects after the teardown, so they keep covering the rollback path. Assisted-by: Claude Code (model: Claude Opus 5.5, autonomous) Co-Authored-By: Claude Opus 5.5 --- .../integrations/command_switch.py | 21 +++++++++ .../integrations/test_command_switch.py | 46 +++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/src/specify_cli/integrations/command_switch.py b/src/specify_cli/integrations/command_switch.py index 39139fb393..24d0fd2c5a 100644 --- a/src/specify_cli/integrations/command_switch.py +++ b/src/specify_cli/integrations/command_switch.py @@ -19,6 +19,17 @@ from ._helpers import _MANIFEST_READ_ERRORS, _SharedTemplateRefreshError, _clear_init_options_for_integration, _cli_error_detail, _cli_phase_label, _get_speckit_version, _read_integration_json, _register_extensions_for_agent, _register_presets_for_agent, _remove_integration_json, _resolve_integration_options, _resolve_script_type, _set_default_integration, _set_default_integration_or_exit, _unregister_extensions_for_agent, _unregister_presets_for_agent, _write_integration_json +def _missing_required_options(integration, parsed_options) -> list[str]: + """Return the integration's required options that are absent or blank.""" + parsed = parsed_options or {} + missing = [] + for option in integration.options(): + value = parsed.get(option.name.lstrip("-").replace("-", "_")) + if option.required and (value is None or (isinstance(value, str) and not value.strip())): + missing.append(option.name) + return missing + + @integration_app.command("switch") @external_lifecycle("switch") def integration_switch( @@ -131,6 +142,16 @@ def integration_switch( target_integration, current, target, integration_options ) target_integration.is_skills_mode(target_parsed_options, project_root) + # Required options are otherwise only enforced by setup(), which runs + # after Phase 1 has already removed the current integration. + missing_options = _missing_required_options(target_integration, target_parsed_options) + if missing_options: + console.print( + f"[red]Error:[/red] Integration '{escape(target)}' requires " + f"{', '.join(missing_options)} in --integration-options." + ) + console.print("No files were changed.") + raise typer.Exit(1) # Phase 1: Uninstall current integration (if any) if installed_key: diff --git a/tests/specify_cli/integrations/test_command_switch.py b/tests/specify_cli/integrations/test_command_switch.py index b1824102aa..95d9cd6978 100644 --- a/tests/specify_cli/integrations/test_command_switch.py +++ b/tests/specify_cli/integrations/test_command_switch.py @@ -190,6 +190,48 @@ def test_switch_rejects_conflicting_copilot_modes_before_uninstall( ) assert after_state == before_state + @pytest.mark.parametrize( + "integration_options", + [None, "--skills", "--commands-dir="], + ids=["no-options", "options-without-commands-dir", "blank-commands-dir"], + ) + def test_switch_rejects_missing_required_option_before_uninstall( + self, tmp_path, integration_options + ): + project = _init_project(tmp_path, "claude") + claude_skill = project / ".claude" / "skills" / "speckit-plan" / "SKILL.md" + state_path = project / ".specify" / "integration.json" + options_path = project / ".specify" / "init-options.json" + before_state = state_path.read_text(encoding="utf-8") + before_options = options_path.read_text(encoding="utf-8") + + args = ["integration", "switch", "generic", "--script", "sh"] + if integration_options is not None: + args += ["--integration-options", integration_options] + result = _run_in_project(project, args) + + assert result.exit_code == 1, result.output + assert claude_skill.exists(), "switch removed the current integration" + assert state_path.read_text(encoding="utf-8") == before_state + assert options_path.read_text(encoding="utf-8") == before_options + assert "requires --commands-dir" in result.output + + def test_switch_to_generic_with_required_option(self, tmp_path): + project = _init_project(tmp_path, "claude") + + result = _run_in_project(project, [ + "integration", "switch", "generic", + "--script", "sh", + "--integration-options", "--commands-dir .myagent/commands", + ]) + + assert result.exit_code == 0, result.output + assert (project / ".myagent" / "commands" / "speckit.plan.md").exists() + assert not (project / ".claude" / "skills" / "speckit-plan" / "SKILL.md").exists() + data = json.loads((project / ".specify" / "integration.json").read_text(encoding="utf-8")) + assert data["default_integration"] == "generic" + assert data["installed_integrations"] == ["generic"] + def test_switch_preserves_target_options_with_fallback_integration( self, tmp_path ): @@ -731,9 +773,11 @@ def test_failed_switch_keeps_fallback_metadata_consistent(self, tmp_path): ], catch_exceptions=False) assert install.exit_code == 0, install.output + # setup() rejects a commands dir outside the project in Phase 2. result = runner.invoke(app, [ "integration", "switch", "generic", "--script", "sh", + "--integration-options", "--commands-dir ../outside", ], catch_exceptions=False) finally: os.chdir(old_cwd) @@ -777,9 +821,11 @@ def test_failed_switch_rescaffolds_fallback_extensions(self, tmp_path): "precondition: secondary install has no extension artifacts" ) + # setup() rejects a commands dir outside the project in Phase 2. result = _run_in_project(project, [ "integration", "switch", "generic", "--script", "sh", + "--integration-options", "--commands-dir ../outside", ]) assert result.exit_code != 0