mirror of
https://github.com/github/spec-kit.git
synced 2026-07-03 12:28:06 +08:00
fix(opencode): use commands/ directory (plural) to match OpenCode docs (#2453)
* fix(opencode): use commands/ directory (plural) to match OpenCode docs OpenCode documentation (https://opencode.ai/docs/commands/) uses .opencode/commands/ (plural) as the canonical command directory. The OpenCode runtime supports both .opencode/command/ and .opencode/commands/ via a {command,commands} glob, but the singular form was the original convention and is now outdated. Update the OpenCode integration to write to .opencode/commands/ instead of .opencode/command/, aligning with the documented standard and the OpenSpec fix (Fission-AI/OpenSpec#748). Signed-off-by: Marcus Burghardt <maburgha@redhat.com> Assisted-by: OpenCode (claude-opus-4-6) * feat(registrar): add legacy_dir fallback for backward-compatible directory migration Add _resolve_agent_dir() to CommandRegistrar that checks a legacy_dir fallback when the canonical directory does not exist. When legacy_dir is found, a deprecation warning directs users to run "specify integration upgrade" to migrate. The OpenCode integration declares legacy_dir: ".opencode/command" so that extension and preset registration, as well as command cleanup, continue working for projects that have not yet migrated to .opencode/commands/. The legacy_dir mechanism is opt-in: integrations that do not declare it get no fallback and no behavioral change. Add end-to-end test verifying that "specify integration upgrade opencode" migrates commands from legacy .opencode/command/ to canonical .opencode/commands/ and removes stale files. Signed-off-by: Marcus Burghardt <maburgha@redhat.com> Assisted-by: OpenCode (claude-opus-4-6) * fix(registrar): address PR review feedback on legacy_dir handling - Fix deprecation warning formatting: quote paths and remove trailing '/.' that produced confusing '.opencode/commands/.' output - Eliminate duplicate warnings: pass pre-resolved directory to register_commands() via _resolved_dir parameter so _resolve_agent_dir() is only called once per agent - Fix unregister_commands() to clean both canonical and legacy dirs when both exist, preventing orphaned command files after upgrade - Add test_unregister_cleans_legacy_when_both_dirs_exist regression test and tighten warning count assertion to exactly 1 Assisted-by: OpenCode (claude-opus-4-6) Signed-off-by: Marcus Burghardt <maburgha@redhat.com> --------- Signed-off-by: Marcus Burghardt <maburgha@redhat.com>
This commit is contained in:
@@ -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 = (
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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"))
|
||||
|
||||
@@ -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 ───────────────────────────────────────────────────
|
||||
|
||||
|
||||
Reference in New Issue
Block a user