Skip to content

Commit eaa8798

Browse files
marcelsafinCopilot
andcommitted
fix: filter uninstalled-extension commands in reconciliation; allow active-agent layout change with presets
Two follow-ups to the upstream-main merge: - Preset reconciliation (_reconcile_composed_commands) now skips extension-scoped commands (speckit.<ext>.<cmd>) whose extension is not installed, at the single chokepoint every install/remove/rescaffold pass funnels through. Registration already refused them, so reconciliation could materialize files no registry entry tracks. The duplicated per-call-site filters collapse into one _extension_installed_for_command helper. - The #3415 layout-change guard predates this PR's agent-scoped preset rescaffold: for the active integration, _register_presets_for_agent now re-registers enabled presets in the new layout and retires the old layout's stale files, so an active-agent command<->skills toggle proceeds and reconciles instead of being rejected. The guard still rejects non-active agents (no rescaffold runs for them) and still fails closed on an unreadable registry. _installed_presets_affecting_agent also understands the per-agent dict shape of registered_skills this PR writes, instead of raising 'malformed'. Regression tests: rescaffold with an uninstalled extension's command, CLI-level legacy<->skills toggle with an installed preset (both directions), secondary-agent rejection, and dict-shaped registered_skills in the guard helper. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent f918be5 commit eaa8798

4 files changed

Lines changed: 270 additions & 80 deletions

File tree

src/specify_cli/integrations/_migrate_commands.py

Lines changed: 45 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -119,12 +119,19 @@ def _installed_presets_affecting_agent(project_root, agent_key: str) -> list[str
119119
f"preset '{preset_id}' registered_commands is malformed"
120120
)
121121
registered_skills = meta.get("registered_skills", [])
122-
if not isinstance(registered_skills, (list, tuple)):
122+
if isinstance(registered_skills, dict):
123+
# Per-agent provenance ({agent: [skill names]}): only entries for
124+
# *this* agent make the preset affect it.
125+
has_skills = bool(registered_skills.get(agent_key))
126+
elif isinstance(registered_skills, (list, tuple)):
127+
# Legacy flat list: not agent-scoped, so any recorded skill may
128+
# belong to this agent — fail closed and count it as affecting.
129+
has_skills = bool(registered_skills)
130+
else:
123131
raise _PresetRegistryUnreadableError(
124132
f"preset '{preset_id}' registered_skills is malformed"
125133
)
126134
has_commands = bool(registered_commands.get(agent_key))
127-
has_skills = bool(registered_skills)
128135
if has_commands or has_skills:
129136
affected.append(preset_id)
130137
return affected
@@ -515,18 +522,23 @@ def integration_upgrade(
515522
integration, current, key, integration_options
516523
)
517524

