fix(bob): reject layout migration when preset overrides are installed (review #3415)

mnriem · Copilot · mnriem · commit d2321b2afcd6 · 2026-07-17T16:33:39.000-05:00
A command↔skills layout change during `integration upgrade` cannot reconcile
preset artifacts: presets track their command/skill files in per-preset
`registered_commands`/`registered_skills` metadata, and there is no
agent-scoped preset re-registration anywhere in the CLI. Migrating would
delete a preset's old-layout files without recreating them in the new layout
and leave the preset registry claiming artifacts that no longer exist.

Detect the intended layout via `is_skills_mode` (so a plain same-layout
upgrade is unaffected) and, when it flips while preset overrides are
installed for the agent, reject the upgrade *before any mutation* with an
actionable error pointing at the remove → upgrade → reinstall workaround.
Extension artifacts are still reconciled for the safe (no-preset) case.

Adds a regression test and documents the migration caveat in the Bob
integration reference entry.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App &lt;223556219+Copilot@users.noreply.github.com&gt;
Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf
diff --git a/docs/reference/integrations.md b/docs/reference/integrations.md
@@ -22,7 +22,7 @@ The Specify CLI supports a wide range of AI coding agents. When you run `specify
 | [Goose](https://goose-docs.ai/)                                                      | `goose`          | Uses YAML recipe format in `.goose/recipes/`                                                                                              |
 | [Grok Build](https://docs.x.ai/build/overview)                                       | `grok`           | Skills-based integration; installs skills into `.grok/skills` and invokes them as `/speckit-<command>`                                    |
 | [Hermes](https://github.com/NousResearch/hermes-agent)                               | `hermes`         | Skills-based integration; installs skills globally into `~/.hermes/skills/`                                                                |
-| [IBM Bob](https://www.ibm.com/products/bob)                                          | `bob`            | Skills-based integration by default; installs skills as `speckit-<command>/SKILL.md` under `.bob/skills/` and invokes them as `/speckit-<command>`. Pass `--integration-options="--legacy-commands"` to scaffold the deprecated Bob 1.x layout (`.bob/commands/*.md`) instead; that flag will be removed in a future release. Existing legacy installs can migrate with `specify integration upgrade bob --integration-options="--skills"`, which converts them to the skills layout and removes the old command files. |
+| [IBM Bob](https://www.ibm.com/products/bob)                                          | `bob`            | Skills-based integration by default; installs skills as `speckit-<command>/SKILL.md` under `.bob/skills/` and invokes them as `/speckit-<command>`. Pass `--integration-options="--legacy-commands"` to scaffold the deprecated Bob 1.x layout (`.bob/commands/*.md`) instead; that flag will be removed in a future release. Existing legacy installs can migrate with `specify integration upgrade bob --integration-options="--skills"`, which converts them to the skills layout and removes the old command files. If preset overrides are installed, the migration is rejected with an actionable error (preset artifacts cannot yet be reconciled across a layout change) — remove the preset(s), migrate, then reinstall them. |
 | [Junie](https://junie.jetbrains.com/)                                                | `junie`          |                                                                                                                                           |
 | [Kilo Code](https://github.com/Kilo-Org/kilocode)                                    | `kilocode`       |                                                                                                                                           |
 | [Kimi Code](https://code.kimi.com/)                                                  | `kimi`           | Skills-based integration; installs into `.kimi-code/skills/`. `--migrate-legacy` moves old `.kimi/skills/` installs to the new paths |
diff --git a/src/specify_cli/integrations/_migrate_commands.py b/src/specify_cli/integrations/_migrate_commands.py
@@ -53,6 +53,42 @@ def _manifest_tracks_skill_layout(manifest) -> bool:
     return any(str(rel).endswith("/SKILL.md") for rel in manifest.files)
 
 
+def _installed_presets_affecting_agent(project_root, agent_key: str) -> list[str]:
+    """Return IDs of installed presets with artifacts registered for *agent_key*.
+
+    Presets register command overrides for every detected agent and mirror
+    skills for the active skills agent, tracking the result in each preset's
+    ``registered_commands`` / ``registered_skills`` metadata. There is no
+    agent-scoped preset re-registration mechanism, so a command↔skills *layout
+    change* cannot reconcile those artifacts (see ``integration_upgrade``).
+    Callers use this to detect the unsafe case and reject the migration rather
+    than silently orphaning preset files / leaving stale registry entries.
+
+    Best-effort: any error resolving preset state yields an empty list so a
+    broken/absent preset registry never blocks an otherwise-valid upgrade.
+    """
+    try:
+        from ..presets import PresetManager
+
+        presets = PresetManager(project_root).registry.list()
+    except Exception:
+        return []
+
+    affected: list[str] = []
+    for preset_id, meta in presets.items():
+        if not isinstance(meta, dict):
+            continue
+        registered_commands = meta.get("registered_commands", {})
+        has_commands = (
+            isinstance(registered_commands, dict)
+            and bool(registered_commands.get(agent_key))
+        )
+        has_skills = bool(meta.get("registered_skills"))
+        if has_commands or has_skills:
+            affected.append(preset_id)
+    return affected
+
+
 @integration_app.command("switch")
 def integration_switch(
     target: str = typer.Argument(help="Integration key to switch to"),
@@ -412,6 +448,42 @@ def integration_upgrade(
         integration, current, key, integration_options
     )
 
+    # Guard: reject a command↔skills layout change while preset overrides are
+    # installed for this agent (review #3415).  A dual-mode agent (e.g. Bob)
+    # can flip layout across an upgrade (``--skills`` / ``--legacy-commands``).
+    # Extension artifacts are reconciled after the flip (see below), but preset
+    # artifacts cannot be: there is no agent-scoped preset re-registration
+    # anywhere in the CLI, so migrating would delete a preset's old-layout
+    # files without recreating them in the new layout and leave the preset
+    # registry claiming artifacts that no longer exist.  Detect the intended
+    # layout (``is_skills_mode`` reflects the resolved flags/disk state, so a
+    # plain same-layout upgrade is unaffected) and bail out *before* any
+    # mutation with an actionable error so the project is never left in a
+    # half-migrated, inconsistent state.
+    if _manifest_tracks_skill_layout(old_manifest) != integration.is_skills_mode(
+        parsed_options, project_root
+    ):
+        affected_presets = _installed_presets_affecting_agent(project_root, key)
+        if affected_presets:
+            preset_list = ", ".join(sorted(affected_presets))
+            console.print(
+                f"[red]Error:[/red] Cannot change '{key}' command layout while "
+                f"preset override(s) are installed: [bold]{preset_list}[/bold]."
+            )
+            console.print(
+                "Preset artifacts cannot yet be reconciled across a command↔skills "
+                "layout change, so the migration would orphan their files and leave "
+                "the preset registry inconsistent."
+            )
+            console.print(
+                "Remove the preset(s), run the upgrade, then reinstall them:\n"
+                f"  [cyan]specify preset remove <id>[/cyan]\n"
+                f"  [cyan]specify integration upgrade {key} "
+                f"--integration-options \"...\"[/cyan]\n"
+                f"  [cyan]specify preset add <id>[/cyan]"
+            )
+            raise typer.Exit(1)
+
     # Ensure shared infrastructure is up to date; --force overwrites existing files.
     infra_integration = integration
     infra_key = key
@@ -544,11 +616,12 @@ def integration_upgrade(
     # layout change. There is no agent-scoped preset re-registration mechanism
     # anywhere in the CLI — ``use`` / ``switch`` / ``upgrade`` never reconcile
     # presets for any agent (presets are only (un)registered at preset
-    # install/remove time). Reconciling them here would require a new
-    # cross-cutting PresetManager subsystem affecting every dual-layout agent,
-    # which is out of scope for this Bob migration. A project that changes Bob's
-    # layout while a preset override is installed should re-run
-    # ``preset remove``/``preset install`` to refresh those artifacts.
+    # install/remove time). Rather than silently orphan them, the guard near
+    # the top of this function rejects a layout-changing upgrade while preset
+    # overrides are installed, so control only reaches here (with a changed
+    # layout) when no preset artifacts are at stake. Full preset reconciliation
+    # would require a new cross-cutting PresetManager subsystem affecting every
+    # dual-layout agent, which is out of scope for this Bob migration.
     if _manifest_tracks_skill_layout(old_manifest) != _manifest_tracks_skill_layout(
         new_manifest
     ):
diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py
@@ -2606,6 +2606,62 @@ def _git_registry():
         cmds_agents, skill_names = _git_registry()
         assert "bob" in cmds_agents and not skill_names
 
+    def test_upgrade_bob_layout_change_rejected_with_presets_installed(self, tmp_path):
+        """Regression (review #3415, 4726193915).
+
+        A command↔skills layout change cannot reconcile preset artifacts (no
+        agent-scoped preset re-registration exists). Rather than silently
+        orphaning preset files / leaving the registry inconsistent, a
+        layout-changing ``upgrade`` must reject the migration with an
+        actionable error *before any mutation* when preset overrides are
+        installed for the agent. A same-layout upgrade must still succeed.
+        """
+        project = _init_project(
+            tmp_path, "bob", integration_options="--legacy-commands"
+        )
+        commands = project / ".bob" / "commands"
+        skills = project / ".bob" / "skills"
+        assert sorted(commands.glob("speckit.*.md"))
+
+        # Simulate an installed preset that registered command overrides for bob.
+        presets_dir = project / ".specify" / "presets"
+        presets_dir.mkdir(parents=True, exist_ok=True)
+        (presets_dir / ".registry").write_text(
+            json.dumps({
+                "presets": {
+                    "my-preset": {
+                        "version": "1.0.0",
+                        "enabled": True,
+                        "registered_commands": {"bob": ["speckit.plan"]},
+                        "registered_skills": [],
+                    }
+                }
+            }),
+            encoding="utf-8",
+        )
+
+        # Layout-changing upgrade is rejected, and nothing is mutated.
+        result = _run_in_project(project, [
+            "integration", "upgrade", "bob",
+            "--integration-options", "--skills",
+            "--script", "sh", "--force",
+        ])
+        assert result.exit_code != 0, "layout change with presets must be rejected"
+        assert "preset" in result.output.lower()
+        assert "my-preset" in result.output
+        assert not skills.exists(), "no skills layout must be scaffolded on rejection"
+        assert sorted(commands.glob("speckit.*.md")), (
+            "legacy command files must be left untouched on rejection"
+        )
+
+        # A same-layout upgrade (no flag) must still succeed with presets present.
+        result = _run_in_project(project, [
+            "integration", "upgrade", "bob", "--script", "sh", "--force",
+        ])
+        assert result.exit_code == 0, (
+            f"same-layout upgrade must not be blocked by presets: {result.output}"
+        )
+
     def test_upgrade_preserves_existing_vscode_settings(self, tmp_path):
         """Regression: copilot upgrade must not stale-delete .vscode/settings.json.
 
