diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 4d78d5ac4..a1be34dcc 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -438,6 +438,7 @@ class CommandRegistrar: source_dir: Path, project_root: Path, context_note: str = None, + _resolved_dir: Path = None, ) -> List[str]: """Register commands for a specific agent. @@ -448,6 +449,10 @@ class CommandRegistrar: source_dir: Directory containing command source files project_root: Path to project root context_note: Custom context comment for markdown output + _resolved_dir: Pre-resolved command directory (internal use + only — avoids a second ``_resolve_agent_dir`` call and + duplicate deprecation warnings when invoked from + ``register_commands_for_all_agents``). Returns: List of registered command names @@ -460,7 +465,9 @@ class CommandRegistrar: raise ValueError(f"Unsupported agent: {agent_name}") agent_config = self.AGENT_CONFIGS[agent_name] - commands_dir = project_root / agent_config["dir"] + commands_dir = _resolved_dir or self._resolve_agent_dir( + agent_name, agent_config, project_root, + ) commands_dir.mkdir(parents=True, exist_ok=True) registered = [] @@ -639,6 +646,40 @@ class CommandRegistrar: CommandRegistrar._ensure_inside(prompt_file, prompts_dir) prompt_file.write_text(f"---\nagent: {cmd_name}\n---\n", encoding="utf-8") + @staticmethod + def _resolve_agent_dir( + agent_name: str, + agent_config: dict[str, Any], + project_root: Path, + ) -> Path: + """Return the agent command directory, falling back to legacy_dir. + + When the canonical directory (``agent_config["dir"]``) does not + exist but a ``legacy_dir`` is configured and present on disk, + returns the legacy path and emits a deprecation warning advising + the user to upgrade. + + Integrations that do not declare ``legacy_dir`` get the canonical + path unconditionally — no fallback, no warning. + """ + agent_dir = project_root / agent_config["dir"] + if not agent_dir.exists(): + legacy = agent_config.get("legacy_dir") + if legacy: + legacy_dir = project_root / legacy + if legacy_dir.exists(): + import warnings + + warnings.warn( + f"Found legacy '{legacy}' directory for " + f"{agent_name}. Run 'specify integration " + f"upgrade {agent_name}' to migrate to " + f"'{agent_config['dir']}'.", + stacklevel=3, + ) + return legacy_dir + return agent_dir + def register_commands_for_all_agents( self, commands: List[Dict[str, Any]], @@ -663,7 +704,9 @@ class CommandRegistrar: self._ensure_configs() for agent_name, agent_config in self.AGENT_CONFIGS.items(): - agent_dir = project_root / agent_config["dir"] + agent_dir = self._resolve_agent_dir( + agent_name, agent_config, project_root, + ) if agent_dir.exists(): try: @@ -674,6 +717,7 @@ class CommandRegistrar: source_dir, project_root, context_note=context_note, + _resolved_dir=agent_dir, ) if registered: results[agent_name] = registered @@ -711,7 +755,9 @@ class CommandRegistrar: for agent_name, agent_config in self.AGENT_CONFIGS.items(): if agent_config.get("extension") == "/SKILL.md": continue - agent_dir = project_root / agent_config["dir"] + agent_dir = self._resolve_agent_dir( + agent_name, agent_config, project_root, + ) if agent_dir.exists(): try: registered = self.register_commands( @@ -721,6 +767,7 @@ class CommandRegistrar: source_dir, project_root, context_note=context_note, + _resolved_dir=agent_dir, ) if registered: results[agent_name] = registered @@ -733,6 +780,11 @@ class CommandRegistrar: ) -> None: """Remove previously registered command files from agent directories. + When a ``legacy_dir`` is configured, files are removed from + *both* the canonical and the legacy directory so that orphaned + commands left behind after an ``integration upgrade`` are + cleaned up as well. + Args: registered_commands: Dict mapping agent names to command name lists project_root: Path to project root @@ -743,24 +795,39 @@ class CommandRegistrar: continue agent_config = self.AGENT_CONFIGS[agent_name] - commands_dir = project_root / agent_config["dir"] + commands_dir = self._resolve_agent_dir( + agent_name, agent_config, project_root, + ) + + # Collect all directories to clean: canonical (or resolved + # legacy) plus the legacy dir if it exists separately. + dirs_to_clean = [commands_dir] + legacy = agent_config.get("legacy_dir") + if legacy: + legacy_dir = project_root / legacy + if legacy_dir.exists() and legacy_dir != commands_dir: + dirs_to_clean.append(legacy_dir) for cmd_name in cmd_names: output_name = self._compute_output_name( agent_name, cmd_name, agent_config ) - cmd_file = commands_dir / f"{output_name}{agent_config['extension']}" - if cmd_file.exists(): - cmd_file.unlink() - # For SKILL.md agents each command lives in its own subdirectory - # (e.g. .agents/skills/speckit-ext-cmd/SKILL.md). Remove the - # parent dir when it becomes empty to avoid orphaned directories. - parent = cmd_file.parent - if parent != commands_dir and parent.exists(): - try: - parent.rmdir() # no-op if dir still has other files - except OSError: - pass + for target_dir in dirs_to_clean: + cmd_file = ( + target_dir / f"{output_name}{agent_config['extension']}" + ) + if cmd_file.exists(): + cmd_file.unlink() + # For SKILL.md agents each command lives in its own + # subdirectory (e.g. .agents/skills/speckit-ext-cmd/ + # SKILL.md). Remove the parent dir when it becomes + # empty to avoid orphaned directories. + parent = cmd_file.parent + if parent != target_dir and parent.exists(): + try: + parent.rmdir() + except OSError: + pass if agent_name == "copilot": prompt_file = ( diff --git a/src/specify_cli/integrations/opencode/__init__.py b/src/specify_cli/integrations/opencode/__init__.py index 17db2bd11..4fa9c724a 100644 --- a/src/specify_cli/integrations/opencode/__init__.py +++ b/src/specify_cli/integrations/opencode/__init__.py @@ -8,12 +8,13 @@ class OpencodeIntegration(MarkdownIntegration): config = { "name": "opencode", "folder": ".opencode/", - "commands_subdir": "command", + "commands_subdir": "commands", "install_url": "https://opencode.ai", "requires_cli": True, } registrar_config = { - "dir": ".opencode/command", + "dir": ".opencode/commands", + "legacy_dir": ".opencode/command", "format": "markdown", "args": "$ARGUMENTS", "extension": ".md", diff --git a/tests/integrations/test_integration_opencode.py b/tests/integrations/test_integration_opencode.py index 427fd1516..ba2d15711 100644 --- a/tests/integrations/test_integration_opencode.py +++ b/tests/integrations/test_integration_opencode.py @@ -1,6 +1,10 @@ """Tests for OpencodeIntegration.""" +import warnings + +from specify_cli.agents import CommandRegistrar from specify_cli.integrations import get_integration +from specify_cli.integrations.manifest import IntegrationManifest from .test_integration_base_markdown import MarkdownIntegrationTests @@ -8,8 +12,8 @@ from .test_integration_base_markdown import MarkdownIntegrationTests class TestOpencodeIntegration(MarkdownIntegrationTests): KEY = "opencode" FOLDER = ".opencode/" - COMMANDS_SUBDIR = "command" - REGISTRAR_DIR = ".opencode/command" + COMMANDS_SUBDIR = "commands" + REGISTRAR_DIR = ".opencode/commands" CONTEXT_FILE = "AGENTS.md" def test_build_exec_args_uses_run_command_dispatch(self): @@ -57,3 +61,140 @@ class TestOpencodeIntegration(MarkdownIntegrationTests): args = integration.build_exec_args("explain this repository", output_json=False) assert args == ["opencode", "run", "explain this repository"] + + def test_registrar_config_has_legacy_dir(self): + integration = get_integration(self.KEY) + assert integration.registrar_config["legacy_dir"] == ".opencode/command" + + def test_legacy_dir_extension_registration(self, tmp_path): + """Extensions register in legacy .opencode/command/ with a warning.""" + # Seed a legacy project with only .opencode/command/ + legacy_dir = tmp_path / ".opencode" / "command" + legacy_dir.mkdir(parents=True) + (legacy_dir / "speckit.specify.md").write_text("# existing", encoding="utf-8") + + # Create a source command file for the registrar + src_dir = tmp_path / "_ext_src" + src_dir.mkdir() + (src_dir / "myext.md").write_text( + "---\ndescription: test\n---\n# ext command", encoding="utf-8", + ) + + registrar = CommandRegistrar() + commands = [{"name": "speckit.myext", "file": "myext.md"}] + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + results = registrar.register_commands_for_all_agents( + commands, "test-ext", src_dir, tmp_path, + ) + + # Should have registered in the legacy directory + assert "opencode" in results + assert (legacy_dir / "speckit.myext.md").exists() + # Canonical directory should NOT have been created + assert not (tmp_path / ".opencode" / "commands").exists() + # Should have emitted a deprecation warning + opencode_warnings = [ + w for w in caught + if "legacy" in str(w.message) and "opencode" in str(w.message) + ] + assert len(opencode_warnings) == 1, ( + f"Expected exactly 1 legacy-dir warning, got {len(opencode_warnings)}" + ) + assert "specify integration upgrade" in str(opencode_warnings[0].message) + + def test_legacy_dir_unregister(self, tmp_path): + """Unregister finds commands in legacy .opencode/command/ dir.""" + legacy_dir = tmp_path / ".opencode" / "command" + legacy_dir.mkdir(parents=True) + cmd_file = legacy_dir / "speckit.myext.md" + cmd_file.write_text("# ext command", encoding="utf-8") + + registrar = CommandRegistrar() + + with warnings.catch_warnings(record=True): + warnings.simplefilter("always") + registrar.unregister_commands( + {"opencode": ["speckit.myext"]}, tmp_path, + ) + + assert not cmd_file.exists() + + def test_unregister_cleans_legacy_when_both_dirs_exist(self, tmp_path): + """Unregister removes files from legacy dir even when canonical exists.""" + # Set up both canonical and legacy dirs + canonical_dir = tmp_path / ".opencode" / "commands" + canonical_dir.mkdir(parents=True) + legacy_dir = tmp_path / ".opencode" / "command" + legacy_dir.mkdir(parents=True) + + # Place a command file in the legacy dir (orphaned after upgrade) + legacy_cmd = legacy_dir / "speckit.myext.md" + legacy_cmd.write_text("# orphaned ext command", encoding="utf-8") + # Place the same command in the canonical dir (current) + canonical_cmd = canonical_dir / "speckit.myext.md" + canonical_cmd.write_text("# ext command", encoding="utf-8") + + registrar = CommandRegistrar() + + with warnings.catch_warnings(record=True): + warnings.simplefilter("always") + registrar.unregister_commands( + {"opencode": ["speckit.myext"]}, tmp_path, + ) + + # Both files should be removed + assert not canonical_cmd.exists(), ( + "Command file in canonical dir should be removed" + ) + assert not legacy_cmd.exists(), ( + "Orphaned command file in legacy dir should also be removed" + ) + + def test_canonical_dir_preferred_over_legacy(self, tmp_path): + """When both dirs exist, canonical .opencode/commands/ is used.""" + legacy_dir = tmp_path / ".opencode" / "command" + legacy_dir.mkdir(parents=True) + canonical_dir = tmp_path / ".opencode" / "commands" + canonical_dir.mkdir(parents=True) + (canonical_dir / "speckit.specify.md").write_text("# cmd", encoding="utf-8") + + # Create a source command file for the registrar + src_dir = tmp_path / "_ext_src" + src_dir.mkdir() + (src_dir / "myext.md").write_text( + "---\ndescription: test\n---\n# ext command", encoding="utf-8", + ) + + registrar = CommandRegistrar() + commands = [{"name": "speckit.myext", "file": "myext.md"}] + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + results = registrar.register_commands_for_all_agents( + commands, "test-ext", src_dir, tmp_path, + ) + + # Should register in canonical dir, not legacy + assert "opencode" in results + assert (canonical_dir / "speckit.myext.md").exists() + assert not (legacy_dir / "speckit.myext.md").exists() + # No legacy warning when canonical dir exists + opencode_warnings = [ + w for w in caught + if "legacy" in str(w.message) and "opencode" in str(w.message) + ] + assert len(opencode_warnings) == 0 + + def test_setup_writes_to_canonical_dir(self, tmp_path): + """New installs always write to .opencode/commands/ (plural).""" + integration = get_integration(self.KEY) + manifest = IntegrationManifest(self.KEY, tmp_path) + integration.setup(tmp_path, manifest) + + canonical = tmp_path / ".opencode" / "commands" + legacy = tmp_path / ".opencode" / "command" + assert canonical.is_dir() + assert not legacy.exists() + assert any(canonical.glob("speckit.*.md")) diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index 5163f98db..e36067b3c 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -762,7 +762,7 @@ class TestIntegrationSwitch: assert result.exit_code == 0, result.output # Git extension commands should exist for opencode - opencode_git_feature = project / ".opencode" / "command" / "speckit.git.feature.md" + opencode_git_feature = project / ".opencode" / "commands" / "speckit.git.feature.md" assert opencode_git_feature.exists(), "Git extension command should exist for opencode" # Old kimi extension skills should be removed @@ -837,7 +837,7 @@ class TestIntegrationSwitch: ]) assert result.exit_code == 0, result.output - opencode_git_feature = project / ".opencode" / "command" / "speckit.git.feature.md" + opencode_git_feature = project / ".opencode" / "commands" / "speckit.git.feature.md" assert opencode_git_feature.exists(), "Git extension command should exist for opencode" assert not copilot_git_feature.exists(), "Old Copilot extension skill should be removed" @@ -858,7 +858,7 @@ class TestIntegrationSwitch: result = _run_in_project(project, ["extension", "disable", "git"]) assert result.exit_code == 0, result.output - opencode_git_feature = project / ".opencode" / "command" / "speckit.git.feature.md" + opencode_git_feature = project / ".opencode" / "commands" / "speckit.git.feature.md" assert opencode_git_feature.exists(), "Disabled extension command remains until integration switch" result = _run_in_project(project, [ @@ -1168,6 +1168,49 @@ class TestIntegrationUpgrade: assert data["integration"] == "gemini" assert "/speckit.plan" in template.read_text(encoding="utf-8") + def test_upgrade_migrates_opencode_legacy_dir(self, tmp_path): + """Upgrade moves OpenCode commands from .opencode/command/ to .opencode/commands/.""" + project = _init_project(tmp_path, "opencode") + + # Simulate a legacy project: rename commands/ back to command/ + canonical = project / ".opencode" / "commands" + legacy = project / ".opencode" / "command" + assert canonical.is_dir(), "init should have created .opencode/commands/" + canonical.rename(legacy) + assert legacy.is_dir() + assert not canonical.exists() + + # Patch the manifest to reflect old paths (command/ not commands/) + manifest_path = project / ".specify" / "integrations" / "opencode.manifest.json" + manifest_data = json.loads(manifest_path.read_text(encoding="utf-8")) + patched_files = {} + for path, info in manifest_data.get("files", {}).items(): + patched_files[path.replace(".opencode/commands/", ".opencode/command/")] = info + manifest_data["files"] = patched_files + manifest_path.write_text(json.dumps(manifest_data), encoding="utf-8") + + old_commands = sorted(legacy.glob("speckit.*.md")) + assert len(old_commands) > 0, "Legacy dir should have speckit command files" + + result = _run_in_project(project, [ + "integration", "upgrade", "opencode", + "--script", "sh", + "--force", + ]) + assert result.exit_code == 0, f"upgrade failed: {result.output}" + + # New commands in canonical dir + assert canonical.is_dir(), ".opencode/commands/ should exist after upgrade" + new_commands = sorted(canonical.glob("speckit.*.md")) + assert len(new_commands) > 0, "Commands should exist in .opencode/commands/" + + # Stale files removed from legacy dir + remaining = list(legacy.glob("speckit.*.md")) + assert len(remaining) == 0, ( + f"Legacy .opencode/command/ should have no speckit files after upgrade, " + f"found: {[f.name for f in remaining]}" + ) + # ── Full lifecycle ───────────────────────────────────────────────────