mirror of
https://github.com/github/spec-kit.git
synced 2026-07-03 12:28:06 +08:00
fix: register enabled extensions for agent on integration use/upgrade (#2949)
* fix: register enabled extensions for agent on integration install/upgrade install and upgrade only set up the integration's own core commands; only switch re-registered the enabled extensions' commands for the target agent. A second integration added via install (or refreshed via upgrade) was therefore silently missing the extension commands the existing agents already had (e.g. the bundled agent-context extension). Extract switch's registration into a shared _register_extensions_for_agent helper and call it from install and upgrade too, so every installed agent ends up with every enabled extension's commands — full parity with switch. Closes #2886 * test: pin skills-mode secondary-agent registration; document #2948 limitation Extension skill rendering is scoped to the active agent (init-options track a single ai / ai_skills pair), so a skills-mode agent registered while not active (e.g. Copilot --skills installed as a secondary integration) gets command files rather than skills. install/upgrade match extension add here; only switch renders skills, because it activates the target first. Add a regression test pinning this behavior and document the limitation on the shared helper. Per-agent skills parity is tracked separately in #2948. * fix: don't re-render the active agent's skills when registering a non-active agent register_enabled_extensions_for_agent runs an active-agent-scoped skills pass (_register_extension_skills resolves the skills dir from init-options["ai"], ignoring the passed agent). Routing install/upgrade of a secondary integration through it re-rendered the *active* skills-mode agent's extension skills as a side effect — resurrecting skill files the user had deliberately deleted. Gate the skills pass on the target being the active agent; switch is unaffected because it activates the target first. Also harden the skills-mode install test (assert a core skill so --skills is load-bearing, drop a vacuous registered_skills assertion) and add a regression test. Surfaced by review of the PR; skills parity for non-active agents stays tracked in #2948. * refactor: share the extension-op scaffold and run (un)registration post-commit Review cleanups, no behavior change on the success path: - Extract the best-effort ExtensionManager scaffold (lazy import, instantiate, except -> _print_cli_warning) into _best_effort_extension_op. Both _register_extensions_for_agent and a new _unregister_extensions_for_agent delegate to it, removing the duplicate block left inline in switch. - Invoke the best-effort extension registration AFTER the install/switch/upgrade try/except has committed, so a failure in it can never trigger the rollback (install and switch teardown on except). * docs: clarify extension registration parity scope * fix(integrations): defer extension registration until use * fix(tests): remove redundant shutil import * fix(integrations): backfill extensions for installed switch targets
This commit is contained in:
Pascal THUET
@@ -15,19 +15,22 @@ from tests.conftest import strip_ansi
|
||||
runner = CliRunner()
|
||||
|
||||
|
||||
def _init_project(tmp_path, integration="copilot"):
|
||||
def _init_project(tmp_path, integration="copilot", integration_options=None):
|
||||
"""Helper: init a spec-kit project with the given integration."""
|
||||
project = tmp_path / "proj"
|
||||
project.mkdir()
|
||||
args = [
|
||||
"init", "--here",
|
||||
"--integration", integration,
|
||||
"--script", "sh",
|
||||
"--ignore-agent-tools",
|
||||
]
|
||||
if integration_options:
|
||||
args += ["--integration-options", integration_options]
|
||||
old_cwd = os.getcwd()
|
||||
try:
|
||||
os.chdir(project)
|
||||
result = runner.invoke(app, [
|
||||
"init", "--here",
|
||||
"--integration", integration,
|
||||
"--script", "sh",
|
||||
"--ignore-agent-tools",
|
||||
], catch_exceptions=False)
|
||||
result = runner.invoke(app, args, catch_exceptions=False)
|
||||
finally:
|
||||
os.chdir(old_cwd)
|
||||
assert result.exit_code == 0, f"init failed: {result.output}"
|
||||
@@ -1237,6 +1240,137 @@ class TestIntegrationInstall:
|
||||
assert "/speckit-specify" in script_content
|
||||
assert "/speckit.specify" not in script_content
|
||||
|
||||
def test_install_defers_extension_commands_until_use(self, tmp_path):
|
||||
"""Installing a second integration does not register enabled extensions.
|
||||
|
||||
Maintainer-requested behavior for #2886: extension command back-fill is
|
||||
limited to ``integration use`` / ``switch`` / ``upgrade``. Plain
|
||||
``install`` only adds the integration; selecting it with ``use`` then
|
||||
registers the enabled extensions for that agent.
|
||||
"""
|
||||
project = _init_project(tmp_path, "claude")
|
||||
|
||||
result = _run_in_project(project, ["extension", "add", "git"])
|
||||
assert result.exit_code == 0, f"extension add failed: {result.output}"
|
||||
|
||||
registry_path = project / ".specify" / "extensions" / ".registry"
|
||||
registered = json.loads(registry_path.read_text(encoding="utf-8"))[
|
||||
"extensions"
|
||||
]["git"]["registered_commands"]
|
||||
assert "claude" in registered
|
||||
assert "codex" not in registered, "precondition: codex not yet installed"
|
||||
|
||||
result = _run_in_project(project, [
|
||||
"integration", "install", "codex",
|
||||
"--script", "sh",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
# Install alone does not back-fill the git extension for the secondary
|
||||
# agent.
|
||||
registered = json.loads(registry_path.read_text(encoding="utf-8"))[
|
||||
"extensions"
|
||||
]["git"]["registered_commands"]
|
||||
assert "claude" in registered, "existing agent registration preserved"
|
||||
assert "codex" not in registered
|
||||
assert not (
|
||||
project / ".agents" / "skills" / "speckit-git-feature" / "SKILL.md"
|
||||
).exists()
|
||||
|
||||
result = _run_in_project(project, ["integration", "use", "codex"])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
registered = json.loads(registry_path.read_text(encoding="utf-8"))[
|
||||
"extensions"
|
||||
]["git"]["registered_commands"]
|
||||
assert "codex" in registered, "use should register extension commands (#2886)"
|
||||
assert (
|
||||
project / ".agents" / "skills" / "speckit-git-feature" / "SKILL.md"
|
||||
).exists()
|
||||
|
||||
def test_install_does_not_register_disabled_extensions(self, tmp_path):
|
||||
"""A disabled extension must not be registered for a newly installed agent."""
|
||||
project = _init_project(tmp_path, "claude")
|
||||
|
||||
result = _run_in_project(project, ["extension", "add", "git"])
|
||||
assert result.exit_code == 0, f"extension add failed: {result.output}"
|
||||
result = _run_in_project(project, ["extension", "disable", "git"])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
result = _run_in_project(project, [
|
||||
"integration", "install", "codex",
|
||||
"--script", "sh",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
registry_path = project / ".specify" / "extensions" / ".registry"
|
||||
git_meta = json.loads(registry_path.read_text(encoding="utf-8"))[
|
||||
"extensions"
|
||||
]["git"]
|
||||
assert git_meta["enabled"] is False
|
||||
assert "codex" not in git_meta["registered_commands"]
|
||||
assert not (
|
||||
project / ".agents" / "skills" / "speckit-git-feature" / "SKILL.md"
|
||||
).exists()
|
||||
|
||||
def test_install_skills_mode_secondary_agent_defers_extension_artifacts(self, tmp_path):
|
||||
"""A non-active skills-mode agent gets extension artifacts only on use.
|
||||
|
||||
Plain ``install`` has no extension side effects. Once the secondary
|
||||
Copilot ``--skills`` integration is selected with ``use``, it becomes the
|
||||
active agent and receives extension skills.
|
||||
"""
|
||||
project = _init_project(tmp_path, "claude")
|
||||
|
||||
result = _run_in_project(project, ["extension", "add", "git"])
|
||||
assert result.exit_code == 0, f"extension add failed: {result.output}"
|
||||
|
||||
# Copilot is not multi_install_safe, so --force is required to add it
|
||||
# alongside the existing default integration.
|
||||
result = _run_in_project(project, [
|
||||
"integration", "install", "copilot",
|
||||
"--script", "sh",
|
||||
"--integration-options", "--skills",
|
||||
"--force",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
# Precondition that makes --skills load-bearing: copilot IS in skills
|
||||
# mode, so its own core commands are scaffolded as skills.
|
||||
assert (
|
||||
project / ".github" / "skills" / "speckit-specify" / "SKILL.md"
|
||||
).exists(), "precondition: copilot installed in skills mode"
|
||||
|
||||
# The git extension is not registered for the non-active copilot agent
|
||||
# during install.
|
||||
git_meta = json.loads(
|
||||
(project / ".specify" / "extensions" / ".registry").read_text(encoding="utf-8")
|
||||
)["extensions"]["git"]
|
||||
assert "copilot" not in git_meta["registered_commands"]
|
||||
assert not (
|
||||
project / ".github" / "agents" / "speckit.git.feature.agent.md"
|
||||
).exists()
|
||||
assert not (
|
||||
project / ".github" / "skills" / "speckit-git-feature" / "SKILL.md"
|
||||
).exists()
|
||||
|
||||
result = _run_in_project(project, ["integration", "use", "copilot"])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
git_meta = json.loads(
|
||||
(project / ".specify" / "extensions" / ".registry").read_text(encoding="utf-8")
|
||||
)["extensions"]["git"]
|
||||
# `use` makes copilot active, so extension artifacts follow copilot's
|
||||
# skills-mode layout.
|
||||
assert "copilot" not in git_meta["registered_commands"]
|
||||
assert "speckit-git-feature" in git_meta["registered_skills"]
|
||||
assert not (
|
||||
project / ".github" / "agents" / "speckit.git.feature.agent.md"
|
||||
).exists()
|
||||
assert (
|
||||
project / ".github" / "skills" / "speckit-git-feature" / "SKILL.md"
|
||||
).exists()
|
||||
|
||||
|
||||
# ── uninstall ────────────────────────────────────────────────────────
|
||||
|
||||
@@ -1724,6 +1858,40 @@ class TestIntegrationSwitch:
|
||||
assert "claude" in registered_commands
|
||||
assert "opencode" not in registered_commands
|
||||
|
||||
def test_switch_installed_target_backfills_extension_commands(self, tmp_path):
|
||||
"""Switching to an already-installed agent should register extensions."""
|
||||
project = _init_project(tmp_path, "claude")
|
||||
|
||||
result = _run_in_project(project, ["extension", "add", "git"])
|
||||
assert result.exit_code == 0, f"extension add failed: {result.output}"
|
||||
|
||||
registry_path = project / ".specify" / "extensions" / ".registry"
|
||||
registered = json.loads(registry_path.read_text(encoding="utf-8"))[
|
||||
"extensions"
|
||||
]["git"]["registered_commands"]
|
||||
assert "claude" in registered
|
||||
assert "codex" not in registered, "precondition: codex not yet installed"
|
||||
|
||||
result = _run_in_project(project, [
|
||||
"integration", "install", "codex",
|
||||
"--script", "sh",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
codex_git_feature = (
|
||||
project / ".agents" / "skills" / "speckit-git-feature" / "SKILL.md"
|
||||
)
|
||||
assert not codex_git_feature.exists()
|
||||
|
||||
result = _run_in_project(project, ["integration", "switch", "codex"])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
registered = json.loads(registry_path.read_text(encoding="utf-8"))[
|
||||
"extensions"
|
||||
]["git"]["registered_commands"]
|
||||
assert "codex" in registered
|
||||
assert codex_git_feature.exists()
|
||||
|
||||
def test_switch_migrates_copilot_skills_extension_commands(self, tmp_path):
|
||||
"""Copilot --skills should receive extension skills, not .agent.md files."""
|
||||
project = _init_project(tmp_path, "opencode")
|
||||
@@ -2324,6 +2492,93 @@ class TestIntegrationUpgrade:
|
||||
"shared .sh scripts must be executable after upgrade"
|
||||
)
|
||||
|
||||
def test_upgrade_backfills_extension_commands_for_agent(self, tmp_path):
|
||||
"""Upgrade re-registers enabled extensions for the upgraded agent.
|
||||
|
||||
Regression for #2886: agents installed before extension back-fill
|
||||
existed (or whose extension artifacts went missing) should regain the
|
||||
enabled extensions' commands on ``upgrade``, reaching parity with
|
||||
``switch``.
|
||||
"""
|
||||
project = _init_project(tmp_path, "claude")
|
||||
|
||||
result = _run_in_project(project, ["extension", "add", "git"])
|
||||
assert result.exit_code == 0, f"extension add failed: {result.output}"
|
||||
|
||||
result = _run_in_project(project, [
|
||||
"integration", "install", "codex",
|
||||
"--script", "sh",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
# Simulate a project created before the install/upgrade back-fill: drop
|
||||
# codex's extension registration and its rendered artifacts.
|
||||
registry_path = project / ".specify" / "extensions" / ".registry"
|
||||
registry = json.loads(registry_path.read_text(encoding="utf-8"))
|
||||
registry["extensions"]["git"]["registered_commands"].pop("codex", None)
|
||||
registry_path.write_text(json.dumps(registry), encoding="utf-8")
|
||||
agents_skills = project / ".agents" / "skills"
|
||||
for skill_dir in agents_skills.glob("speckit-git-*"):
|
||||
shutil.rmtree(skill_dir)
|
||||
|
||||
# Precondition: codex is now missing the git extension.
|
||||
assert "codex" not in json.loads(registry_path.read_text(encoding="utf-8"))[
|
||||
"extensions"
|
||||
]["git"]["registered_commands"]
|
||||
assert not (agents_skills / "speckit-git-feature" / "SKILL.md").exists()
|
||||
|
||||
result = _run_in_project(project, [
|
||||
"integration", "upgrade", "codex",
|
||||
"--script", "sh",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
# Upgrade back-filled the git extension for codex.
|
||||
registered = json.loads(registry_path.read_text(encoding="utf-8"))[
|
||||
"extensions"
|
||||
]["git"]["registered_commands"]
|
||||
assert "codex" in registered, "upgrade should re-register extension commands (#2886)"
|
||||
assert (agents_skills / "speckit-git-feature" / "SKILL.md").exists()
|
||||
|
||||
def test_upgrade_non_active_agent_preserves_active_agent_skills(self, tmp_path):
|
||||
"""Upgrading a non-active agent must not touch the active agent's skills.
|
||||
|
||||
Regression for the #2886 wiring: extension skill rendering is
|
||||
active-agent-scoped, so routing upgrade of a *secondary* agent through
|
||||
``register_enabled_extensions_for_agent`` used to re-render the
|
||||
*active* skills-mode agent's extension skills as a side effect —
|
||||
resurrecting skill files the user had deliberately deleted. The skills
|
||||
pass is now gated on the target being the active agent. (Skills parity
|
||||
for non-active agents is tracked separately in #2948.)
|
||||
"""
|
||||
# Active agent: copilot in skills mode → git extension renders as skills.
|
||||
project = _init_project(tmp_path, "copilot", integration_options="--skills")
|
||||
result = _run_in_project(project, ["extension", "add", "git"])
|
||||
assert result.exit_code == 0, f"extension add failed: {result.output}"
|
||||
|
||||
skill = project / ".github" / "skills" / "speckit-git-feature" / "SKILL.md"
|
||||
assert skill.exists(), "precondition: active copilot has the git extension skill"
|
||||
|
||||
# Add a secondary (non-active) agent; copilot is not multi_install_safe.
|
||||
result = _run_in_project(project, [
|
||||
"integration", "install", "codex", "--script", "sh", "--force",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
# The user deliberately removes the active agent's git skill.
|
||||
shutil.rmtree(skill.parent)
|
||||
assert not skill.exists()
|
||||
|
||||
# Upgrading the *non-active* agent must not re-render copilot's skills.
|
||||
result = _run_in_project(project, [
|
||||
"integration", "upgrade", "codex", "--script", "sh",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
assert not skill.exists(), (
|
||||
"upgrading a non-active agent must not resurrect the active agent's "
|
||||
"deleted extension skill (#2886)"
|
||||
)
|
||||
|
||||
|
||||
# ── Full lifecycle ───────────────────────────────────────────────────
|
||||
|
||||
|
||||
Reference in New Issue
Block a user