Skip to content

Commit 9cdc8d0

Browse files
mnriemCopilot
andcommitted
fix(bob): restrict layout reconciliation to the active integration (review #3415)
`integration_upgrade` supports upgrading a secondary (non-active) integration, but the layout-change extension reconciliation was unsafe there. `ExtensionManager.unregister_agent_artifacts()` treats the unscoped per-extension `registered_skills` list as belonging to the passed agent and, when that agent's skills directory is absent, falls back to scanning every agent's skills directory — so reconciling a secondary Bob layout flip could delete or untrack the *active* agent's extension skills. The subsequent re-registration cannot repair that because extension skill rendering is intentionally scoped to the active agent (#2948). Gate the unregister-before-register reconciliation on `installed_key == key` so it only runs for the active integration. Secondary agents only ever have extension command files (skills are active-agent-only), which the existing re-registration rewrites in place, so skipping the unregister orphans nothing new. Adds a regression test asserting a secondary Bob layout change leaves the active agent's extension skill intact on disk and in the registry. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf
1 parent d2321b2 commit 9cdc8d0

2 files changed

Lines changed: 74 additions & 2 deletions

File tree

src/specify_cli/integrations/_migrate_commands.py

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,19 @@ def integration_upgrade(
612612
# re-registration below recreates them in the new layout. ``upgrade``s that
613613
# don't change layout skip this to avoid needless remove/re-add churn.
614614
#
615+
# Only the *active* integration is reconciled this way (``installed_key ==
616+
# key``). ``ExtensionManager.unregister_agent_artifacts`` treats the
617+
# per-extension ``registered_skills`` list as belonging to the passed agent
618+
# and, when that agent's skills directory is absent, falls back to scanning
619+
# every agent's skills directory — so running it for a *secondary*
620+
# (non-active) agent could delete or untrack the *active* agent's extension
621+
# skills. The subsequent re-registration cannot repair that because
622+
# extension skill rendering is intentionally scoped to the active agent
623+
# (#2948). Extension skills only ever exist for the active agent, so
624+
# skipping the unregister for a secondary agent orphans nothing new: a
625+
# secondary agent only has extension *command* files, which the
626+
# re-registration below rewrites in place regardless of layout.
627+
#
615628
# Known limitation: preset command/skill artifacts are NOT reconciled on a
616629
# layout change. There is no agent-scoped preset re-registration mechanism
617630
# anywhere in the CLI — ``use`` / ``switch`` / ``upgrade`` never reconcile
@@ -622,8 +635,10 @@ def integration_upgrade(
622635
# layout) when no preset artifacts are at stake. Full preset reconciliation
623636
# would require a new cross-cutting PresetManager subsystem affecting every
624637
# dual-layout agent, which is out of scope for this Bob migration.
625-
if _manifest_tracks_skill_layout(old_manifest) != _manifest_tracks_skill_layout(
626-
new_manifest
638+
if (
639+
installed_key == key
640+
and _manifest_tracks_skill_layout(old_manifest)
641+
!= _manifest_tracks_skill_layout(new_manifest)
627642
):
628643
_unregister_extensions_for_agent(
629644
project_root,

tests/integrations/test_integration_subcommand.py

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2662,6 +2662,63 @@ def test_upgrade_bob_layout_change_rejected_with_presets_installed(self, tmp_pat
26622662
f"same-layout upgrade must not be blocked by presets: {result.output}"
26632663
)
26642664

2665+
def test_upgrade_secondary_bob_layout_change_preserves_active_agent_skills(
2666+
self, tmp_path
2667+
):
2668+
"""Regression (review #3415, 4726347306).
2669+
2670+
``integration upgrade`` supports upgrading a *secondary* (non-active)
2671+
integration. The layout-change extension reconciliation must NOT run
2672+
for a secondary agent: ``unregister_agent_artifacts`` treats the
2673+
unscoped per-extension ``registered_skills`` as belonging to the passed
2674+
agent and, if that agent's skills dir is absent, scans every agent's
2675+
skills dir — which could delete/untrack the *active* agent's extension
2676+
skills. The following re-registration cannot repair that because
2677+
extension skill rendering is active-agent-scoped (#2948).
2678+
"""
2679+
# Active agent: copilot in skills mode → git extension renders as skills.
2680+
project = _init_project(tmp_path, "copilot", integration_options="--skills")
2681+
result = _run_in_project(project, ["extension", "add", "git"])
2682+
assert result.exit_code == 0, f"extension add failed: {result.output}"
2683+
2684+
skill = project / ".github" / "skills" / "speckit-git-feature" / "SKILL.md"
2685+
assert skill.exists(), "precondition: active copilot has the git extension skill"
2686+
2687+
registry_path = project / ".specify" / "extensions" / ".registry"
2688+
2689+
def _git_skills():
2690+
data = json.loads(registry_path.read_text(encoding="utf-8"))
2691+
return data["extensions"]["git"].get("registered_skills", [])
2692+
2693+
assert _git_skills(), "precondition: git skills registered for active copilot"
2694+
2695+
# Add a secondary (non-active) Bob in the legacy commands layout.
2696+
result = _run_in_project(project, [
2697+
"integration", "install", "bob",
2698+
"--integration-options", "--legacy-commands",
2699+
"--script", "sh", "--force",
2700+
])
2701+
assert result.exit_code == 0, result.output
2702+
2703+
# Flip the *secondary* Bob's layout to skills. copilot stays active.
2704+
result = _run_in_project(project, [
2705+
"integration", "upgrade", "bob",
2706+
"--integration-options", "--skills",
2707+
"--script", "sh", "--force",
2708+
])
2709+
assert result.exit_code == 0, result.output
2710+
2711+
# The active agent's extension skill must be untouched on disk and in
2712+
# the registry — the secondary layout change must not reconcile it.
2713+
assert skill.exists(), (
2714+
"secondary Bob layout change must not delete the active agent's "
2715+
"extension skill"
2716+
)
2717+
assert _git_skills(), (
2718+
"secondary Bob layout change must not untrack the active agent's "
2719+
"extension skills in the registry"
2720+
)
2721+
26652722
def test_upgrade_preserves_existing_vscode_settings(self, tmp_path):
26662723
"""Regression: copilot upgrade must not stale-delete .vscode/settings.json.
26672724

0 commit comments

Comments
 (0)