mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
fix: register extensions for the active integration only
extension add registered commands for every detected agent, and integration upgrade back-filled enabled extensions for non-active integrations. Maintainer direction on #2948: treat the project as single-active. Only the active integration gets extension artifacts; use/switch rescaffold the target when the user selects it. - extension add now routes through the all-agents pass restricted to the active integration (only_agent), keeping detection and missing-skills-dir recovery safeguards. Projects without recorded init-options fall back to detection-based registration. - integration upgrade re-registers extensions only when upgrading the active integration, reversing the #2886 back-fill for non-active targets at maintainer request. Fixes #2948 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -932,6 +932,7 @@ class CommandRegistrar:
|
||||
link_outputs: bool = False,
|
||||
create_missing_active_skills_dir: bool = False,
|
||||
extension_id: Optional[str] = None,
|
||||
only_agent: Optional[str] = None,
|
||||
) -> Dict[str, List[str]]:
|
||||
"""Register commands for all detected agents in the project.
|
||||
|
||||
@@ -949,6 +950,8 @@ class CommandRegistrar:
|
||||
skills directory) and is skipped when safe resolution or
|
||||
creation fails.
|
||||
extension_id: Extension id when rendering extension-owned commands.
|
||||
only_agent: If set, restrict registration to this single agent
|
||||
while keeping all detection and recovery safeguards (#2948).
|
||||
|
||||
Returns:
|
||||
Dictionary mapping agent names to list of registered commands
|
||||
@@ -972,6 +975,8 @@ class CommandRegistrar:
|
||||
)
|
||||
active_created_skills_dir: Optional[Path] = None
|
||||
for agent_name, agent_config in self.AGENT_CONFIGS.items():
|
||||
if only_agent is not None and agent_name != only_agent:
|
||||
continue
|
||||
active_skills_output = (
|
||||
agent_name == active_skills_agent
|
||||
and agent_config.get("extension") == "/SKILL.md"
|
||||
|
||||
@@ -975,6 +975,64 @@ class ExtensionManager:
|
||||
return _ensure_usable(agent_skills_dir)
|
||||
return _ensure_usable(skills_dir)
|
||||
|
||||
def _register_commands_for_active_agent(
|
||||
self,
|
||||
manifest: ExtensionManifest,
|
||||
extension_dir: Path,
|
||||
link_outputs: bool = False,
|
||||
) -> Dict[str, List[str]]:
|
||||
"""Register extension commands for the active integration only.
|
||||
|
||||
Maintainer-requested behavior for #2948: ``extension add`` treats the
|
||||
project as single-active — only the integration recorded in
|
||||
init-options gets command files. Non-active integrations receive them
|
||||
when selected via ``integration use`` / ``switch`` (rescaffold).
|
||||
|
||||
Projects without a recorded active integration (pre-init-options
|
||||
layouts or direct library use) fall back to detection-based
|
||||
registration for all agents.
|
||||
|
||||
Returns:
|
||||
Mapping of agent name to registered command names, matching the
|
||||
``registered_commands`` registry shape.
|
||||
"""
|
||||
from .. import load_init_options
|
||||
|
||||
registrar = CommandRegistrar()
|
||||
init_options = load_init_options(self.project_root)
|
||||
if not isinstance(init_options, dict):
|
||||
init_options = {}
|
||||
active_agent = init_options.get("ai")
|
||||
|
||||
if not active_agent or active_agent not in registrar.AGENT_CONFIGS:
|
||||
return registrar.register_commands_for_all_agents(
|
||||
manifest,
|
||||
extension_dir,
|
||||
self.project_root,
|
||||
link_outputs=link_outputs,
|
||||
create_missing_active_skills_dir=True,
|
||||
)
|
||||
|
||||
agent_config = registrar.AGENT_CONFIGS[active_agent]
|
||||
if (
|
||||
is_ai_skills_enabled(init_options)
|
||||
and agent_config.get("extension") != "/SKILL.md"
|
||||
):
|
||||
# Active agent runs skills mode: extension artifacts render as
|
||||
# skills via _register_extension_skills, not as command files.
|
||||
return {}
|
||||
|
||||
# Route through the all-agents pass restricted to the active agent so
|
||||
# detection and missing-skills-dir recovery safeguards still apply.
|
||||
return registrar.register_commands_for_all_agents(
|
||||
manifest,
|
||||
extension_dir,
|
||||
self.project_root,
|
||||
link_outputs=link_outputs,
|
||||
create_missing_active_skills_dir=True,
|
||||
only_agent=active_agent,
|
||||
)
|
||||
|
||||
def _register_extension_skills(
|
||||
self,
|
||||
manifest: ExtensionManifest,
|
||||
@@ -1404,17 +1462,11 @@ class ExtensionManager:
|
||||
ignore_fn = self._load_extensionignore(source_dir)
|
||||
shutil.copytree(source_dir, dest_dir, ignore=ignore_fn)
|
||||
|
||||
# Register commands with AI agents
|
||||
# Register commands with AI agents (active integration only, #2948)
|
||||
registered_commands = {}
|
||||
if register_commands:
|
||||
registrar = CommandRegistrar()
|
||||
# Register for all detected agents
|
||||
registered_commands = registrar.register_commands_for_all_agents(
|
||||
manifest,
|
||||
dest_dir,
|
||||
self.project_root,
|
||||
link_outputs=link_commands,
|
||||
create_missing_active_skills_dir=True,
|
||||
registered_commands = self._register_commands_for_active_agent(
|
||||
manifest, dest_dir, link_outputs=link_commands
|
||||
)
|
||||
|
||||
# Auto-register extension commands as agent skills when skills mode
|
||||
@@ -1701,11 +1753,10 @@ class ExtensionManager:
|
||||
"""Register installed, enabled extensions for ``agent_name``.
|
||||
|
||||
Command-file registration is scoped to the explicit ``agent_name``
|
||||
argument, so this method can be used after install, upgrade, or switch.
|
||||
Extension skill rendering is still scoped to the active ``ai`` /
|
||||
``ai_skills`` settings in init-options, so non-active skills-mode
|
||||
targets receive command files here. Per-agent skills parity is tracked
|
||||
separately in #2948.
|
||||
argument. Since #2948, callers pass the active agent only (``use`` /
|
||||
``switch`` activate the target first; ``upgrade`` calls it only for
|
||||
the active integration), so extension skill rendering — scoped to the
|
||||
active ``ai`` / ``ai_skills`` init-options — matches ``agent_name``.
|
||||
"""
|
||||
if not agent_name:
|
||||
return
|
||||
@@ -1765,13 +1816,11 @@ class ExtensionManager:
|
||||
# Extension *skills* are only ever rendered for the active agent:
|
||||
# `_register_extension_skills` resolves the skills dir and
|
||||
# frontmatter from init-options["ai"], ignoring ``agent_name``.
|
||||
# When this method runs for a non-active agent — as install/upgrade
|
||||
# now do for a secondary integration (#2886) — the skills pass would
|
||||
# re-render the *active* agent's extension skills as a side effect,
|
||||
# Running the skills pass for a non-active agent would re-render
|
||||
# the *active* agent's extension skills as a side effect,
|
||||
# resurrecting skill files the user deliberately deleted. Skip it
|
||||
# unless the target is the active agent; `switch` is unaffected
|
||||
# because it activates the target before registering. (Rendering
|
||||
# skills for a non-active target is tracked separately in #2948.)
|
||||
# unless the target is the active agent (defense in depth: since
|
||||
# #2948 callers only pass the active agent anyway).
|
||||
if agent_name == active_agent:
|
||||
try:
|
||||
registered_skills = self._register_extension_skills(
|
||||
@@ -1970,6 +2019,7 @@ class CommandRegistrar:
|
||||
project_root: Path,
|
||||
link_outputs: bool = False,
|
||||
create_missing_active_skills_dir: bool = False,
|
||||
only_agent: Optional[str] = None,
|
||||
) -> Dict[str, List[str]]:
|
||||
"""Register extension commands for all detected agents."""
|
||||
context_note = f"\n<!-- Extension: {manifest.id} -->\n<!-- Config: .specify/extensions/{manifest.id}/ -->\n"
|
||||
@@ -1981,6 +2031,7 @@ class CommandRegistrar:
|
||||
context_note=context_note,
|
||||
link_outputs=link_outputs,
|
||||
create_missing_active_skills_dir=create_missing_active_skills_dir,
|
||||
only_agent=only_agent,
|
||||
extension_id=manifest.id,
|
||||
)
|
||||
|
||||
|
||||
@@ -376,19 +376,14 @@ def _register_extensions_for_agent(
|
||||
"""Register all enabled extensions' commands/skills for ``agent_key``.
|
||||
|
||||
``use`` / ``switch`` re-register enabled extensions for the agent they
|
||||
activate; ``upgrade`` backfills them for the refreshed agent. Plain
|
||||
``install`` deliberately does not call this helper so adding a secondary
|
||||
integration has no extension side effects until it is selected or upgraded.
|
||||
See issue #2886.
|
||||
activate (rescaffold); ``upgrade`` does so only for the *active*
|
||||
integration. Plain ``install`` and upgrade of a non-active integration
|
||||
deliberately skip this helper so a secondary integration has no extension
|
||||
side effects until it is selected. See issues #2886 and #2948.
|
||||
|
||||
Known limitation: extension *skill* rendering is scoped to the active
|
||||
agent (init-options track a single ``ai`` / ``ai_skills`` pair). A
|
||||
skills-mode agent registered while it is *not* the active agent (e.g.
|
||||
Copilot ``--skills`` registered while non-active) therefore
|
||||
receives command files rather than skills here — matching ``extension
|
||||
add``'s multi-agent behavior. ``use`` / ``switch`` avoid this because they
|
||||
make the target the active agent first. Per-agent skills parity is tracked in
|
||||
#2948.
|
||||
Callers always pass the active agent (use/switch activate the target
|
||||
before registering), so extension *skill* rendering — which is scoped to
|
||||
the active ``ai`` / ``ai_skills`` init-options — matches ``agent_key``.
|
||||
|
||||
Best-effort: never aborts the surrounding integration operation. Callers
|
||||
invoke it *after* the use/upgrade/switch transaction has committed so a
|
||||
|
||||
@@ -491,17 +491,19 @@ def integration_upgrade(
|
||||
if stale_removed:
|
||||
console.print(f" Removed {len(stale_removed)} stale file(s) from previous install")
|
||||
|
||||
# Re-register enabled extensions for the upgraded agent so its extension
|
||||
# commands are (re)created — including agents installed before this
|
||||
# back-fill existed. Mirrors switch for command registration; see #2886.
|
||||
# Done after the upgrade has fully settled (Phase 2 included) and outside
|
||||
# the try/except above so this best-effort step cannot affect upgrade
|
||||
# success.
|
||||
_register_extensions_for_agent(
|
||||
project_root,
|
||||
key,
|
||||
continuing="The integration was upgraded, but installed extensions may need re-registration.",
|
||||
)
|
||||
# Re-register enabled extensions only when upgrading the *active*
|
||||
# integration, so its extension commands are (re)created after the
|
||||
# upgrade settled (Phase 2 included). Done outside the try/except above
|
||||
# so this best-effort step cannot affect upgrade success. Non-active
|
||||
# integrations are rescaffolded by `use` / `switch` instead — the #2886
|
||||
# back-fill for non-active agents was removed at maintainer request
|
||||
# (#2948).
|
||||
if key == installed_key:
|
||||
_register_extensions_for_agent(
|
||||
project_root,
|
||||
key,
|
||||
continuing="The integration was upgraded, but installed extensions may need re-registration.",
|
||||
)
|
||||
|
||||
name = (integration.config or {}).get("name", key)
|
||||
console.print(f"\n[green]✓[/green] Integration '{name}' upgraded successfully")
|
||||
|
||||
@@ -1371,6 +1371,53 @@ class TestIntegrationInstall:
|
||||
project / ".github" / "skills" / "speckit-git-feature" / "SKILL.md"
|
||||
).exists()
|
||||
|
||||
def test_extension_add_registers_active_integration_only(self, tmp_path):
|
||||
"""``extension add`` registers commands for the active integration only.
|
||||
|
||||
Maintainer-requested behavior for #2948: with multiple integrations
|
||||
installed, ``extension add`` must treat the project as single-active —
|
||||
only the current integration gets the new extension's commands.
|
||||
Non-active integrations receive them when selected via
|
||||
``integration use`` / ``switch`` (rescaffold).
|
||||
"""
|
||||
project = _init_project(tmp_path, "claude")
|
||||
|
||||
result = _run_in_project(project, [
|
||||
"integration", "install", "codex",
|
||||
"--script", "sh",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
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, "active integration gets the extension"
|
||||
assert "codex" not in registered, (
|
||||
"non-active integration must not be registered on add (#2948)"
|
||||
)
|
||||
assert (
|
||||
project / ".claude" / "skills" / "speckit-git-feature" / "SKILL.md"
|
||||
).exists()
|
||||
assert not (
|
||||
project / ".agents" / "skills" / "speckit-git-feature" / "SKILL.md"
|
||||
).exists()
|
||||
|
||||
# Selecting the other integration rescaffolds it with the extension.
|
||||
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 registers extensions for the new active agent"
|
||||
assert (
|
||||
project / ".agents" / "skills" / "speckit-git-feature" / "SKILL.md"
|
||||
).exists()
|
||||
|
||||
|
||||
# ── uninstall ────────────────────────────────────────────────────────
|
||||
|
||||
@@ -2492,13 +2539,13 @@ 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.
|
||||
def test_upgrade_does_not_backfill_non_active_integration(self, tmp_path):
|
||||
"""Upgrading a non-active integration must not register extensions for it.
|
||||
|
||||
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``.
|
||||
Maintainer-requested behavior for #2948 (reverses the #2886 upgrade
|
||||
back-fill): non-active integrations only receive extension artifacts
|
||||
when selected via ``integration use`` / ``switch``. Upgrade of a
|
||||
non-active integration refreshes its own files and nothing else.
|
||||
"""
|
||||
project = _init_project(tmp_path, "claude")
|
||||
|
||||
@@ -2511,21 +2558,10 @@ class TestIntegrationUpgrade:
|
||||
])
|
||||
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",
|
||||
@@ -2533,12 +2569,41 @@ class TestIntegrationUpgrade:
|
||||
])
|
||||
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()
|
||||
assert "codex" not in registered, (
|
||||
"upgrade must not back-fill non-active integrations (#2948)"
|
||||
)
|
||||
assert not (
|
||||
project / ".agents" / "skills" / "speckit-git-feature" / "SKILL.md"
|
||||
).exists()
|
||||
|
||||
def test_upgrade_active_integration_reregisters_extensions(self, tmp_path):
|
||||
"""Upgrading the active integration restores its extension commands.
|
||||
|
||||
The active integration keeps the re-registration pass on upgrade so
|
||||
missing or stale extension command files are recreated (#2948 scopes
|
||||
the pass to the active integration; #2886 introduced it).
|
||||
"""
|
||||
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}"
|
||||
|
||||
cmd_file = project / ".claude" / "skills" / "speckit-git-feature" / "SKILL.md"
|
||||
assert cmd_file.exists(), "precondition: extension command registered"
|
||||
cmd_file.unlink()
|
||||
|
||||
result = _run_in_project(project, [
|
||||
"integration", "upgrade", "claude",
|
||||
"--script", "sh",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
assert cmd_file.exists(), (
|
||||
"upgrade of the active integration re-registers extension commands"
|
||||
)
|
||||
|
||||
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.
|
||||
|
||||
@@ -1148,6 +1148,7 @@ class TestExtensionManager:
|
||||
link_outputs=False,
|
||||
create_missing_active_skills_dir=False,
|
||||
extension_id=None,
|
||||
only_agent=None,
|
||||
):
|
||||
captured["create_missing_active_skills_dir"] = (
|
||||
create_missing_active_skills_dir
|
||||
@@ -7448,7 +7449,7 @@ $ARGUMENTS
|
||||
from specify_cli import app
|
||||
from specify_cli.agents import CommandRegistrar
|
||||
|
||||
project_dir, ext_dir, claude_commands_dir = self._setup_mock_extension(tmp_path, "claude")
|
||||
project_dir, ext_dir, agents_commands_dir = self._setup_mock_extension(tmp_path, "amp")
|
||||
|
||||
# 3. Run specify extension add
|
||||
runner = CliRunner()
|
||||
@@ -7465,8 +7466,8 @@ $ARGUMENTS
|
||||
assert "speckit-mock-ext-hello" not in result.output
|
||||
|
||||
# Verify on-disk command names are dotted
|
||||
hello_file = claude_commands_dir / "speckit.mock-ext.hello.md"
|
||||
greet_file = claude_commands_dir / "speckit.mock-ext.greet.md"
|
||||
hello_file = agents_commands_dir / "speckit.mock-ext.hello.md"
|
||||
greet_file = agents_commands_dir / "speckit.mock-ext.greet.md"
|
||||
|
||||
assert hello_file.exists()
|
||||
assert greet_file.exists()
|
||||
|
||||
Reference in New Issue
Block a user