Skip to content

Commit b68eec5

Browse files
committed
fix: isolate per-extension failures in register_enabled_extensions_for_agent
The per-extension loop had no error isolation: if registering one enabled extension raised (e.g. an OSError writing a command file), the loop aborted and the exception propagated, so every subsequent enabled extension was silently skipped. Callers wrap the whole call in a single best-effort try/except, so the wholesale abort surfaced as one warning while the command still exited 0 — leaving the agent with only a prefix of its extensions. Wrap the per-extension body in try/except: warn (naming the extension) and continue, so one bad extension can no longer drop the others. Add a regression test that forces the first-iterated extension to raise and asserts the rest still register. Closes #2950
1 parent 1b0556c commit b68eec5

2 files changed

Lines changed: 93 additions & 27 deletions

File tree

src/specify_cli/extensions.py

Lines changed: 45 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1677,35 +1677,53 @@ def register_enabled_extensions_for_agent(self, agent_name: str) -> None:
16771677
continue
16781678

16791679
ext_dir = self.extensions_dir / ext_id
1680-
updates: Dict[str, Any] = {}
16811680

1682-
if agent_config and not skills_mode_active:
1683-
registered = registrar.register_commands_for_agent(
1684-
agent_name, manifest, ext_dir, self.project_root
1685-
)
1686-
registered_commands = metadata.get("registered_commands", {})
1687-
if not isinstance(registered_commands, dict):
1688-
registered_commands = {}
1689-
new_registered = copy.deepcopy(registered_commands)
1690-
if registered:
1691-
new_registered[agent_name] = registered
1692-
else:
1693-
# Registration returned empty list (e.g., corrupted
1694-
# manifest pointing at missing command files). Clear
1695-
# stale entry so later cleanup doesn't try to remove
1696-
# files that were never written.
1697-
new_registered.pop(agent_name, None)
1698-
if new_registered != registered_commands:
1699-
updates["registered_commands"] = new_registered
1700-
1701-
registered_skills = self._register_extension_skills(manifest, ext_dir)
1702-
if registered_skills:
1703-
existing_skills = self._valid_name_list(metadata.get("registered_skills", []))
1704-
merged_skills = list(dict.fromkeys(existing_skills + registered_skills))
1705-
updates["registered_skills"] = merged_skills
1681+
# Isolate per-extension failures: one extension that fails to
1682+
# register (e.g. an OSError writing a command file) must not abort
1683+
# registration of the remaining enabled extensions for this agent.
1684+
try:
1685+
updates: Dict[str, Any] = {}
17061686

1707-
if updates:
1708-
self.registry.update(ext_id, updates)
1687+
if agent_config and not skills_mode_active:
1688+
registered = registrar.register_commands_for_agent(
1689+
agent_name, manifest, ext_dir, self.project_root
1690+
)
1691+
registered_commands = metadata.get("registered_commands", {})
1692+
if not isinstance(registered_commands, dict):
1693+
registered_commands = {}
1694+
new_registered = copy.deepcopy(registered_commands)
1695+
if registered:
1696+
new_registered[agent_name] = registered
1697+
else:
1698+
# Registration returned empty list (e.g., corrupted
1699+
# manifest pointing at missing command files). Clear
1700+
# stale entry so later cleanup doesn't try to remove
1701+
# files that were never written.
1702+
new_registered.pop(agent_name, None)
1703+
if new_registered != registered_commands:
1704+
updates["registered_commands"] = new_registered
1705+
1706+
registered_skills = self._register_extension_skills(manifest, ext_dir)
1707+
if registered_skills:
1708+
existing_skills = self._valid_name_list(metadata.get("registered_skills", []))
1709+
merged_skills = list(dict.fromkeys(existing_skills + registered_skills))
1710+
updates["registered_skills"] = merged_skills
1711+
1712+
if updates:
1713+
self.registry.update(ext_id, updates)
1714+
except Exception as ext_err:
1715+
# Best-effort per extension: warn and move on so a single bad
1716+
# extension cannot silently drop the others. See #2950.
1717+
from . import _print_cli_warning
1718+
1719+
_print_cli_warning(
1720+
"register extension artifacts for",
1721+
"extension",
1722+
ext_id,
1723+
ext_err,
1724+
continuing="Continuing with the remaining extensions.",
1725+
)
1726+
continue
17091727

17101728
def list_installed(self) -> List[Dict[str, Any]]:
17111729
"""List all installed extensions with metadata.

tests/test_extension_skills.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -980,6 +980,54 @@ def test_non_boolean_ai_skills_does_not_skip_default_agent_reregistration(
980980
assert metadata["registered_skills"] == []
981981
assert (project_dir / ".github" / "agents").is_dir()
982982

983+
def test_one_failing_extension_does_not_abort_the_rest(
984+
self, project_dir, temp_dir, monkeypatch
985+
):
986+
"""A single failing extension must not block registration of the others.
987+
988+
Regression for #2950: ``register_enabled_extensions_for_agent`` iterates
989+
enabled extensions; before the per-extension isolation, the first one
990+
that raised (e.g. an OSError writing a command file) aborted the loop and
991+
the exception propagated, so every later extension was silently skipped.
992+
"""
993+
from specify_cli.extensions import CommandRegistrar
994+
995+
_create_init_options(project_dir, ai="claude", ai_skills=False)
996+
manager = ExtensionManager(project_dir)
997+
# Two enabled extensions; the first one iterated ("aaa-fail") will raise.
998+
manager.install_from_directory(
999+
_create_extension_dir(temp_dir, ext_id="aaa-fail"), "0.1.0",
1000+
register_commands=False,
1001+
)
1002+
manager.install_from_directory(
1003+
_create_extension_dir(temp_dir, ext_id="bbb-ok"), "0.1.0",
1004+
register_commands=False,
1005+
)
1006+
1007+
original = CommandRegistrar.register_commands_for_agent
1008+
1009+
def flaky(self, agent_name, manifest, ext_dir, project_root, link_outputs=False):
1010+
if manifest.id == "aaa-fail":
1011+
raise OSError("simulated command-file write failure")
1012+
return original(
1013+
self, agent_name, manifest, ext_dir, project_root,
1014+
link_outputs=link_outputs,
1015+
)
1016+
1017+
monkeypatch.setattr(CommandRegistrar, "register_commands_for_agent", flaky)
1018+
1019+
# Must not propagate, despite the first extension failing.
1020+
manager.register_enabled_extensions_for_agent("claude")
1021+
1022+
# The healthy extension was still registered for the agent...
1023+
ok_meta = manager.registry.get("bbb-ok")
1024+
assert "claude" in ok_meta["registered_commands"], (
1025+
"a later extension must still register after an earlier one fails (#2950)"
1026+
)
1027+
# ...and the failing one was not.
1028+
fail_meta = manager.registry.get("aaa-fail")
1029+
assert "claude" not in fail_meta.get("registered_commands", {})
1030+
9831031
def test_existing_agent_command_path_file_is_not_detected(
9841032
self, project_dir, temp_dir
9851033
):

0 commit comments

Comments
 (0)