Skip to content

Commit ce01877

Browse files
authored
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
1 parent afe7657 commit ce01877

5 files changed

Lines changed: 437 additions & 71 deletions

File tree

src/specify_cli/extensions/__init__.py

Lines changed: 45 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1678,16 +1678,12 @@ def unregister_agent_artifacts(self, agent_name: str) -> None:
16781678
def register_enabled_extensions_for_agent(self, agent_name: str) -> None:
16791679
"""Register installed, enabled extensions for ``agent_name``.
16801680
1681-
This is intended to be called after switching integrations. Command
1682-
registration is scoped to the explicit ``agent_name`` argument, but some
1683-
behavior still depends on the current init-options state (for example,
1684-
skills-mode handling uses the active ``ai`` / ``ai_skills`` settings).
1685-
1686-
Callers should therefore pass the agent that has just been made active
1687-
in init-options; in normal use, ``agent_name`` is expected to match the
1688-
current ``ai`` value. This mirrors extension install behavior while
1689-
avoiding stale default-mode command directories when that active agent
1690-
is running in skills mode (notably Copilot ``--skills``).
1681+
Command-file registration is scoped to the explicit ``agent_name``
1682+
argument, so this method can be used after install, upgrade, or switch.
1683+
Extension skill rendering is still scoped to the active ``ai`` /
1684+
``ai_skills`` settings in init-options, so non-active skills-mode
1685+
targets receive command files here. Per-agent skills parity is tracked
1686+
separately in #2948.
16911687
"""
16921688
if not agent_name:
16931689
return
@@ -1744,31 +1740,46 @@ def register_enabled_extensions_for_agent(self, agent_name: str) -> None:
17441740
if new_registered != registered_commands:
17451741
updates["registered_commands"] = new_registered
17461742

