Skip to content

fix(integrations): validate required options before switch uninstalls - #5008

Merged
mnriem merged 1 commit into
github:mainfrom
yxshee:fix/switch-required-options
Oct 19, 2026
Merged

mnriem merged 1 commit into
github:mainfrom
yxshee:fix/switch-required-options

Conversation

@yxshee

@yxshee yxshee commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Description

specify integration switch generic checks generic's required --commands-dir too late. Running it without --integration-options, with options that leave out --commands-dir (e.g. --skills), or with a blank --commands-dir= uninstalls the current integration before it fails. If that was the only integration, the project is left with none:

$ specify init sw3 --integration claude --ignore-agent-tools --non-interactive && cd sw3
$ specify integration switch generic
Uninstalling current integration: claude
  Removed 10 file(s)
Updated execute permissions on 6 script(s) recursively
Installing integration: generic
Error: Failed to install integration 'generic' during switch: --commands-dir is required for the generic integration
exit=1
$ ls .specify/integration.json .claude
ls: .specify/integration.json: No such file or directory
ls: .claude: No such file or directory
$ specify integration status
Integration status: ERROR
Default integration: none
Installed integrations: none

init-options.json also loses its ai and integration keys.

Root cause:

  • integration_switch() already validates the target's options before Phase 1 so that invalid options can't leave a project half switched (command_switch.py, the block before # Phase 1).
  • That check only parses the options and calls is_skills_mode(). Neither _parse_integration_options() nor is_skills_mode() reads IntegrationOption.required.
  • The only enforcement of generic's required --commands-dir is in GenericIntegration._resolve_commands_dir(), which setup() calls. Switch reaches setup() in Phase 2.
  • By then Phase 1 has run the teardown, cleared the integration from init-options.json and removed .specify/integration.json, so the rollback has nothing to fall back to.

The fix adds a small _missing_required_options() check next to the existing pre-Phase-1 validation. It exits 1 before any file is touched when a required option is missing or blank:

$ specify integration switch generic
Error: Integration 'generic' requires --commands-dir in --integration-options.
No files were changed.
exit=1        # .specify/integration.json still lists claude; .claude/skills/ still has all 10 skills
$ specify integration switch generic --integration-options="--commands-dir .myagent/commands"
✓ Switched to integration 'Generic (bring your own agent)'

Switching back to an already-installed generic without passing options still works, because _resolve_integration_options() returns its stored parsed options, which include commands_dir.

Testing

  • Tested locally with uv run specify --help

  • Ran existing tests with uv sync && uv run pytest

  • Tested with a sample project (if applicable): the transcripts above, from a claude project initialized with this checkout

  • New test_switch_rejects_missing_required_option_before_uninstall, parametrized for no options, options without --commands-dir, and a blank --commands-dir=. It asserts exit 1, that the claude skills are still on disk, that integration.json and init-options.json are byte-identical, and the error message. All three cases fail on main (AssertionError: switch removed the current integration) and pass with the change.

  • New test_switch_to_generic_with_required_option covers the positive path.

  • Two existing rollback tests (test_failed_switch_keeps_fallback_metadata_consistent, test_failed_switch_rescaffolds_fallback_extensions) used a bare switch generic only to make Phase 2 fail. They now pass --commands-dir ../outside, which setup() still rejects after the teardown, so they keep covering the rollback path. They pass both with and without the fix.

  • tests/specify_cli/integrations/test_command_switch.py: 30 passed. With tests/integrations/test_integration_generic.py: 271 passed.

  • Full suite (macOS, Python 3.13): 12334 passed, 376 skipped. 11 failures in the tests/test_*_python_parity.py files occur identically on unmodified main on this machine under full-suite load; those files pass when run on their own (257 passed, 112 skipped).

  • uvx ruff@0.15.0 check src tests: clean

AI Disclosure

  • I did not use AI assistance for this contribution
  • I did use AI assistance (fill in the disclosure below)

AI disclosure: Claude Code (model: Claude Opus 5.5, max reasoning effort, autonomous agent mode) opened this PR on behalf of @yxshee. A Claude Code subagent (same model, autonomous) found the bug by exercising the CLI, then wrote the fix, the tests and the reproductions above. The lead agent independently re-ran the new tests against main (3 failed) and the branch (30 passed), reproduced both transcripts above, reviewed the diff, and wrote this description. No human reviewed the change line by line before it was opened.

🤖 Generated with Claude Code

`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 <noreply@anthropic.com>
@yxshee
yxshee requested a review from mnriem as a code owner October 17, 2026 19:02
@mnriem mnriem added the triage-must-have Verdict: high-value, important work for Spec Kit — do first label Oct 18, 2026
@mnriem
mnriem requested a max review from Copilot October 19, 2026 12:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused guard prevents the reported destructive failure and has positive, negative, and reported before-and-after regression coverage.

No open issues 🎉

What changed in this PR

Prevents integration switches from removing the current integration when required target options are missing or blank.

Changes:

  • Validates required options before teardown.
  • Adds rejection and success tests while preserving rollback coverage.

Tests were not rerun; validation used code inspection and the PR’s reported regression results.

File Description
tests/​specify_cli/​integrations/​test_command_switch.py Adds required-option coverage and adjusts rollback failure inputs.
src/​specify_cli/​integrations/​command_switch.py Rejects missing or blank required options before uninstalling.

Review effort: Max

@mnriem
mnriem merged commit 3c4ef3e into github:main Oct 19, 2026
16 checks passed
@mnriem

mnriem commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

triage-must-have Verdict: high-value, important work for Spec Kit — do first

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants