Skip to content

Commit f8f2f35

Browse files
committed
fix(integrations): defer extension registration until use
1 parent f8c4275 commit f8f2f35

4 files changed

Lines changed: 65 additions & 54 deletions

File tree

src/specify_cli/integrations/_helpers.py

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,7 @@ def _set_default_integration_or_exit(*args: Any, **kwargs: Any) -> None:
386386

387387

388388
# ---------------------------------------------------------------------------
389-
# Extension (un)registration helpers (shared by switch / install / upgrade)
389+
# Extension (un)registration helpers (shared by use / switch / upgrade)
390390
# ---------------------------------------------------------------------------
391391

392392
def _best_effort_extension_op(
@@ -423,21 +423,23 @@ def _register_extensions_for_agent(
423423
) -> None:
424424
"""Register all enabled extensions' commands/skills for ``agent_key``.
425425
426-
``switch`` has always re-registered enabled extensions for the agent it
427-
activates; ``install`` and ``upgrade`` call this so a newly added (or
428-
refreshed) agent reaches command-registration parity. See issue #2886.
426+
``use`` / ``switch`` re-register enabled extensions for the agent they
427+
activate; ``upgrade`` backfills them for the refreshed agent. Plain
428+
``install`` deliberately does not call this helper so adding a secondary
429+
integration has no extension side effects until it is selected or upgraded.
430+
See issue #2886.
429431
430432
Known limitation: extension *skill* rendering is scoped to the active
431433
agent (init-options track a single ``ai`` / ``ai_skills`` pair). A
432434
skills-mode agent registered while it is *not* the active agent (e.g.
433-
Copilot ``--skills`` installed as a secondary integration) therefore
435+
Copilot ``--skills`` registered while non-active) therefore
434436
receives command files rather than skills here — matching ``extension
435-
add``'s multi-agent behavior. ``switch`` avoids this only because it makes
436-
the target the active agent first. Per-agent skills parity is tracked in
437+
add``'s multi-agent behavior. ``use`` / ``switch`` avoid this because they
438+
make the target the active agent first. Per-agent skills parity is tracked in
437439
#2948.
438440
439441
Best-effort: never aborts the surrounding integration operation. Callers
440-
invoke it *after* the install/upgrade/switch transaction has committed so a
442+
invoke it *after* the use/upgrade/switch transaction has committed so a
441443
failure here cannot trigger a rollback.
442444
"""
443445
_best_effort_extension_op(

src/specify_cli/integrations/_install_commands.py

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
_get_speckit_version,
2727
_read_integration_json,
2828
_refresh_init_options_speckit_version,
29-
_register_extensions_for_agent,
3029
_remove_integration_json,
3130
_resolve_integration_options,
3231
_resolve_script_type,
@@ -189,18 +188,6 @@ def integration_install(
189188
)
190189
raise typer.Exit(1)
191190

192-
# Register enabled extensions for the newly installed agent so it gets the
193-
# same extension commands the existing agents already have for command
194-
# registration parity with switch; otherwise a second integration silently
195-
# lacks the first agent's extension commands. See #2886. Done after the
196-
# try/except (the install has committed) so this best-effort step can never
197-
# trigger the rollback above.
198-
_register_extensions_for_agent(
199-
project_root,
200-
integration.key,
201-
continuing="The integration was installed, but installed extensions may need re-registration.",
202-
)
203-
204191
name = (integration.config or {}).get("name", key)
205192
console.print(f"\n[green]✓[/green] Integration '{name}' installed successfully")
206193
if default_key:

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

tests/integrations/test_integration_subcommand.py

Lines changed: 49 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1240,15 +1240,13 @@ def test_install_bare_project_gets_shared_infra(self, tmp_path):
12401240
assert "/speckit-specify" in script_content
12411241
assert "/speckit.specify" not in script_content
12421242

1243-
def test_install_registers_extension_commands_for_new_agent(self, tmp_path):
1244-
"""Installing a second integration registers enabled extensions for it.
1245-
1246-
Regression for #2886: only ``switch`` used to register extension
1247-
commands for the newly active agent, so a second integration added via
1248-
``install`` was silently missing the extension commands the first agent
1249-
had. ``install`` must reach command-registration parity with
1250-
``switch``; active-agent-scoped extension skill rendering remains
1251-
tracked in #2948.
1243+
def test_install_defers_extension_commands_until_use(self, tmp_path):
1244+
"""Installing a second integration does not register enabled extensions.
1245+
1246+
Maintainer-requested behavior for #2886: extension command back-fill is
1247+
limited to ``integration use`` / ``switch`` / ``upgrade``. Plain
1248+
``install`` only adds the integration; selecting it with ``use`` then
1249+
registers the enabled extensions for that agent.
12521250
"""
12531251
project = _init_project(tmp_path, "claude")
12541252

@@ -1268,15 +1266,24 @@ def test_install_registers_extension_commands_for_new_agent(self, tmp_path):
12681266
])
12691267
assert result.exit_code == 0, result.output
12701268

1271-
# The new agent now carries the git extension's commands too, and the
1272-
# existing agent's registration is preserved.
1269+
# Install alone does not back-fill the git extension for the secondary
1270+
# agent.
12731271
registered = json.loads(registry_path.read_text(encoding="utf-8"))[
12741272
"extensions"
12751273
]["git"]["registered_commands"]
12761274
assert "claude" in registered, "existing agent registration preserved"
1277-
assert "codex" in registered, "new agent should receive extension commands (#2886)"
1275+
assert "codex" not in registered
1276+
assert not (
1277+
project / ".agents" / "skills" / "speckit-git-feature" / "SKILL.md"
1278+
).exists()
12781279

1279-
# codex renders command-derived artifacts as skills (.agents/skills).
1280+
result = _run_in_project(project, ["integration", "use", "codex"])
1281+
assert result.exit_code == 0, result.output
1282+
1283+
registered = json.loads(registry_path.read_text(encoding="utf-8"))[
1284+
"extensions"
1285+
]["git"]["registered_commands"]
1286+
assert "codex" in registered, "use should register extension commands (#2886)"
12801287
assert (
12811288
project / ".agents" / "skills" / "speckit-git-feature" / "SKILL.md"
12821289
).exists()
@@ -1306,18 +1313,12 @@ def test_install_does_not_register_disabled_extensions(self, tmp_path):
13061313
project / ".agents" / "skills" / "speckit-git-feature" / "SKILL.md"
13071314
).exists()
13081315

1309-
def test_install_skills_mode_secondary_agent_registers_commands(self, tmp_path):
1310-
"""A non-active skills-mode agent gets extension *commands*, not skills.
1311-
1312-
Pins a known, system-wide limitation (tracked in #2948): extension
1313-
skill rendering is scoped to the active agent — init-options track a
1314-
single ``ai`` / ``ai_skills`` pair — so a skills-mode agent installed
1315-
as a *non-active* secondary integration (Copilot ``--skills``) receives
1316-
``.github/agents/*.agent.md`` command files rather than
1317-
``.github/skills/.../SKILL.md``. This matches what ``extension add``
1318-
already does across multiple agents; ``install`` (the #2886 fix) stays
1319-
consistent with it. ``switch`` differs only because it makes the target
1320-
the active agent before registering.
1316+
def test_install_skills_mode_secondary_agent_defers_extension_artifacts(self, tmp_path):
1317+
"""A non-active skills-mode agent gets extension artifacts only on use.
1318+
1319+
Plain ``install`` has no extension side effects. Once the secondary
1320+
Copilot ``--skills`` integration is selected with ``use``, it becomes the
1321+
active agent and receives extension skills.
13211322
"""
13221323
project = _init_project(tmp_path, "claude")
13231324

@@ -1340,21 +1341,36 @@ def test_install_skills_mode_secondary_agent_registers_commands(self, tmp_path):
13401341
project / ".github" / "skills" / "speckit-specify" / "SKILL.md"
13411342
).exists(), "precondition: copilot installed in skills mode"
13421343

1344+
# The git extension is not registered for the non-active copilot agent
1345+
# during install.
13431346
git_meta = json.loads(
13441347
(project / ".specify" / "extensions" / ".registry").read_text(encoding="utf-8")
13451348
)["extensions"]["git"]
1346-
# The git extension is still registered as commands for the non-active
1347-
# copilot agent...
1348-
assert "copilot" in git_meta["registered_commands"]
1349-
assert (
1349+
assert "copilot" not in git_meta["registered_commands"]
1350+
assert not (
13501351
project / ".github" / "agents" / "speckit.git.feature.agent.md"
13511352
).exists()
1352-
# ...and does NOT follow copilot's core commands into skills, because
1353-
# skill rendering is scoped to the active agent (claude here); #2948.
13541353
assert not (
13551354
project / ".github" / "skills" / "speckit-git-feature" / "SKILL.md"
13561355
).exists()
13571356

1357+
result = _run_in_project(project, ["integration", "use", "copilot"])
1358+
assert result.exit_code == 0, result.output
1359+
1360+
git_meta = json.loads(
1361+
(project / ".specify" / "extensions" / ".registry").read_text(encoding="utf-8")
1362+
)["extensions"]["git"]
1363+
# `use` makes copilot active, so extension artifacts follow copilot's
1364+
# skills-mode layout.
1365+
assert "copilot" not in git_meta["registered_commands"]
1366+
assert "speckit-git-feature" in git_meta["registered_skills"]
1367+
assert not (
1368+
project / ".github" / "agents" / "speckit.git.feature.agent.md"
1369+
).exists()
1370+
assert (
1371+
project / ".github" / "skills" / "speckit-git-feature" / "SKILL.md"
1372+
).exists()
1373+
13581374

13591375
# ── uninstall ────────────────────────────────────────────────────────
13601376

@@ -2494,8 +2510,8 @@ def test_upgrade_non_active_agent_preserves_active_agent_skills(self, tmp_path):
24942510
"""Upgrading a non-active agent must not touch the active agent's skills.
24952511
24962512
Regression for the #2886 wiring: extension skill rendering is
2497-
active-agent-scoped, so routing install/upgrade of a *secondary* agent
2498-
through ``register_enabled_extensions_for_agent`` used to re-render the
2513+
active-agent-scoped, so routing upgrade of a *secondary* agent through
2514+
``register_enabled_extensions_for_agent`` used to re-render the
24992515
*active* skills-mode agent's extension skills as a side effect —
25002516
resurrecting skill files the user had deliberately deleted. The skills
25012517
pass is now gated on the target being the active agent. (Skills parity

0 commit comments

Comments
 (0)