1747-
try:
1748-
registered_skills = self._register_extension_skills(manifest, ext_dir)
1749-
except Exception as skills_err:
1750-
# Skills are a companion artifact. If command registration
1751-
# already succeeded, still persist it so later cleanup can
1752-
# find those command files.
1753-
from .. import _print_cli_warning
1754-
1755-
_print_cli_warning(
1756-
"register extension skills for",
1757-
"extension",
1758-
ext_id,
1759-
skills_err,
1760-
continuing=(
1761-
"Continuing with available registration results for this "
1762-
"extension and the remaining extensions."
1763-
),
1764-
)
1765-
else:
1766-
if registered_skills:
1767-
existing_skills = self._valid_name_list(
1768-
metadata.get("registered_skills", [])
1743+
# Extension *skills* are only ever rendered for the active agent:
1744+
# `_register_extension_skills` resolves the skills dir and
1745+
# frontmatter from init-options["ai"], ignoring ``agent_name``.
1746+
# When this method runs for a non-active agent — as install/upgrade
1747+
# now do for a secondary integration (#2886) — the skills pass would
1748+
# re-render the *active* agent's extension skills as a side effect,
1749+
# resurrecting skill files the user deliberately deleted. Skip it
1750+
# unless the target is the active agent; `switch` is unaffected
1751+
# because it activates the target before registering. (Rendering
1752+
# skills for a non-active target is tracked separately in #2948.)
1753+
if agent_name == active_agent:
1754+
try:
1755+
registered_skills = self._register_extension_skills(
1756+
manifest, ext_dir
1757+
)
1758+
except Exception as skills_err:
1759+
# Skills are a companion artifact. If command registration
1760+
# already succeeded, still persist it so later cleanup can
1761+
# find those command files.
1762+
from .. import _print_cli_warning
1763+
1764+
_print_cli_warning(
1765+
"register extension skills for",
1766+
"extension",
1767+
ext_id,
1768+
skills_err,
1769+
continuing=(
1770+
"Continuing with available registration results for this "
1771+
"extension and the remaining extensions."
1772+
),
17691773
)
1770-
merged_skills = list(dict.fromkeys(existing_skills + registered_skills))
1771-
updates["registered_skills"] = merged_skills
1774+
else:
1775+
if registered_skills:
1776+
existing_skills = self._valid_name_list(
1777+
metadata.get("registered_skills", [])
1778+
)
1779+
merged_skills = list(
1780+
dict.fromkeys(existing_skills + registered_skills)
1781+
)
1782+
updates["registered_skills"] = merged_skills
17721783

17731784
if updates:
17741785
self.registry.update(ext_id, updates)

src/specify_cli/integrations/_helpers.py

Lines changed: 88 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
import os
55
from pathlib import Path
6-
from typing import Any
6+
from typing import Any, Callable
77

88
import typer
99

@@ -387,6 +387,93 @@ def _set_default_integration_or_exit(*args: Any, **kwargs: Any) -> None:
387387
raise typer.Exit(1)
388388

389389

390+
# ---------------------------------------------------------------------------
391+
# Extension (un)registration helpers (shared by use / switch / upgrade)
392+
# ---------------------------------------------------------------------------
393+
394+
def _best_effort_extension_op(
395+
project_root: Path,
396+
agent_key: str,
397+
op: Callable[[Any, str], None],
398+
*,
399+
phase: str,
400+
continuing: str,
401+
) -> None:
402+
"""Run a best-effort ``ExtensionManager`` operation for ``agent_key``.
403+
404+
``op`` receives the ``ExtensionManager`` and ``agent_key``. Any failure is
405+
surfaced as a warning via ``_print_cli_warning`` and never aborts the
406+
surrounding integration operation. ``continuing`` describes what already
407+
succeeded so the warning makes the partial outcome clear.
408+
"""
409+
try:
410+
from ..extensions import ExtensionManager
411+
412+
ext_mgr = ExtensionManager(project_root)
413+
op(ext_mgr, agent_key)
414+
except Exception as ext_err:
415+
from .. import _print_cli_warning
416+
417+
_print_cli_warning(phase, "integration", agent_key, ext_err, continuing=continuing)
418+
419+
420+
def _register_extensions_for_agent(
421+
project_root: Path,
422+
agent_key: str,
423+
*,
424+
continuing: str,
425+
) -> None:
426+
"""Register all enabled extensions' commands/skills for ``agent_key``.
427+
428+
``use`` / ``switch`` re-register enabled extensions for the agent they
429+
activate; ``upgrade`` backfills them for the refreshed agent. Plain
430+
``install`` deliberately does not call this helper so adding a secondary
431+
integration has no extension side effects until it is selected or upgraded.
432+
See issue #2886.
433+
434+
Known limitation: extension *skill* rendering is scoped to the active
435+
agent (init-options track a single ``ai`` / ``ai_skills`` pair). A
436+
skills-mode agent registered while it is *not* the active agent (e.g.
437+
Copilot ``--skills`` registered while non-active) therefore
438+
receives command files rather than skills here — matching ``extension
439+
add``'s multi-agent behavior. ``use`` / ``switch`` avoid this because they
440+
make the target the active agent first. Per-agent skills parity is tracked in
441+
#2948.
442+
443+
Best-effort: never aborts the surrounding integration operation. Callers
444+
invoke it *after* the use/upgrade/switch transaction has committed so a
445+
failure here cannot trigger a rollback.
446+
"""
447+
_best_effort_extension_op(
448+
project_root,
449+
agent_key,
450+
lambda mgr, key: mgr.register_enabled_extensions_for_agent(key),
451+
phase="register extension artifacts for",
452+
continuing=continuing,
453+
)
454+
455+
456+
def _unregister_extensions_for_agent(
457+
project_root: Path,
458+
agent_key: str,
459+
*,
460+
continuing: str,
461+
) -> None:
462+
"""Best-effort removal of ``agent_key``'s extension artifacts.
463+
464+
Used by ``switch`` when uninstalling the previous integration so its
465+
extension command/skill files don't linger as orphans in the old agent's
466+
directory.
467+
"""
468+
_best_effort_extension_op(
469+
project_root,
470+
agent_key,
471+
lambda mgr, key: mgr.unregister_agent_artifacts(key),
472+
phase="clean up extension artifacts for",
473+
continuing=continuing,
474+
)
475+
476+
390477
# ---------------------------------------------------------------------------
391478
# CLI formatting helpers (re-exported from _commands.py)
392479
# ---------------------------------------------------------------------------

src/specify_cli/integrations/_migrate_commands.py

Lines changed: 36 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,14 @@
2727
_get_speckit_version,
2828
_read_integration_json,
2929
_refresh_init_options_speckit_version,
30+
_register_extensions_for_agent,
3031
_remove_integration_json,
3132
_resolve_integration_options,
3233
_resolve_integration_script_type,
3334
_resolve_script_type,
3435
_set_default_integration,
3536
_set_default_integration_or_exit,
37+
_unregister_extensions_for_agent,
3638
_update_init_options_for_integration,
3739
_write_integration_json,
3840
)
@@ -120,6 +122,14 @@ def integration_switch(
120122
parsed_options=parsed_options,
121123
refresh_templates_force=force,
122124
)
125+
_register_extensions_for_agent(
126+
project_root,
127+
target,
128+
continuing=(
129+
"The integration switch succeeded, but installed extensions may "
130+
"need re-registration."
131+
),
132+
)
123133
console.print(f"\n[green]✓[/green] Default integration set to [bold]{target}[/bold].")
124134
raise typer.Exit(0)
125135

