fix: migrate extension commands on integration switch

[cyliu0]

Description

When switching integrations (e.g., specify integration switch opencode), extension commands were not re-registered for the new agent. This caused two problems:

1. Missing commands: The new agent's directory did not receive extension commands (e.g., git extension commands like speckit.git.feature were absent after switching from kimi to opencode)
2. Orphaned files: Extension files remained in the old agent's directory with stale registry entries pointing to the old agent

Root Cause

Extension commands are registered during specify extension add via CommandRegistrar.register_commands_for_all_agents(), which writes commands for all agents that already have directories. When switching integrations later, the new agent gets a fresh directory but extensions are never re-registered.

Fix

Added two new ExtensionManager methods:

1. unregister_agent_artifacts(agent_name) — Called after uninstalling the old integration. Removes extension command files and skills for the old agent, and cleans up registry entries.

2. register_enabled_extensions_for_agent(agent_name) — Called after installing the new integration. Re-registers all enabled extensions for the new agent. Correctly handles Copilot --skills mode by detecting ai_skills in init-options and skipping command registration when skills mode is active.

Wired both into integration_switch() in src/specify_cli/__init__.py.

Testing

• Tested locally with uv run specify --help
• Ran existing tests with uv sync --extra test && uv run pytest
• Tested with a sample project (see below)

Automated tests added

• test_switch_migrates_extension_commands: Tests kimi → opencode → claude migration, verifying extension commands appear for the new agent and are removed from the old
• test_switch_migrates_copilot_skills_extension_commands: Tests Copilot with --skills flag to ensure extensions are installed as skills, not commands
• test_switch_does_not_register_disabled_extensions: Ensures disabled extensions are not re-registered after switch

Manual test results

Agent: Manual CLI testing | OS/Shell: macOS/zsh

Command tested	Notes
specify init . --integration kimi	Git extension skills created in .kimi/skills/
specify integration switch opencode	Extension commands migrated to .opencode/command/, old skills removed
specify integration switch claude	Extension skills migrated to .claude/skills/, old commands removed

Test results

tests/integrations/test_integration_subcommand.py: 28 passed
tests/test_extensions.py: 190 passed
tests/integrations/: 918 passed

AI Disclosure

• I did not use AI assistance for this contribution
• I did use AI assistance (describe below)

This fix was developed with assistance from OpenAI Codex and Kimi coding agents. AI was used for:

• Initial code exploration and root cause analysis
• Implementation of the unregister_agent_artifacts and register_enabled_extensions_for_agent methods
• Test case generation and verification

All changes were reviewed, tested, and validated manually before submission.

[Copilot]

Pull request overview

Fixes extension command/skill migration when switching integrations so extension artifacts are cleaned up for the old agent and re-registered for the new agent, keeping agent directories and the extension registry consistent.

Changes:

• Added ExtensionManager.unregister_agent_artifacts() to remove old agent extension command files and extension-generated skills, and to update the registry accordingly.
• Added ExtensionManager.register_enabled_extensions_for_agent() to re-register all enabled extensions for the new integration, including Copilot --skills mode behavior.
• Wired both steps into integration_switch() and added integration-switch tests covering migration, skills mode, and disabled extensions.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
src/specify_cli/__init__.py	Calls extension cleanup before clearing metadata and re-registration after installing the target integration.
src/specify_cli/extensions.py	Implements agent artifact unregistration and enabled-extension re-registration logic.
tests/integrations/test_integration_subcommand.py	Adds regression tests ensuring artifacts migrate/clean correctly across integrations and modes.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[cyliu0]

Thanks for the review! All three comments have been addressed in the latest commit:

1. Redundant inner if installed_key check (src/specify_cli/init.py): Removed the redundant inner check. The outer block already guarantees installed_key is truthy.

2. commands_dir.is_dir() guard (src/specify_cli/extensions.py): Removed the guard and replaced it with commands_dir.mkdir(parents=True, exist_ok=True) so extensions are always re-registered on switch, even if the directory does not yet exist.

3. Agent-scoped skills cleanup (src/specify_cli/extensions.py): Fixed unregister_agent_artifacts to explicitly resolve the skills directory for the provided agent_name using the same _get_skills_dir helper that extension install uses, rather than relying on the currently-active agent in init-options. Also updated _unregister_extension_skills to accept an optional skills_dir parameter.

[Copilot]

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 3/3 changed files
• Comments generated: 3

[mnriem]

Please address Copilot feedback. If not applicable, please explain why

[cyliu0]

Thanks for the second round of review! All three comments have been addressed:

1. Refactored test_switch_migrates_extension_commands to use the existing _run_in_project() helper instead of repeated os.chdir()/try/finally blocks, keeping the test consistent with others in the file.

2. Removed the redundant commands_dir.mkdir() call in register_enabled_extensions_for_agent() since CommandRegistrar.register_commands_for_agent() already creates the directory.

3. Updated the warning message in integration_switch() to mention both commands and skills: Could not register extension commands, skills, or related artifacts.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This test verifies that Copilot skills mode creates SKILL.md and does not create .agent.md files, but it doesn’t assert that the generated SKILL.md frontmatter is correct for Copilot (notably the injected mode: field). Adding an assertion on the SKILL.md content (e.g., that mode: maps back to the dotted command name for the extension command) would prevent regressions where the skill exists but Copilot can’t route it correctly.

Suggested change

	assert not copilot_agent_file.exists(), "Copilot skills mode should not create extension .agent.md files"
	assert not copilot_agent_file.exists(), "Copilot skills mode should not create extension .agent.md files"
	copilot_git_feature_content = copilot_git_feature.read_text(encoding="utf-8")
	assert "mode: speckit.git.feature" in copilot_git_feature_content, (
	"Git extension skill should include Copilot mode frontmatter for the dotted command name"
	)

[cyliu0]

Addressing the two new comments:

Comment 7 (Copilot skills mode frontmatter): The concern about mode: mapping is already handled correctly. CopilotIntegration.post_process_skill_content() parses the name: field from frontmatter and converts speckit-git-feature back to speckit.git-feature format before injecting mode:. Verified locally that generated SKILL.md contains mode: speckit.git-feature.

Comment 8 (Test verification): Added assertion in test_switch_migrates_copilot_skills_extension_commands to verify the generated SKILL.md contains the correct mode: field mapping.

[Copilot]

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 3/4 changed files
• Comments generated: 2

[Copilot]

unregister_agent_artifacts() removes agent_name from the registered_commands registry even when agent_name is not a supported/known agent in CommandRegistrar.AGENT_CONFIGS (in that case unregister_commands() is a no-op and the command files will remain on disk). This can happen during the “unknown integration via manifest” switch path, leaving orphaned files while also losing the registry pointer needed to clean them up later. Consider only popping/updating registered_commands[agent_name] after confirming the agent is supported (or otherwise performing a best-effort filesystem cleanup based on the integration manifest) so registry state stays consistent with what was actually removed.

[Copilot]

agent_skills_dir is always passed into _unregister_extension_skills(), even when that directory doesn’t exist (e.g., switching away from a non-skills agent). Because _unregister_extension_skills() treats any non-None skills_dir as the “fast path”, this disables its safe fallback scan of all known skills directories, which is useful for cleaning up stale registered_skills entries created by previous integration switches/older versions. Consider only passing skills_dir when it is_dir(), otherwise pass None to allow the fallback scan (it still verifies metadata.source before deleting).

[cyliu0]

Addressing the two new comments:

Comment 9 (Unknown agent cleanup): Added a guard in unregister_agent_artifacts() to return early when agent_name is not present in CommandRegistrar.AGENT_CONFIGS. This prevents losing registry entries while leaving orphaned files on disk during the "unknown integration via manifest" switch path.

Comment 10 (Skills directory fallback): Changed the skills_dir passed to _unregister_extension_skills() to be None when the resolved agent_skills_dir does not exist on disk. This re-enables the fallback scan of all known agent skills directories, which is important for cleaning up stale registered_skills entries created by earlier installs.

[Copilot]

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 3/4 changed files
• Comments generated: 1

[Copilot]

unregister_agent_artifacts() clears registered_skills unconditionally after calling _unregister_extension_skills(). _unregister_extension_skills() intentionally skips deletion when it can’t verify ownership via metadata.source (or when files are missing/unreadable), so this can leave extension-created skill directories on disk while dropping the only registry reference needed for future cleanup.

Consider only removing skill names from the registry when their directories were actually deleted (e.g., re-check existence under the targeted skills_dir, or keep the list unchanged when cleanup can’t be verified).

Suggested change

	updates["registered_skills"] = []
	# Only drop registry entries for skill directories that were
	# actually removed. _unregister_extension_skills may
	# intentionally skip deletion when ownership cannot be
	# verified, so re-check the relevant skills directories and
	# keep any still-present skill names registered for future
	# cleanup attempts.
	verification_dirs: List[Path]
	if skills_dir is not None:
	verification_dirs = [skills_dir]
	else:
	verification_dirs = [
	resolve_skills_dir(self.project_root, known_agent)
	for known_agent in registrar.AGENT_CONFIGS
	]
	remaining_registered_skills = [
	skill_name
	for skill_name in registered_skills
	if any((base_dir / skill_name).exists() for base_dir in verification_dirs)
	]
	updates["registered_skills"] = remaining_registered_skills

