Skip to content

Commit acab576

Browse files
committed
fix: also render skills for non-active agents via 'extension add' (#2948)
Addresses PR review feedback: install_from_directory() (the 'extension add' / dev-install path) still called _register_extension_skills() with no agent_name, so it remained scoped to only the active agent, unlike register_enabled_extensions_for_agent() which was already fixed to target a specific agent. Changes: - Add ExtensionManager._register_extension_skills_for_installed_agents(), which renders skills for the active agent (legacy single-agent projects) plus every agent listed in .specify/integration.json's installed_integrations, for whichever of those are individually in skills mode. Failures for one agent are warned and do not block the others. - install_from_directory() now calls this helper instead of a single active-agent-only call. - _get_skills_dir()'s agent_name branch now mirrors resolve_active_skills_dir()'s safety checks (_ensure_safe_shared_directory symlink/containment/is-a-directory validation, plus the Kimi native-skills-dir-must-already-exist fallback) instead of resolving the naive per-agent path unchecked — fixes a regression this exposed in the Hermes marker-file detection test and restores exact parity with the active-agent path. Tests: - Added TestNonActiveAgentSkillRegistration with 3 new tests covering register_enabled_extensions_for_agent for a non-active skills-mode agent, isolation from the active agent's own skills, and extension add rendering skills for all installed skills-mode agents. - pytest tests/ -k extension: 619/619 pass (616 previous + 3 new). - Manually verified: 'specify extension add git' with Claude active and Copilot installed in --skills mode now renders SKILL.md files for both agents ('5 agent skill(s) auto-registered'), with no change to Claude's own skills. - git diff --check: clean, no whitespace errors.
1 parent 082e421 commit acab576

2 files changed

Lines changed: 220 additions & 4 deletions

File tree

src/specify_cli/extensions/__init__.py

Lines changed: 92 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -974,10 +974,29 @@ def _ensure_usable(skills_dir: Path) -> Optional[Path]:
974974
if not isinstance(selected_ai, str) or not selected_ai:
975975
return _ensure_usable(skills_dir)
976976
else:
977-
if not is_agent_skills_enabled(self.project_root, agent_name, opts):
977+
from ..shared_infra import _ensure_safe_shared_directory
978+
979+
ai_skills_enabled = is_agent_skills_enabled(
980+
self.project_root, agent_name, opts
981+
)
982+
if not ai_skills_enabled and agent_name != "kimi":
978983
return None
979984
try:
980985
skills_dir = resolve_agent_skills_dir(self.project_root, agent_name)
986+
if not ai_skills_enabled:
987+
# Kimi native-skills fallback: only use the directory if
988+
# it already exists; do not create it on demand.
989+
if not skills_dir.is_dir():
990+
return None
991+
_ensure_safe_shared_directory(
992+
self.project_root, skills_dir,
993+
create=False, context="agent skills directory",
994+
)
995+
else:
996+
_ensure_safe_shared_directory(
997+
self.project_root, skills_dir,
998+
context="agent skills directory",
999+
)
9811000
except (ValueError, OSError) as exc:
9821001
_print_cli_warning(
9831002
"resolve",
@@ -1191,6 +1210,73 @@ def _replacement(match: re.Match[str]) -> str:
11911210

11921211
return written
11931212

1213+
def _register_extension_skills_for_installed_agents(
1214+
self,
1215+
manifest: ExtensionManifest,
1216+
extension_dir: Path,
1217+
link_outputs: bool = False,
1218+
) -> List[str]:
1219+
"""Render extension skills for every skills-mode agent detected.
1220+
1221+
Used by paths (like ``extension add``) that register an extension
1222+
for all agents at once, rather than a single explicit target agent.
1223+
Checks the active agent (legacy single-agent projects) plus every
1224+
agent recorded in ``.specify/integration.json``'s installed
1225+
integrations, and renders skills for whichever of those have skills
1226+
mode enabled for them specifically, instead of only the active
1227+
agent (#2948).
1228+
"""
1229+
from .. import load_init_options
1230+
from ..integration_state import (
1231+
try_read_integration_json,
1232+
installed_integration_keys,
1233+
)
1234+
1235+
opts = load_init_options(self.project_root)
1236+
if not isinstance(opts, dict):
1237+
opts = {}
1238+
1239+
candidate_agents: List[str] = []
1240+
active_agent = opts.get("ai")
1241+
if isinstance(active_agent, str) and active_agent:
1242+
candidate_agents.append(active_agent)
1243+
1244+
state, _error = try_read_integration_json(self.project_root)
1245+
if state:
1246+
for key in installed_integration_keys(state):
1247+
if key not in candidate_agents:
1248+
candidate_agents.append(key)
1249+
1250+
combined: List[str] = []
1251+
seen = set()
1252+
for agent_name in candidate_agents:
1253+
try:
1254+
agent_skills = self._register_extension_skills(
1255+
manifest,
1256+
extension_dir,
1257+
link_outputs=link_outputs,
1258+
agent_name=agent_name,
1259+
)
1260+
except Exception as skills_err:
1261+
from .. import _print_cli_warning
1262+
_print_cli_warning(
1263+
"register extension skills for",
1264+
"extension",
1265+
manifest.id,
1266+
skills_err,
1267+
continuing=(
1268+
"Continuing with available registration results for "
1269+
"this extension and the remaining agents."
1270+
),
1271+
)
1272+
continue
1273+
for skill_name in agent_skills:
1274+
if skill_name not in seen:
1275+
seen.add(skill_name)
1276+
combined.append(skill_name)
1277+
1278+
return combined
1279+
11941280
@staticmethod
11951281
def _is_expected_dev_symlink(skill_file: Path, cache_file: Path) -> bool:
11961282
"""Return True when an existing skill file links to its dev cache."""
@@ -1475,9 +1561,11 @@ def install_from_directory(
14751561
create_missing_active_skills_dir=True,
14761562
)
14771563

1478-
# Auto-register extension commands as agent skills when skills mode
1479-
# was used during project initialisation (feature parity).
1480-
registered_skills = self._register_extension_skills(
1564+
# Auto-register extension commands as agent skills for every
1565+
# skills-mode agent detected (active agent plus any other
1566+
# installed integrations in skills mode), not just the active
1567+
# agent (#2948).
1568+
registered_skills = self._register_extension_skills_for_installed_agents(
14811569
manifest, dest_dir, link_outputs=link_commands
14821570
)
14831571

tests/test_extension_skills.py

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,40 @@ def no_skills_project(project_dir):
219219
return project_dir
220220

221221

222+
def _create_integration_json(
223+
project_root: Path,
224+
*,
225+
default_agent: str,
226+
installed: list,
227+
skills_by_agent: dict,
228+
):
229+
"""Write a .specify/integration.json with per-agent skills settings.
230+
231+
``skills_by_agent`` maps agent key -> bool for
232+
``integration_settings[agent].parsed_options.skills``.
233+
"""
234+
specify_dir = project_root / ".specify"
235+
specify_dir.mkdir(parents=True, exist_ok=True)
236+
settings = {}
237+
for agent in installed:
238+
entry = {"script": "sh", "invoke_separator": "-"}
239+
if agent in skills_by_agent:
240+
entry["raw_options"] = "--skills" if skills_by_agent[agent] else ""
241+
entry["parsed_options"] = {"skills": skills_by_agent[agent]}
242+
settings[agent] = entry
243+
payload = {
244+
"version": "0.1.0",
245+
"integration_state_schema": 1,
246+
"installed_integrations": installed,
247+
"integration_settings": settings,
248+
"integration": default_agent,
249+
"default_integration": default_agent,
250+
}
251+
(specify_dir / "integration.json").write_text(
252+
json.dumps(payload), encoding="utf-8"
253+
)
254+
255+
222256
# ===== ExtensionManager._get_skills_dir Tests =====
223257

224258
class TestExtensionManagerGetSkillsDir:
@@ -1906,3 +1940,97 @@ def test_remove_cleans_up_when_ai_skills_toggled(self, skills_project, extension
19061940
assert result is True
19071941
assert not (skills_dir / "speckit-test-ext-hello").exists()
19081942
assert not (skills_dir / "speckit-test-ext-world").exists()
1943+
1944+
1945+
# ===== Per-agent (non-active) skills mode tests (#2948) =====
1946+
class TestNonActiveAgentSkillRegistration:
1947+
"""Skills should render for any skills-mode agent, not just the active one."""
1948+
1949+
def test_register_enabled_extensions_for_agent_renders_non_active_agent_skills(
1950+
self, project_dir, extension_dir
1951+
):
1952+
"""upgrade/install-style registration should target the given agent."""
1953+
_create_init_options(project_dir, ai="claude", ai_skills=False)
1954+
_create_integration_json(
1955+
project_dir,
1956+
default_agent="claude",
1957+
installed=["claude", "copilot"],
1958+
skills_by_agent={"copilot": True},
1959+
)
1960+
copilot_skills_dir = _create_skills_dir(project_dir, ai="copilot")
1961+
1962+
manager = ExtensionManager(project_dir)
1963+
manager.install_from_directory(
1964+
extension_dir, "0.1.0", register_commands=False
1965+
)
1966+
manifest = manager.get_extension("test-ext")
1967+
1968+
manager.register_enabled_extensions_for_agent("copilot")
1969+
1970+
metadata = manager.registry.get(manifest.id)
1971+
assert "speckit-test-ext-hello" in metadata["registered_skills"]
1972+
assert (
1973+
copilot_skills_dir / "speckit-test-ext-hello" / "SKILL.md"
1974+
).exists()
1975+
1976+
def test_register_enabled_extensions_for_agent_does_not_affect_active_agent(
1977+
self, project_dir, extension_dir
1978+
):
1979+
"""Rendering skills for a non-active agent must not touch the active agent's own skills."""
1980+
_create_init_options(project_dir, ai="claude", ai_skills=True)
1981+
claude_skills_dir = _create_skills_dir(project_dir, ai="claude")
1982+
_create_integration_json(
1983+
project_dir,
1984+
default_agent="claude",
1985+
installed=["claude", "copilot"],
1986+
skills_by_agent={"copilot": True},
1987+
)
1988+
copilot_skills_dir = _create_skills_dir(project_dir, ai="copilot")
1989+
1990+
manager = ExtensionManager(project_dir)
1991+
manager.install_from_directory(
1992+
extension_dir, "0.1.0", register_commands=False
1993+
)
1994+
1995+
# Simulate the user deleting the claude skill files before re-running
1996+
# registration for a different (copilot) agent.
1997+
claude_skill_dir = claude_skills_dir / "speckit-test-ext-hello"
1998+
shutil.rmtree(claude_skill_dir)
1999+
assert not claude_skill_dir.exists()
2000+
2001+
manager.register_enabled_extensions_for_agent("copilot")
2002+
2003+
# Copilot's skills were rendered...
2004+
assert (
2005+
copilot_skills_dir / "speckit-test-ext-hello" / "SKILL.md"
2006+
).exists()
2007+
# ...but claude's deleted skill was not resurrected.
2008+
assert not claude_skill_dir.exists()
2009+
2010+
def test_extension_add_renders_skills_for_all_installed_skills_mode_agents(
2011+
self, project_dir, extension_dir
2012+
):
2013+
"""extension add should render skills for every installed skills-mode agent."""
2014+
_create_init_options(project_dir, ai="claude", ai_skills=True)
2015+
claude_skills_dir = _create_skills_dir(project_dir, ai="claude")
2016+
_create_integration_json(
2017+
project_dir,
2018+
default_agent="claude",
2019+
installed=["claude", "copilot"],
2020+
skills_by_agent={"copilot": True},
2021+
)
2022+
copilot_skills_dir = _create_skills_dir(project_dir, ai="copilot")
2023+
2024+
manager = ExtensionManager(project_dir)
2025+
manifest = manager.install_from_directory(
2026+
extension_dir, "0.1.0", register_commands=False
2027+
)
2028+
2029+
metadata = manager.registry.get(manifest.id)
2030+
assert "speckit-test-ext-hello" in metadata["registered_skills"]
2031+
assert (
2032+
claude_skills_dir / "speckit-test-ext-hello" / "SKILL.md"
2033+
).exists()
2034+
assert (
2035+
copilot_skills_dir / "speckit-test-ext-hello" / "SKILL.md"
2036+
).exists()

0 commit comments

Comments
 (0)