518-
# Guard: reject a command↔skills layout change while preset overrides are
519-
# installed for this agent (review #3415). A dual-mode agent (e.g. Bob)
520-
# can flip layout across an upgrade (``--skills`` / ``--legacy-commands``).
521-
# Extension artifacts are reconciled after the flip (see below), but preset
522-
# artifacts cannot be: there is no agent-scoped preset re-registration
523-
# anywhere in the CLI, so migrating would delete a preset's old-layout
524-
# files without recreating them in the new layout and leave the preset
525-
# registry claiming artifacts that no longer exist. Detect the intended
526-
# layout (``is_skills_mode`` reflects the resolved flags/disk state, so a
527-
# plain same-layout upgrade is unaffected) and bail out *before* any
528-
# mutation with an actionable error so the project is never left in a
529-
# half-migrated, inconsistent state.
525+
# Guard: a command↔skills layout change is refused while preset overrides
526+
# are installed for a *non-active* agent, or while the preset registry is
527+
# unreadable (review #3415). A dual-mode agent (e.g. Bob) can flip layout
528+
# across an upgrade (``--skills`` / ``--legacy-commands``). Extension
529+
# artifacts are reconciled after the flip (see below), and for the
530+
# *active* integration preset artifacts are too: the post-upgrade
531+
# ``_register_presets_for_agent`` rescaffold re-registers every enabled
532+
# preset in the new layout and retires the old layout's stale files
533+
# (#2948), so an active-agent layout change proceeds. A non-active
534+
# integration gets no such rescaffold (preset registration is
535+
# active-only), so migrating it would delete a preset's old-layout files
536+
# without recreating them and leave the preset registry claiming
537+
# artifacts that no longer exist — bail out *before* any mutation with an
538+
# actionable error. An unreadable registry fails closed for either
539+
# agent: the rescaffold cannot reconcile unknown state. Detect the
540+
# intended layout (``is_skills_mode`` reflects the resolved flags/disk
541+
# state, so a plain same-layout upgrade is unaffected).
530542
if _manifest_tracks_skill_layout(old_manifest) != integration.is_skills_mode(
531543
parsed_options, project_root
532544
):
@@ -545,19 +557,21 @@ def integration_upgrade(
545557
"[cyan].specify/presets/.registry[/cyan] and retry."
546558
)
547559
raise typer.Exit(1)
548-
if affected_presets:
560+
if affected_presets and key != installed_key:
549561
preset_list = ", ".join(sorted(affected_presets))
550562
console.print(
551-
f"[red]Error:[/red] Cannot change '{key}' command layout while "
552-
f"preset override(s) are installed: [bold]{preset_list}[/bold]."
563+
f"[red]Error:[/red] Cannot change the non-active integration "
564+
f"'{key}' command layout while preset override(s) are installed "
565+
f"for it: [bold]{preset_list}[/bold]."
553566
)
554567
console.print(
555-
"Preset artifacts cannot yet be reconciled across a command↔skills "
556-
"layout change, so the migration would orphan their files and leave "
557-
"the preset registry inconsistent."
568+
"Preset artifacts are only reconciled for the active "
569+
"integration, so this migration would orphan the preset's "
570+
"old-layout files and leave the preset registry inconsistent."
558571
)
559572
console.print(
560-
"Remove the preset(s), run the upgrade, then reinstall them:\n"
573+
"Switch to it first (which rescaffolds presets), or remove the "
574+
"preset(s), run the upgrade, then reinstall them:\n"
561575
f" [cyan]specify preset remove <id>[/cyan]\n"
562576
f" [cyan]specify integration upgrade {key} "
563577
f"--integration-options \"...\"[/cyan]\n"
@@ -707,16 +721,16 @@ def integration_upgrade(
707721
# secondary agent only has extension *command* files, which the
708722
# re-registration below rewrites in place regardless of layout.
709723
#
710-
# Known limitation: preset command/skill artifacts are NOT reconciled on a
711-
# layout change. There is no agent-scoped preset re-registration mechanism
712-
# anywhere in the CLI — ``use`` / ``switch`` / ``upgrade`` never reconcile
713-
# presets for any agent (presets are only (un)registered at preset
714-
# install/remove time). Rather than silently orphan them, the guard near
715-
# the top of this function rejects a layout-changing upgrade while preset
716-
# overrides are installed, so control only reaches here (with a changed
717-
# layout) when no preset artifacts are at stake. Full preset reconciliation
718-
# would require a new cross-cutting PresetManager subsystem affecting every
719-
# dual-layout agent, which is out of scope for this Bob migration.
724+
# Preset artifacts follow the same pattern via the
725+
# ``_register_presets_for_agent`` rescaffold below: for the active
726+
# integration, ``register_enabled_presets_for_agent`` re-registers every
727+
# enabled preset in the new layout and retires the old layout's stale
728+
# command/skill files itself (its command↔skills toggle handling), so a
729+
# layout change needs no preset-specific unregister step here. Non-active
730+
# integrations are not rescaffolded (preset registration is active-only,
731+
# #2948); a layout-changing upgrade of a non-active agent with preset
732+
# artifacts is rejected by the guard near the top of this function
733+
# instead, so control never reaches here in that state.
720734
if key == installed_key:
721735
if _manifest_tracks_skill_layout(old_manifest) != _manifest_tracks_skill_layout(
722736
new_manifest

src/specify_cli/presets/__init__.py

Lines changed: 49 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -701,6 +701,25 @@ def check_compatibility(
701701

702702
return True
703703

704+
def _extension_installed_for_command(self, command_name: str) -> bool:
705+
"""Whether *command_name* may be materialized in this project.
706+
707+
Extension command overrides follow ``speckit.<ext-id>.<cmd-name>``;
708+
they must be skipped everywhere preset artifacts are written —
709+
registration *and* reconciliation — when the extension isn't
710+
installed, or reconciliation would materialize files that
711+
registration refused to track. Core commands (single-dot names,
712+
e.g. ``speckit.specify``) always pass.
713+
"""
714+
parts = command_name.split(".")
715+
if len(parts) >= 3 and parts[0] == "speckit":
716+
ext_id = parts[1]
717+
if not (
718+
self.project_root / ".specify" / "extensions" / ext_id
719+
).is_dir():
720+
return False
721+
return True
722+
704723
def _register_commands(
705724
self,
706725
manifest: PresetManifest,
@@ -730,17 +749,11 @@ def _register_commands(
730749
return {}
731750

732751
# Filter out extension command overrides if the extension isn't installed.
733-
# Command names follow the pattern: speckit.<ext-id>.<cmd-name>
734-
# Core commands (e.g. speckit.specify) have only one dot — always register.
735-
extensions_dir = self.project_root / ".specify" / "extensions"
736-
filtered = []
737-
for cmd in command_templates:
738-
parts = cmd["name"].split(".")
739-
if len(parts) >= 3 and parts[0] == "speckit":
740-
ext_id = parts[1]
741-
if not (extensions_dir / ext_id).is_dir():
742-
continue
743-
filtered.append(cmd)
752+
filtered = [
753+
cmd
754+
for cmd in command_templates
755+
if self._extension_installed_for_command(cmd["name"])
756+
]
744757

745758
if not filtered:
746759
return {}
@@ -1546,6 +1559,21 @@ def _reconcile_composed_commands(
15461559
if not command_names:
15471560
return set()
15481561

1562+
# Never materialize extension-scoped commands whose extension isn't
1563+
# installed. Registration (_register_commands / _register_skills)
1564+
# already refuses them, so a reconciliation pass writing them would
1565+
# create files no registry entry tracks. Filtering here — the single
1566+
# chokepoint every install/remove/rescaffold reconciliation funnels
1567+
# through — keeps all callers consistent without each one re-applying
1568+
# the filter when seeding names from manifest templates.
1569+
command_names = [
1570+
name
1571+
for name in command_names
1572+
if self._extension_installed_for_command(name)
1573+
]
1574+
if not command_names:
1575+
return set()
1576+
15491577
try:
15501578
from ..agents import CommandRegistrar
15511579
except ImportError:
@@ -2503,15 +2531,11 @@ def _register_skills(
25032531

25042532
# Filter out extension command overrides if the extension isn't installed,
25052533
# matching the same logic used by _register_commands().
2506-
extensions_dir = self.project_root / ".specify" / "extensions"
2507-
filtered = []
2508-
for cmd in command_templates:
2509-
parts = cmd["name"].split(".")
2510-
if len(parts) >= 3 and parts[0] == "speckit":
2511-
ext_id = parts[1]
2512-
if not (extensions_dir / ext_id).is_dir():
2513-
continue
2514-
filtered.append(cmd)
2534+
filtered = [
2535+
cmd
2536+
for cmd in command_templates
2537+
if self._extension_installed_for_command(cmd["name"])
2538+
]
25152539

25162540
if not filtered:
25172541
return {}
@@ -3369,20 +3393,11 @@ def install_from_directory(
33693393

33703394
# Reconcile all affected commands from the full priority stack so that
33713395
# install order doesn't determine the winning command file.
3372-
# Apply the same extension-installed filter as _register_commands to
3373-
# avoid reconciling extension commands when the extension isn't installed.
3374-
extensions_dir = self.project_root / ".specify" / "extensions"
3375-
cmd_names = []
3376-
for t in manifest.templates:
3377-
if t.get("type") != "command":
3378-
continue
3379-
name = t["name"]
3380-
parts = name.split(".")
3381-
if len(parts) >= 3 and parts[0] == "speckit":
3382-
ext_id = parts[1]
3383-
if not (extensions_dir / ext_id).is_dir():
3384-
continue
3385-
cmd_names.append(name)
3396+
cmd_names = [
3397+
t["name"]
3398+
for t in manifest.templates
3399+
if t.get("type") == "command"
3400+
]
33863401
if cmd_names:
33873402
try:
33883403
self._reconcile_composed_commands(cmd_names)

0 commit comments

Comments
 (0)