[mnriem]

Please address Copilot feedback

[cyliu0]

Addressing the latest Copilot comment (comment #11):

unregister_agent_artifacts() was unconditionally clearing registered_skills after calling _unregister_extension_skills(). However, _unregister_extension_skills() may intentionally skip deletion when it cannot verify ownership via metadata.source (e.g., corrupted/missing SKILL.md). This left skill directories on disk while removing the only registry reference needed for future cleanup.

Fixed by checking which skill directories still exist after cleanup and only keeping those still-present names in the registry. Future cleanup attempts will still be able to find skipped skills.

[cyliu0]

@mnriem All Copilot feedback has been addressed across multiple commits. Here's a summary of all 11 comments and their resolutions:

1. ✅ Redundant inner if installed_key check - removed
2. ✅ commands_dir.is_dir() guard preventing re-registration - replaced with mkdir()
3. ✅ Agent-scoped skills cleanup - now uses _get_skills_dir() for the specific agent
4. ✅ Test using manual os.chdir() - refactored to use _run_in_project() helper
5. ✅ Redundant commands_dir mkdir() - removed (registrar handles it)
6. ✅ Warning message accuracy - updated to mention skills/artifacts
7. ✅ Copilot skills mode: frontmatter mapping - verified correct, added test assertion
8. ✅ Test missing mode: assertion - added assertion for mode: field
9. ✅ Unknown agent cleanup guard - added AGENT_CONFIGS check to skip unsupported agents
10. ✅ Skills directory fallback - pass None when dir doesn't exist to enable fallback scan
11. ✅ Unconditional registered_skills clearing - now only removes entries for actually-deleted skills

All 244 tests pass. Please re-review when you have a chance.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

register_enabled_extensions_for_agent() only updates registered_commands when registered is non-empty. If command registration yields an empty list (e.g., corrupted manifest pointing at missing command files), any existing registered_commands[agent_name] entry will be left in the registry, creating stale metadata that later cleanup will try to remove. Consider explicitly removing/clearing the agent entry when registration returns an empty list.

Suggested change

	if registered:
	registered_commands = metadata.get("registered_commands", {})
	if not isinstance(registered_commands, dict):
	registered_commands = {}
	new_registered = copy.deepcopy(registered_commands)
	new_registered[agent_name] = registered
	registered_commands = metadata.get("registered_commands", {})
	if not isinstance(registered_commands, dict):
	registered_commands = {}
	new_registered = copy.deepcopy(registered_commands)
	if registered:
	new_registered[agent_name] = registered
	else:
	new_registered.pop(agent_name, None)
	if new_registered != registered_commands:

[Copilot]

In unregister_agent_artifacts(), remaining_skills is computed using agent_skills_dir even when skills_dir is None and _unregister_extension_skills() may fall back to a different directory (active agent) or scan multiple candidate skill dirs. This can incorrectly clear registered_skills in the registry even if the skill directories were not removed, or if they were removed from a different location than agent_skills_dir. Consider either (a) skipping the registry update when agent_skills_dir doesn't exist / when you didn't target a specific dir, or (b) making _unregister_extension_skills return the list of removed skills so the registry can be updated accurately.

Suggested change

	# _unregister_extension_skills may intentionally skip deletion
	# when ownership cannot be verified (e.g., corrupted/missing
	# SKILL.md or mismatching metadata.source). Only drop registry
	# entries for skill directories that were actually removed so
	# future cleanup attempts can still find skipped ones.
	remaining_skills = [
	skill_name
	for skill_name in registered_skills
	if (agent_skills_dir / skill_name / "SKILL.md").exists()
	]
	if remaining_skills != registered_skills:
	updates["registered_skills"] = remaining_skills
	# Only reconcile registry state when cleanup was scoped to a
	# specific existing directory. When skills_dir is None,
	# _unregister_extension_skills may fall back to a different
	# agent's directory or scan multiple candidate directories, so
	# agent_skills_dir cannot be used to infer what was removed.
	if skills_dir is not None:
	# _unregister_extension_skills may intentionally skip
	# deletion when ownership cannot be verified (e.g.,
	# corrupted/missing SKILL.md or mismatching
	# metadata.source). Only drop registry entries for skill
	# directories that were actually removed so future cleanup
	# attempts can still find skipped ones.
	remaining_skills = [
	skill_name
	for skill_name in registered_skills
	if (skills_dir / skill_name / "SKILL.md").exists()
	]
	if remaining_skills != registered_skills:
	updates["registered_skills"] = remaining_skills

[cyliu0]

Addressing the two new Copilot comments (round 4):

Comment 12 (Stale registry on empty registration): register_enabled_extensions_for_agent() now explicitly removes the agent entry from registered_commands when registration returns an empty list (e.g., corrupted manifest pointing at missing command files). This prevents stale metadata that later cleanup would try to remove.

Comment 13 (Incorrect registry reconciliation during fallback cleanup): When skills_dir is None (fallback scan mode), we no longer use agent_skills_dir to check remaining skills. Registry reconciliation now only happens when skills_dir is explicitly set (specific directory targeted), correctly matching what _unregister_extension_skills() actually removed.

All 220 tests pass (30 integration subcommand + 190 extension tests).

[Copilot]

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

In unregister_agent_artifacts(), the registry reconciliation for registered_skills treats a skill as "removed" when (skills_dir / skill_name / "SKILL.md").exists() is false. But _unregister_extension_skills() explicitly skips deletion when SKILL.md is missing/unreadable, leaving the directory in place. In that case this check will incorrectly drop the skill from registered_skills, preventing future cleanup attempts and leaving an orphaned directory. Consider checking whether the skill directory still exists (e.g., (skills_dir / skill_name).is_dir()) rather than SKILL.md existence, or have _unregister_extension_skills() report which skills were actually deleted so the registry can be updated accurately.

Suggested change

	if (skills_dir / skill_name / "SKILL.md").exists()
	if (skills_dir / skill_name).is_dir()

[cyliu0]

Fixed comment 14: Changed registry reconciliation to check (skills_dir / skill_name).is_dir() instead of SKILL.md existence. When _unregister_extension_skills() skips deletion due to missing SKILL.md (leaving directory in place), the previous check would incorrectly drop the skill from registered_skills. Now we check directory existence so skipped skills remain in the registry for future cleanup attempts.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

In unregister_agent_artifacts, when agent_skills_dir does not exist you pass skills_dir=None, which makes _unregister_extension_skills() fall back to scanning all known agent skills directories. That behavior is not agent-scoped and can delete extension skill directories for other agents (contradicting this method’s purpose/name). Consider always passing the resolved agent_skills_dir (even if it doesn't exist) to keep the operation scoped, or add an explicit flag to _unregister_extension_skills() to disable the global scan when called from integration switch cleanup.

Suggested change

	# Only pass the resolved skills_dir when it actually exists.
	# Otherwise let _unregister_extension_skills fall back to
	# scanning all known agent skills directories, which is useful
	# for cleaning up stale entries created by earlier installs.
	skills_dir = agent_skills_dir if agent_skills_dir.is_dir() else None
	self._unregister_extension_skills(
	registered_skills, ext_id, skills_dir=skills_dir
	)
	# Only reconcile registry state when cleanup was scoped to a
	# specific existing directory. When skills_dir is None,
	# _unregister_extension_skills falls back to scanning multiple
	# candidate directories, so agent_skills_dir cannot be used to
	# infer what was removed. When skills_dir is set,
	# _unregister_extension_skills may intentionally skip deletion
	# when ownership cannot be verified (e.g., corrupted/missing
	# SKILL.md or mismatching metadata.source). Only drop registry
	# entries for skill directories that were actually removed so
	# future cleanup attempts can still find skipped ones.
	if skills_dir is not None:
	remaining_skills = [
	skill_name
	for skill_name in registered_skills
	if (skills_dir / skill_name).is_dir()
	]
	if remaining_skills != registered_skills:
	updates["registered_skills"] = remaining_skills
	# Keep cleanup scoped to the target agent even when its skills
	# directory does not exist. Passing None would allow
	# _unregister_extension_skills to fall back to scanning other
	# known agent skill directories, which is not appropriate for
	# unregister_agent_artifacts.
	skills_dir = agent_skills_dir
	self._unregister_extension_skills(
	registered_skills, ext_id, skills_dir=skills_dir
	)
	# Reconcile registry state against the target agent's skills
	# directory only. _unregister_extension_skills may
	# intentionally skip deletion when ownership cannot be
	# verified (e.g., corrupted/missing SKILL.md or mismatching
	# metadata.source), so only remove registry entries for skill
	# directories that are actually gone.
	remaining_skills = [
	skill_name
	for skill_name in registered_skills
	if (skills_dir / skill_name).is_dir()
	]
	if remaining_skills != registered_skills:
	updates["registered_skills"] = remaining_skills

[Copilot]

This warning says it couldn’t clean up “extension commands”, but the called helper (unregister_agent_artifacts) also removes extension skills and updates registry entries. Consider adjusting the message to refer to “extension artifacts” (commands/skills/registry) so users aren’t misled about what may be left behind.

[cyliu0]

Fixed: Updated the warning message to refer to "extension artifacts (commands, skills, registry entries)" instead of just "extension commands", since unregister_agent_artifacts() also removes skills and updates registry entries.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.