@@ -171,19 +181,11 @@ def integration_switch(
171181

172182
# Unregister extension commands for the old agent so they don't
173183
# remain as orphans in the old agent's directory.
174-
try:
175-
from ..extensions import ExtensionManager
176-
177-
ext_mgr = ExtensionManager(project_root)
178-
ext_mgr.unregister_agent_artifacts(installed_key)
179-
except Exception as ext_err:
180-
_print_cli_warning(
181-
"clean up extension artifacts for",
182-
"integration",
183-
installed_key,
184-
ext_err,
185-
continuing="Continuing with integration switch; old extension artifacts may need manual cleanup.",
186-
)
184+
_unregister_extensions_for_agent(
185+
project_root,
186+
installed_key,
187+
continuing="Continuing with integration switch; old extension artifacts may need manual cleanup.",
188+
)
187189

188190
# Clear metadata so a failed Phase 2 doesn't leave stale references
189191
installed_keys = [installed for installed in installed_keys if installed != installed_key]
@@ -270,22 +272,6 @@ def integration_switch(
270272
parsed_options=parsed_options,
271273
)
272274

273-
# Re-register extension commands for the new agent so that
274-
# previously-installed extensions are available in the new integration.
275-
try:
276-
from ..extensions import ExtensionManager
277-
278-
ext_mgr = ExtensionManager(project_root)
279-
ext_mgr.register_enabled_extensions_for_agent(target)
280-
except Exception as ext_err:
281-
_print_cli_warning(
282-
"register extension artifacts for",
283-
"integration",
284-
target,
285-
ext_err,
286-
continuing="The integration switch succeeded, but installed extensions may need re-registration.",
287-
)
288-
289275
except Exception as exc:
290276
# Attempt rollback of any files written by setup
291277
try:
@@ -333,6 +319,15 @@ def integration_switch(
333319
)
334320
raise typer.Exit(1)
335321

322+
# Re-register extension commands for the new agent so previously-installed
323+
# extensions are available in it. Done after the try/except (the switch has
324+
# committed) so this best-effort step can never trigger the rollback above.
325+
_register_extensions_for_agent(
326+
project_root,
327+
target,
328+
continuing="The integration switch succeeded, but installed extensions may need re-registration.",
329+
)
330+
336331
name = (target_integration.config or {}).get("name", target)
337332
console.print(f"\n[green]✓[/green] Switched to integration '{name}'")
338333

@@ -496,5 +491,17 @@ def integration_upgrade(
496491
if stale_removed:
497492
console.print(f" Removed {len(stale_removed)} stale file(s) from previous install")
498493

494+
# Re-register enabled extensions for the upgraded agent so its extension
495+
# commands are (re)created — including agents installed before this
496+
# back-fill existed. Mirrors switch for command registration; see #2886.
497+
# Done after the upgrade has fully settled (Phase 2 included) and outside
498+
# the try/except above so this best-effort step cannot affect upgrade
499+
# success.
500+
_register_extensions_for_agent(
501+
project_root,
502+
key,
503+
continuing="The integration was upgraded, but installed extensions may need re-registration.",
504+
)
505+
499506
name = (integration.config or {}).get("name", key)
500507
console.print(f"\n[green]✓[/green] Integration '{name}' upgraded successfully")

src/specify_cli/integrations/_query_commands.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
from ._commands import integration_app, integration_catalog_app
1818
from ._helpers import (
1919
_read_integration_json,
20+
_register_extensions_for_agent,
2021
_resolve_integration_options,
2122
_set_default_integration_or_exit,
2223
)
@@ -242,6 +243,11 @@ def integration_use(
242243
f"[cyan]specify integration use {key} --force[/cyan]."
243244
),
244245
)
246+
_register_extensions_for_agent(
247+
project_root,
248+
key,
249+
continuing="The integration was selected, but installed extensions may need re-registration.",
250+
)
245251
console.print(f"[green]✓[/green] Default integration set to [bold]{key}[/bold].")
246252

247253

0 commit comments

Comments
 (0)