From b68eec595fc2d600d9e982ba222dc5e2ce75f47e Mon Sep 17 00:00:00 2001 From: Pascal Date: Fri, 12 Jun 2026 11:50:01 +0200 Subject: [PATCH] fix: isolate per-extension failures in register_enabled_extensions_for_agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/specify_cli/extensions.py | 72 +++++++++++++++++++++------------- tests/test_extension_skills.py | 48 +++++++++++++++++++++++ 2 files changed, 93 insertions(+), 27 deletions(-) diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index db53b7997f..8f1e4fe299 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -1677,35 +1677,53 @@ def register_enabled_extensions_for_agent(self, agent_name: str) -> None: continue ext_dir = self.extensions_dir / ext_id - updates: Dict[str, Any] = {} - if agent_config and not skills_mode_active: - registered = registrar.register_commands_for_agent( - agent_name, manifest, ext_dir, self.project_root - ) - registered_commands = metadata.get("registered_commands", {}) - if not isinstance(registered_commands, dict): - registered_commands = {} - new_registered = copy.deepcopy(registered_commands) - if registered: - new_registered[agent_name] = registered - else: - # Registration returned empty list (e.g., corrupted - # manifest pointing at missing command files). Clear - # stale entry so later cleanup doesn't try to remove - # files that were never written. - new_registered.pop(agent_name, None) - if new_registered != registered_commands: - updates["registered_commands"] = new_registered - - registered_skills = self._register_extension_skills(manifest, ext_dir) - if registered_skills: - existing_skills = self._valid_name_list(metadata.get("registered_skills", [])) - merged_skills = list(dict.fromkeys(existing_skills + registered_skills)) - updates["registered_skills"] = merged_skills + # Isolate per-extension failures: one extension that fails to + # register (e.g. an OSError writing a command file) must not abort + # registration of the remaining enabled extensions for this agent. + try: + updates: Dict[str, Any] = {} - if updates: - self.registry.update(ext_id, updates) + if agent_config and not skills_mode_active: + registered = registrar.register_commands_for_agent( + agent_name, manifest, ext_dir, self.project_root + ) + registered_commands = metadata.get("registered_commands", {}) + if not isinstance(registered_commands, dict): + registered_commands = {} + new_registered = copy.deepcopy(registered_commands) + if registered: + new_registered[agent_name] = registered + else: + # Registration returned empty list (e.g., corrupted + # manifest pointing at missing command files). Clear + # stale entry so later cleanup doesn't try to remove + # files that were never written. + new_registered.pop(agent_name, None) + if new_registered != registered_commands: + updates["registered_commands"] = new_registered + + registered_skills = self._register_extension_skills(manifest, ext_dir) + if registered_skills: + existing_skills = self._valid_name_list(metadata.get("registered_skills", [])) + merged_skills = list(dict.fromkeys(existing_skills + registered_skills)) + updates["registered_skills"] = merged_skills + + if updates: + self.registry.update(ext_id, updates) + except Exception as ext_err: + # Best-effort per extension: warn and move on so a single bad + # extension cannot silently drop the others. See #2950. + from . import _print_cli_warning + + _print_cli_warning( + "register extension artifacts for", + "extension", + ext_id, + ext_err, + continuing="Continuing with the remaining extensions.", + ) + continue def list_installed(self) -> List[Dict[str, Any]]: """List all installed extensions with metadata. diff --git a/tests/test_extension_skills.py b/tests/test_extension_skills.py index 67eda3b6cb..977451a224 100644 --- a/tests/test_extension_skills.py +++ b/tests/test_extension_skills.py @@ -980,6 +980,54 @@ def test_non_boolean_ai_skills_does_not_skip_default_agent_reregistration( assert metadata["registered_skills"] == [] assert (project_dir / ".github" / "agents").is_dir() + def test_one_failing_extension_does_not_abort_the_rest( + self, project_dir, temp_dir, monkeypatch + ): + """A single failing extension must not block registration of the others. + + Regression for #2950: ``register_enabled_extensions_for_agent`` iterates + enabled extensions; before the per-extension isolation, the first one + that raised (e.g. an OSError writing a command file) aborted the loop and + the exception propagated, so every later extension was silently skipped. + """ + from specify_cli.extensions import CommandRegistrar + + _create_init_options(project_dir, ai="claude", ai_skills=False) + manager = ExtensionManager(project_dir) + # Two enabled extensions; the first one iterated ("aaa-fail") will raise. + manager.install_from_directory( + _create_extension_dir(temp_dir, ext_id="aaa-fail"), "0.1.0", + register_commands=False, + ) + manager.install_from_directory( + _create_extension_dir(temp_dir, ext_id="bbb-ok"), "0.1.0", + register_commands=False, + ) + + original = CommandRegistrar.register_commands_for_agent + + def flaky(self, agent_name, manifest, ext_dir, project_root, link_outputs=False): + if manifest.id == "aaa-fail": + raise OSError("simulated command-file write failure") + return original( + self, agent_name, manifest, ext_dir, project_root, + link_outputs=link_outputs, + ) + + monkeypatch.setattr(CommandRegistrar, "register_commands_for_agent", flaky) + + # Must not propagate, despite the first extension failing. + manager.register_enabled_extensions_for_agent("claude") + + # The healthy extension was still registered for the agent... + ok_meta = manager.registry.get("bbb-ok") + assert "claude" in ok_meta["registered_commands"], ( + "a later extension must still register after an earlier one fails (#2950)" + ) + # ...and the failing one was not. + fail_meta = manager.registry.get("aaa-fail") + assert "claude" not in fail_meta.get("registered_commands", {}) + def test_existing_agent_command_path_file_is_not_detected( self, project_dir, temp_dir ):