From b01f1f6c978ddafd572839339437828c87d4782b Mon Sep 17 00:00:00 2001 From: Davide Barletta Date: Wed, 8 Jul 2026 20:26:03 +0200 Subject: [PATCH 01/27] feat: update Bob integration to skills-based layout for Bob 2.0 Bob 2.0 replaces the command-based workflow (.bob/commands/*.md) with a skills-based layout (.bob/skills/speckit-/SKILL.md), matching the pattern used by Claude Code, Codex, and other skills-first agents. - Switch BobIntegration from MarkdownIntegration to SkillsIntegration - Update folder/dir from .bob/commands to .bob/skills - Change extension from .md to /SKILL.md (skills layout) - Add --skills option (default: True) consistent with Codex pattern - Update tests to inherit from SkillsIntegrationTests (28 tests pass) - Bump catalog entry to version 2.0.0 with updated description Assisted-by: IBM Bob (model: claude-sonnet-4-5, autonomous) --- integrations/catalog.json | 4 +-- src/specify_cli/integrations/bob/__init__.py | 31 ++++++++++++++++---- tests/integrations/test_integration_bob.py | 8 ++--- 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/integrations/catalog.json b/integrations/catalog.json index 601ae0ad92..221ba15d9d 100644 --- a/integrations/catalog.json +++ b/integrations/catalog.json @@ -177,8 +177,8 @@ "bob": { "id": "bob", "name": "IBM Bob", - "version": "1.0.0", - "description": "IBM Bob IDE integration", + "version": "2.0.0", + "description": "IBM Bob 2.0 IDE integration", "author": "spec-kit-core", "repository": "https://github.com/github/spec-kit", "tags": ["ide", "ibm"] diff --git a/src/specify_cli/integrations/bob/__init__.py b/src/specify_cli/integrations/bob/__init__.py index b953151bd2..d4c75ef425 100644 --- a/src/specify_cli/integrations/bob/__init__.py +++ b/src/specify_cli/integrations/bob/__init__.py @@ -1,20 +1,39 @@ -"""IBM Bob integration.""" +"""IBM Bob integration. -from ..base import MarkdownIntegration +Bob 2.0 uses the ``.bob/skills/speckit-/SKILL.md`` layout. +The legacy ``.bob/commands/`` layout (Bob 1.x) is no longer supported. +""" +from __future__ import annotations + +from ..base import IntegrationOption, SkillsIntegration + + +class BobIntegration(SkillsIntegration): + """Integration for IBM Bob IDE.""" -class BobIntegration(MarkdownIntegration): key = "bob" config = { "name": "IBM Bob", "folder": ".bob/", - "commands_subdir": "commands", + "commands_subdir": "skills", "install_url": None, "requires_cli": False, } registrar_config = { - "dir": ".bob/commands", + "dir": ".bob/skills", "format": "markdown", "args": "$ARGUMENTS", - "extension": ".md", + "extension": "/SKILL.md", } + + @classmethod + def options(cls) -> list[IntegrationOption]: + return [ + IntegrationOption( + "--skills", + is_flag=True, + default=True, + help="Install as agent skills (default for Bob 2.0)", + ), + ] diff --git a/tests/integrations/test_integration_bob.py b/tests/integrations/test_integration_bob.py index 8e0e72f0bd..a5cf8f70d4 100644 --- a/tests/integrations/test_integration_bob.py +++ b/tests/integrations/test_integration_bob.py @@ -1,10 +1,10 @@ """Tests for BobIntegration.""" -from .test_integration_base_markdown import MarkdownIntegrationTests +from .test_integration_base_skills import SkillsIntegrationTests -class TestBobIntegration(MarkdownIntegrationTests): +class TestBobIntegration(SkillsIntegrationTests): KEY = "bob" FOLDER = ".bob/" - COMMANDS_SUBDIR = "commands" - REGISTRAR_DIR = ".bob/commands" + COMMANDS_SUBDIR = "skills" + REGISTRAR_DIR = ".bob/skills" From dea23f06be96d5edacab65f4bc7490cb445ad3e3 Mon Sep 17 00:00:00 2001 From: Davide Barletta Date: Mon, 13 Jul 2026 16:59:59 +0200 Subject: [PATCH 02/27] PR comments fix: keep old Bob 1 commands till next release --- src/specify_cli/integrations/bob/__init__.py | 129 ++++++++- tests/integrations/test_integration_bob.py | 273 ++++++++++++++++++- 2 files changed, 389 insertions(+), 13 deletions(-) diff --git a/src/specify_cli/integrations/bob/__init__.py b/src/specify_cli/integrations/bob/__init__.py index d4c75ef425..c7afe926e6 100644 --- a/src/specify_cli/integrations/bob/__init__.py +++ b/src/specify_cli/integrations/bob/__init__.py @@ -1,16 +1,43 @@ """IBM Bob integration. -Bob 2.0 uses the ``.bob/skills/speckit-/SKILL.md`` layout. -The legacy ``.bob/commands/`` layout (Bob 1.x) is no longer supported. +Bob 2.0 supports the ``.bob/skills/speckit-/SKILL.md`` layout. +The legacy ``.bob/commands/*.md`` layout (Bob 1.x) remains the default +for this release and will be deprecated in a future Spec Kit release. + +Deprecation cycle: + This release: Markdown layout is default; skills layout is opt-in via + ``--integration-options "--skills"``. + Next cycle: Skills layout becomes default; markdown remains opt-in. + Cycle after: Markdown layout removed. """ from __future__ import annotations -from ..base import IntegrationOption, SkillsIntegration +import warnings +from pathlib import Path +from typing import Any + +from ..base import IntegrationBase, IntegrationOption, SkillsIntegration +from ..manifest import IntegrationManifest + + +def _warn_legacy_markdown_default() -> None: + """Warn that Bob's default markdown scaffold is being phased out.""" + warnings.warn( + "Bob legacy markdown mode (.bob/commands/) is deprecated and will stop " + "being the default in a future Spec Kit release; pass " + '--integration-options "--skills" to opt in to Bob skills mode now.', + UserWarning, + stacklevel=3, + ) -class BobIntegration(SkillsIntegration): - """Integration for IBM Bob IDE.""" +class _BobSkillsHelper(SkillsIntegration): + """Internal helper used when Bob is scaffolded in skills mode. + + Not registered in the integration registry — only used as a delegate + by ``BobIntegration`` when ``--skills`` is passed. + """ key = "bob" config = { @@ -27,13 +54,101 @@ class BobIntegration(SkillsIntegration): "extension": "/SKILL.md", } + +class BobIntegration(IntegrationBase): + """Integration for IBM Bob IDE. + + Default mode: installs ``.bob/commands/speckit..md`` files + (Bob 1.x markdown layout — legacy, will be deprecated). + + Skills mode (``--skills``): installs + ``.bob/skills/speckit-/SKILL.md`` files (Bob 2.0 layout). + """ + + key = "bob" + config = { + "name": "IBM Bob", + "folder": ".bob/", + "commands_subdir": "commands", + "install_url": None, + "requires_cli": False, + } + registrar_config = { + "dir": ".bob/commands", + "format": "markdown", + "args": "$ARGUMENTS", + "extension": ".md", + } + + # Mutable flag set by setup() — indicates the active scaffolding mode. + _skills_mode: bool = False + + def effective_invoke_separator( + self, parsed_options: dict[str, Any] | None = None + ) -> str: + """Return ``"-"`` when skills mode is requested, ``"."`` otherwise.""" + if parsed_options and parsed_options.get("skills"): + return "-" + if self._skills_mode: + return "-" + return self.invoke_separator + @classmethod def options(cls) -> list[IntegrationOption]: return [ IntegrationOption( "--skills", is_flag=True, - default=True, - help="Install as agent skills (default for Bob 2.0)", + default=False, + help=( + "Scaffold commands as agent skills " + "(.bob/skills/speckit-/SKILL.md) instead of " + "the legacy .bob/commands/*.md layout" + ), ), ] + + def setup( + self, + project_root: Path, + manifest: IntegrationManifest, + parsed_options: dict[str, Any] | None = None, + **opts: Any, + ) -> list[Path]: + """Install Bob commands. + + When ``parsed_options["skills"]`` is truthy, delegates to skills + scaffolding (``.bob/skills/speckit-/SKILL.md``). + Otherwise uses the default ``.bob/commands/speckit..md`` layout + and emits a deprecation warning. + """ + parsed_options = parsed_options or {} + self._skills_mode = bool(parsed_options.get("skills")) + if self._skills_mode: + return self._setup_skills(project_root, manifest, parsed_options, **opts) + if "skills" not in parsed_options: + _warn_legacy_markdown_default() + return self._setup_default(project_root, manifest, parsed_options, **opts) + + def _setup_default( + self, + project_root: Path, + manifest: IntegrationManifest, + parsed_options: dict[str, Any] | None = None, + **opts: Any, + ) -> list[Path]: + """Default mode: ``.bob/commands/speckit..md`` layout.""" + from ..base import MarkdownIntegration + + return MarkdownIntegration.setup(self, project_root, manifest, parsed_options, **opts) + + def _setup_skills( + self, + project_root: Path, + manifest: IntegrationManifest, + parsed_options: dict[str, Any] | None = None, + **opts: Any, + ) -> list[Path]: + """Skills mode: delegate to ``_BobSkillsHelper``.""" + helper = _BobSkillsHelper() + return SkillsIntegration.setup(helper, project_root, manifest, parsed_options, **opts) diff --git a/tests/integrations/test_integration_bob.py b/tests/integrations/test_integration_bob.py index a5cf8f70d4..fc1a91c5eb 100644 --- a/tests/integrations/test_integration_bob.py +++ b/tests/integrations/test_integration_bob.py @@ -1,10 +1,271 @@ """Tests for BobIntegration.""" -from .test_integration_base_skills import SkillsIntegrationTests +import os +import warnings +import pytest -class TestBobIntegration(SkillsIntegrationTests): - KEY = "bob" - FOLDER = ".bob/" - COMMANDS_SUBDIR = "skills" - REGISTRAR_DIR = ".bob/skills" +from specify_cli.integrations import INTEGRATION_REGISTRY, get_integration +from specify_cli.integrations.base import IntegrationBase, SkillsIntegration +from specify_cli.integrations.manifest import IntegrationManifest + + +class TestBobIntegrationRegistration: + def test_registered(self): + assert "bob" in INTEGRATION_REGISTRY + assert get_integration("bob") is not None + + def test_is_integration_base(self): + assert isinstance(get_integration("bob"), IntegrationBase) + + def test_not_skills_integration_directly(self): + """BobIntegration is not itself a SkillsIntegration — it's dual-mode.""" + from specify_cli.integrations.bob import BobIntegration + assert not isinstance(BobIntegration(), SkillsIntegration) + + def test_key_and_config(self): + bob = get_integration("bob") + assert bob.key == "bob" + assert bob.config["folder"] == ".bob/" + # Default mode is commands (markdown) + assert bob.config["commands_subdir"] == "commands" + assert bob.registrar_config["dir"] == ".bob/commands" + assert bob.registrar_config["extension"] == ".md" + + +class TestBobOptionsFlag: + def test_options_include_skills_flag(self): + bob = get_integration("bob") + opts = bob.options() + skills_opts = [o for o in opts if o.name == "--skills"] + assert len(skills_opts) == 1 + opt = skills_opts[0] + assert opt.is_flag is True + # Skills must be OPT-IN (default=False) — not the default + assert opt.default is False + + +class TestBobDefaultMarkdownMode: + """Default mode: .bob/commands/speckit..md layout.""" + + def test_setup_creates_markdown_files(self, tmp_path): + from specify_cli.integrations.bob import BobIntegration + bob = BobIntegration() + m = IntegrationManifest("bob", tmp_path) + created = bob.setup(tmp_path, m) + assert len(created) > 0 + for f in created: + assert f.exists() + assert f.suffix == ".md" + assert f.name.startswith("speckit.") + assert f.parent == tmp_path / ".bob" / "commands" + + def test_setup_warns_legacy_markdown_is_deprecated(self, tmp_path): + from specify_cli.integrations.bob import BobIntegration + bob = BobIntegration() + m = IntegrationManifest("bob", tmp_path) + + with pytest.warns(UserWarning, match="Bob legacy markdown mode"): + bob.setup(tmp_path, m) + + def test_setup_explicit_skills_false_no_warn(self, tmp_path): + """Explicitly passing skills=False is a conscious choice — no warning. + + The warning fires only when ``skills`` is absent from parsed_options + (i.e. the user gave no opinion), matching the Copilot pattern. + """ + from specify_cli.integrations.bob import BobIntegration + bob = BobIntegration() + m = IntegrationManifest("bob", tmp_path) + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + bob.setup(tmp_path, m, parsed_options={"skills": False}) + + assert not any( + "Bob legacy markdown mode" in str(item.message) + for item in caught + ) + + def test_setup_no_skills_dirs_created(self, tmp_path): + from specify_cli.integrations.bob import BobIntegration + bob = BobIntegration() + m = IntegrationManifest("bob", tmp_path) + bob.setup(tmp_path, m) + skills_dir = tmp_path / ".bob" / "skills" + assert not skills_dir.exists() + + def test_templates_are_processed(self, tmp_path): + from specify_cli.integrations.bob import BobIntegration + bob = BobIntegration() + m = IntegrationManifest("bob", tmp_path) + bob.setup(tmp_path, m) + commands_dir = tmp_path / ".bob" / "commands" + for md_file in commands_dir.glob("speckit.*.md"): + content = md_file.read_text(encoding="utf-8") + assert "{SCRIPT}" not in content, f"{md_file.name} has unprocessed {{SCRIPT}}" + assert "__AGENT__" not in content, f"{md_file.name} has unprocessed __AGENT__" + assert "{ARGS}" not in content, f"{md_file.name} has unprocessed {{ARGS}}" + assert "__SPECKIT_COMMAND_" not in content, f"{md_file.name} has unprocessed __SPECKIT_COMMAND_*__" + + def test_all_files_tracked_in_manifest(self, tmp_path): + from specify_cli.integrations.bob import BobIntegration + bob = BobIntegration() + m = IntegrationManifest("bob", tmp_path) + created = bob.setup(tmp_path, m) + for f in created: + rel = f.resolve().relative_to(tmp_path.resolve()).as_posix() + assert rel in m.files, f"{rel} not tracked in manifest" + + def test_install_uninstall_roundtrip(self, tmp_path): + from specify_cli.integrations.bob import BobIntegration + bob = BobIntegration() + m = IntegrationManifest("bob", tmp_path) + created = bob.install(tmp_path, m) + assert len(created) > 0 + m.save() + for f in created: + assert f.exists() + removed, skipped = bob.uninstall(tmp_path, m) + assert len(removed) == len(created) + assert skipped == [] + + +class TestBobSkillsMode: + """Skills mode: .bob/skills/speckit-/SKILL.md layout.""" + + def test_setup_skills_creates_skill_files(self, tmp_path): + from specify_cli.integrations.bob import BobIntegration + bob = BobIntegration() + m = IntegrationManifest("bob", tmp_path) + created = bob.setup(tmp_path, m, parsed_options={"skills": True}) + assert len(created) > 0 + for f in created: + assert f.exists() + assert f.name == "SKILL.md" + assert f.parent.name.startswith("speckit-") + + def test_setup_skills_does_not_warn_about_legacy(self, tmp_path): + from specify_cli.integrations.bob import BobIntegration + bob = BobIntegration() + m = IntegrationManifest("bob", tmp_path) + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + bob.setup(tmp_path, m, parsed_options={"skills": True}) + + assert not any( + "Bob legacy markdown mode" in str(item.message) + for item in caught + ) + + def test_setup_skills_creates_correct_directory(self, tmp_path): + from specify_cli.integrations.bob import BobIntegration + bob = BobIntegration() + m = IntegrationManifest("bob", tmp_path) + bob.setup(tmp_path, m, parsed_options={"skills": True}) + skills_dir = tmp_path / ".bob" / "skills" + assert skills_dir.is_dir() + + def test_setup_skills_no_commands_dir(self, tmp_path): + from specify_cli.integrations.bob import BobIntegration + bob = BobIntegration() + m = IntegrationManifest("bob", tmp_path) + bob.setup(tmp_path, m, parsed_options={"skills": True}) + commands_dir = tmp_path / ".bob" / "commands" + assert not commands_dir.exists() + + def test_setup_skills_has_expected_commands(self, tmp_path): + from specify_cli.integrations.bob import BobIntegration + bob = BobIntegration() + m = IntegrationManifest("bob", tmp_path) + created = bob.setup(tmp_path, m, parsed_options={"skills": True}) + + expected_commands = { + "analyze", "clarify", "constitution", "converge", "implement", + "plan", "checklist", "specify", "tasks", "taskstoissues", + } + actual_commands = {f.parent.name.removeprefix("speckit-") for f in created} + assert actual_commands == expected_commands + + def test_setup_skills_all_files_tracked(self, tmp_path): + from specify_cli.integrations.bob import BobIntegration + bob = BobIntegration() + m = IntegrationManifest("bob", tmp_path) + created = bob.setup(tmp_path, m, parsed_options={"skills": True}) + for f in created: + rel = f.resolve().relative_to(tmp_path.resolve()).as_posix() + assert rel in m.files, f"{rel} not tracked in manifest" + + def test_skills_uninstall_roundtrip(self, tmp_path): + from specify_cli.integrations.bob import BobIntegration + bob = BobIntegration() + m = IntegrationManifest("bob", tmp_path) + created = bob.install(tmp_path, m, parsed_options={"skills": True}) + assert len(created) > 0 + m.save() + removed, skipped = bob.uninstall(tmp_path, m) + assert len(removed) == len(created) + assert skipped == [] + + +class TestBobInitFlowDefault: + """CLI init integration tests for default (markdown) mode.""" + + def test_init_default_creates_commands(self, tmp_path): + from typer.testing import CliRunner + from specify_cli import app + + target = tmp_path / "test-proj" + result = CliRunner().invoke(app, [ + "init", str(target), "--integration", "bob", + "--ignore-agent-tools", "--script", "sh", + ]) + assert result.exit_code == 0, f"init --integration bob failed: {result.output}" + assert (target / ".bob" / "commands" / "speckit.plan.md").exists() + assert not (target / ".bob" / "skills").exists() + + def test_init_default_complete_file_inventory_sh(self, tmp_path): + """Default init creates .bob/commands/*.md files.""" + from typer.testing import CliRunner + from specify_cli import app + + project = tmp_path / "inventory-sh-bob" + project.mkdir() + old_cwd = os.getcwd() + try: + os.chdir(project) + result = CliRunner().invoke(app, [ + "init", "--here", "--integration", "bob", "--script", "sh", + "--ignore-agent-tools", + ], catch_exceptions=False) + finally: + os.chdir(old_cwd) + assert result.exit_code == 0, f"init failed: {result.output}" + + commands = [ + "analyze", "clarify", "constitution", "converge", "implement", + "plan", "checklist", "specify", "tasks", "taskstoissues", + ] + for cmd in commands: + assert (project / ".bob" / "commands" / f"speckit.{cmd}.md").exists(), ( + f"Missing .bob/commands/speckit.{cmd}.md" + ) + + +class TestBobInitFlowSkills: + """CLI init integration tests for skills mode.""" + + def test_init_skills_creates_skill_files(self, tmp_path): + from typer.testing import CliRunner + from specify_cli import app + + target = tmp_path / "test-proj" + result = CliRunner().invoke(app, [ + "init", str(target), "--integration", "bob", + "--integration-options", "--skills", + "--ignore-agent-tools", "--script", "sh", + ]) + assert result.exit_code == 0, f"init --integration bob --skills failed: {result.output}" + assert (target / ".bob" / "skills" / "speckit-plan" / "SKILL.md").exists() + assert not (target / ".bob" / "commands").exists() From 1c3d1cfdcbfab6a01805f3871ded4983abb67c77 Mon Sep 17 00:00:00 2001 From: Davide Barletta <98974352+davidebibm@users.noreply.github.com> Date: Tue, 14 Jul 2026 08:40:52 +0200 Subject: [PATCH 03/27] Copilot suggested change Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/specify_cli/integrations/bob/__init__.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/specify_cli/integrations/bob/__init__.py b/src/specify_cli/integrations/bob/__init__.py index c7afe926e6..b2650ce8e5 100644 --- a/src/specify_cli/integrations/bob/__init__.py +++ b/src/specify_cli/integrations/bob/__init__.py @@ -152,3 +152,7 @@ def _setup_skills( """Skills mode: delegate to ``_BobSkillsHelper``.""" helper = _BobSkillsHelper() return SkillsIntegration.setup(helper, project_root, manifest, parsed_options, **opts) + + def post_process_skill_content(self, content: str) -> str: + """Apply shared skills post-processing to externally generated skills.""" + return _BobSkillsHelper().post_process_skill_content(content) From 674a8db4249314a673360960aef6c64cb4dbfa14 Mon Sep 17 00:00:00 2001 From: Davide Barletta Date: Tue, 14 Jul 2026 08:48:13 +0200 Subject: [PATCH 04/27] feat(bob): address copilot comments, make skills layout default, demote legacy commands to opt-in --- src/specify_cli/integrations/bob/__init__.py | 121 ++++----- tests/integrations/test_integration_bob.py | 250 ++++++++++--------- 2 files changed, 180 insertions(+), 191 deletions(-) diff --git a/src/specify_cli/integrations/bob/__init__.py b/src/specify_cli/integrations/bob/__init__.py index b2650ce8e5..e47f3b4a28 100644 --- a/src/specify_cli/integrations/bob/__init__.py +++ b/src/specify_cli/integrations/bob/__init__.py @@ -1,14 +1,14 @@ """IBM Bob integration. -Bob 2.0 supports the ``.bob/skills/speckit-/SKILL.md`` layout. -The legacy ``.bob/commands/*.md`` layout (Bob 1.x) remains the default -for this release and will be deprecated in a future Spec Kit release. +Bob 2.0 uses the ``.bob/skills/speckit-/SKILL.md`` layout. +The legacy ``.bob/commands/*.md`` layout (Bob 1.x) is available as an +opt-in for projects that have not yet migrated, via +``--integration-options "--legacy-commands"``. Deprecation cycle: - This release: Markdown layout is default; skills layout is opt-in via - ``--integration-options "--skills"``. - Next cycle: Skills layout becomes default; markdown remains opt-in. - Cycle after: Markdown layout removed. + This release: Skills layout is the default; legacy ``.bob/commands/`` + is opt-in via ``--legacy-commands``. + Next cycle: ``--legacy-commands`` flag removed. """ from __future__ import annotations @@ -17,93 +17,85 @@ from pathlib import Path from typing import Any -from ..base import IntegrationBase, IntegrationOption, SkillsIntegration +from ..base import IntegrationOption, MarkdownIntegration, SkillsIntegration from ..manifest import IntegrationManifest -def _warn_legacy_markdown_default() -> None: - """Warn that Bob's default markdown scaffold is being phased out.""" +def _warn_legacy_commands_deprecated() -> None: + """Warn that Bob's legacy markdown layout is being phased out.""" warnings.warn( - "Bob legacy markdown mode (.bob/commands/) is deprecated and will stop " - "being the default in a future Spec Kit release; pass " - '--integration-options "--skills" to opt in to Bob skills mode now.', + "Bob legacy commands mode (.bob/commands/) is deprecated and will be " + "removed in a future Spec Kit release. Omit --legacy-commands to use " + "the default skills layout (.bob/skills/).", UserWarning, stacklevel=3, ) -class _BobSkillsHelper(SkillsIntegration): - """Internal helper used when Bob is scaffolded in skills mode. +class _BobMarkdownHelper(MarkdownIntegration): + """Internal helper used when Bob is scaffolded in legacy commands mode. Not registered in the integration registry — only used as a delegate - by ``BobIntegration`` when ``--skills`` is passed. + by ``BobIntegration`` when ``--legacy-commands`` is passed. """ key = "bob" config = { "name": "IBM Bob", "folder": ".bob/", - "commands_subdir": "skills", + "commands_subdir": "commands", "install_url": None, "requires_cli": False, } registrar_config = { - "dir": ".bob/skills", + "dir": ".bob/commands", "format": "markdown", "args": "$ARGUMENTS", - "extension": "/SKILL.md", + "extension": ".md", } -class BobIntegration(IntegrationBase): +class BobIntegration(SkillsIntegration): """Integration for IBM Bob IDE. - Default mode: installs ``.bob/commands/speckit..md`` files - (Bob 1.x markdown layout — legacy, will be deprecated). + Default mode: installs ``.bob/skills/speckit-/SKILL.md`` files + (Bob 2.0 skills layout). - Skills mode (``--skills``): installs - ``.bob/skills/speckit-/SKILL.md`` files (Bob 2.0 layout). + Legacy mode (``--legacy-commands``): installs + ``.bob/commands/speckit..md`` files (Bob 1.x layout — deprecated). + + Inheriting ``SkillsIntegration`` ensures ``invoke_separator = "-"`` is + set at the class level so ``CommandRegistrar.AGENT_CONFIGS`` (which reads + the class attribute directly) generates correct hyphenated + ``/speckit-`` references for skills. """ key = "bob" config = { "name": "IBM Bob", "folder": ".bob/", - "commands_subdir": "commands", + "commands_subdir": "skills", "install_url": None, "requires_cli": False, } registrar_config = { - "dir": ".bob/commands", + "dir": ".bob/skills", "format": "markdown", "args": "$ARGUMENTS", - "extension": ".md", + "extension": "/SKILL.md", } - # Mutable flag set by setup() — indicates the active scaffolding mode. - _skills_mode: bool = False - - def effective_invoke_separator( - self, parsed_options: dict[str, Any] | None = None - ) -> str: - """Return ``"-"`` when skills mode is requested, ``"."`` otherwise.""" - if parsed_options and parsed_options.get("skills"): - return "-" - if self._skills_mode: - return "-" - return self.invoke_separator - @classmethod def options(cls) -> list[IntegrationOption]: return [ IntegrationOption( - "--skills", + "--legacy-commands", is_flag=True, default=False, help=( - "Scaffold commands as agent skills " - "(.bob/skills/speckit-/SKILL.md) instead of " - "the legacy .bob/commands/*.md layout" + "Scaffold commands as legacy .bob/commands/*.md files " + "(Bob 1.x layout, deprecated) instead of the default " + "skills layout" ), ), ] @@ -117,42 +109,23 @@ def setup( ) -> list[Path]: """Install Bob commands. - When ``parsed_options["skills"]`` is truthy, delegates to skills - scaffolding (``.bob/skills/speckit-/SKILL.md``). - Otherwise uses the default ``.bob/commands/speckit..md`` layout - and emits a deprecation warning. + Default: skills layout (``.bob/skills/speckit-/SKILL.md``). + When ``parsed_options["legacy_commands"]`` is truthy, falls back to + the deprecated ``.bob/commands/speckit..md`` layout. """ parsed_options = parsed_options or {} - self._skills_mode = bool(parsed_options.get("skills")) - if self._skills_mode: - return self._setup_skills(project_root, manifest, parsed_options, **opts) - if "skills" not in parsed_options: - _warn_legacy_markdown_default() - return self._setup_default(project_root, manifest, parsed_options, **opts) - - def _setup_default( - self, - project_root: Path, - manifest: IntegrationManifest, - parsed_options: dict[str, Any] | None = None, - **opts: Any, - ) -> list[Path]: - """Default mode: ``.bob/commands/speckit..md`` layout.""" - from ..base import MarkdownIntegration + if parsed_options.get("legacy_commands"): + _warn_legacy_commands_deprecated() + return self._setup_legacy(project_root, manifest, parsed_options, **opts) + return SkillsIntegration.setup(self, project_root, manifest, parsed_options, **opts) - return MarkdownIntegration.setup(self, project_root, manifest, parsed_options, **opts) - - def _setup_skills( + def _setup_legacy( self, project_root: Path, manifest: IntegrationManifest, parsed_options: dict[str, Any] | None = None, **opts: Any, ) -> list[Path]: - """Skills mode: delegate to ``_BobSkillsHelper``.""" - helper = _BobSkillsHelper() - return SkillsIntegration.setup(helper, project_root, manifest, parsed_options, **opts) - - def post_process_skill_content(self, content: str) -> str: - """Apply shared skills post-processing to externally generated skills.""" - return _BobSkillsHelper().post_process_skill_content(content) + """Legacy mode: ``.bob/commands/speckit..md`` layout.""" + helper = _BobMarkdownHelper() + return MarkdownIntegration.setup(helper, project_root, manifest, parsed_options, **opts) diff --git a/tests/integrations/test_integration_bob.py b/tests/integrations/test_integration_bob.py index fc1a91c5eb..6d362ca772 100644 --- a/tests/integrations/test_integration_bob.py +++ b/tests/integrations/test_integration_bob.py @@ -4,9 +4,10 @@ import warnings import pytest +import yaml from specify_cli.integrations import INTEGRATION_REGISTRY, get_integration -from specify_cli.integrations.base import IntegrationBase, SkillsIntegration +from specify_cli.integrations.base import SkillsIntegration from specify_cli.integrations.manifest import IntegrationManifest @@ -15,102 +16,134 @@ def test_registered(self): assert "bob" in INTEGRATION_REGISTRY assert get_integration("bob") is not None - def test_is_integration_base(self): - assert isinstance(get_integration("bob"), IntegrationBase) - - def test_not_skills_integration_directly(self): - """BobIntegration is not itself a SkillsIntegration — it's dual-mode.""" - from specify_cli.integrations.bob import BobIntegration - assert not isinstance(BobIntegration(), SkillsIntegration) + def test_is_skills_integration(self): + """BobIntegration is a SkillsIntegration — skills are the default.""" + assert isinstance(get_integration("bob"), SkillsIntegration) def test_key_and_config(self): bob = get_integration("bob") assert bob.key == "bob" assert bob.config["folder"] == ".bob/" - # Default mode is commands (markdown) - assert bob.config["commands_subdir"] == "commands" - assert bob.registrar_config["dir"] == ".bob/commands" - assert bob.registrar_config["extension"] == ".md" + assert bob.config["commands_subdir"] == "skills" + assert bob.registrar_config["dir"] == ".bob/skills" + assert bob.registrar_config["extension"] == "/SKILL.md" + + def test_invoke_separator_is_hyphen(self): + """Class-level invoke_separator must be '-' so CommandRegistrar.AGENT_CONFIGS + generates correct /speckit- refs without calling effective_invoke_separator.""" + bob = get_integration("bob") + assert bob.invoke_separator == "-" class TestBobOptionsFlag: - def test_options_include_skills_flag(self): + def test_options_include_legacy_commands_flag(self): bob = get_integration("bob") opts = bob.options() - skills_opts = [o for o in opts if o.name == "--skills"] - assert len(skills_opts) == 1 - opt = skills_opts[0] + legacy_opts = [o for o in opts if o.name == "--legacy-commands"] + assert len(legacy_opts) == 1 + opt = legacy_opts[0] assert opt.is_flag is True - # Skills must be OPT-IN (default=False) — not the default + # Legacy must be OPT-IN (default=False) — skills are the default assert opt.default is False + def test_no_skills_flag(self): + """The old --skills flag must be gone; it has been replaced by the default.""" + bob = get_integration("bob") + opts = bob.options() + skills_opts = [o for o in opts if o.name == "--skills"] + assert len(skills_opts) == 0 + -class TestBobDefaultMarkdownMode: - """Default mode: .bob/commands/speckit..md layout.""" +class TestBobDefaultSkillsMode: + """Default mode: .bob/skills/speckit-/SKILL.md layout.""" - def test_setup_creates_markdown_files(self, tmp_path): - from specify_cli.integrations.bob import BobIntegration - bob = BobIntegration() + def test_setup_creates_skill_files(self, tmp_path): + bob = get_integration("bob") m = IntegrationManifest("bob", tmp_path) created = bob.setup(tmp_path, m) assert len(created) > 0 for f in created: assert f.exists() - assert f.suffix == ".md" - assert f.name.startswith("speckit.") - assert f.parent == tmp_path / ".bob" / "commands" + assert f.name == "SKILL.md" + assert f.parent.name.startswith("speckit-") - def test_setup_warns_legacy_markdown_is_deprecated(self, tmp_path): - from specify_cli.integrations.bob import BobIntegration - bob = BobIntegration() + def test_setup_writes_to_correct_directory(self, tmp_path): + bob = get_integration("bob") m = IntegrationManifest("bob", tmp_path) + bob.setup(tmp_path, m) + skills_dir = tmp_path / ".bob" / "skills" + assert skills_dir.is_dir() - with pytest.warns(UserWarning, match="Bob legacy markdown mode"): - bob.setup(tmp_path, m) - - def test_setup_explicit_skills_false_no_warn(self, tmp_path): - """Explicitly passing skills=False is a conscious choice — no warning. - - The warning fires only when ``skills`` is absent from parsed_options - (i.e. the user gave no opinion), matching the Copilot pattern. - """ - from specify_cli.integrations.bob import BobIntegration - bob = BobIntegration() + def test_setup_does_not_warn(self, tmp_path): + bob = get_integration("bob") m = IntegrationManifest("bob", tmp_path) - with warnings.catch_warnings(record=True) as caught: warnings.simplefilter("always") - bob.setup(tmp_path, m, parsed_options={"skills": False}) - + bob.setup(tmp_path, m) assert not any( - "Bob legacy markdown mode" in str(item.message) - for item in caught + "legacy" in str(item.message).lower() for item in caught ) - def test_setup_no_skills_dirs_created(self, tmp_path): - from specify_cli.integrations.bob import BobIntegration - bob = BobIntegration() + def test_setup_no_commands_dir(self, tmp_path): + bob = get_integration("bob") m = IntegrationManifest("bob", tmp_path) bob.setup(tmp_path, m) - skills_dir = tmp_path / ".bob" / "skills" - assert not skills_dir.exists() + assert not (tmp_path / ".bob" / "commands").exists() + + def test_skill_directory_structure(self, tmp_path): + """Each command produces speckit-/SKILL.md.""" + bob = get_integration("bob") + m = IntegrationManifest("bob", tmp_path) + created = bob.setup(tmp_path, m) + + expected_commands = { + "analyze", "clarify", "constitution", "converge", "implement", + "plan", "checklist", "specify", "tasks", "taskstoissues", + } + actual_commands = {f.parent.name.removeprefix("speckit-") for f in created} + assert actual_commands == expected_commands + + def test_skill_frontmatter_structure(self, tmp_path): + """SKILL.md must have name, description, compatibility, metadata.""" + bob = get_integration("bob") + m = IntegrationManifest("bob", tmp_path) + created = bob.setup(tmp_path, m) + for f in created: + content = f.read_text(encoding="utf-8") + assert content.startswith("---\n"), f"{f} missing frontmatter" + parts = content.split("---", 2) + fm = yaml.safe_load(parts[1]) + assert "name" in fm + assert "description" in fm + assert "compatibility" in fm + assert "metadata" in fm + assert fm["metadata"]["author"] == "github-spec-kit" def test_templates_are_processed(self, tmp_path): - from specify_cli.integrations.bob import BobIntegration - bob = BobIntegration() + bob = get_integration("bob") m = IntegrationManifest("bob", tmp_path) - bob.setup(tmp_path, m) - commands_dir = tmp_path / ".bob" / "commands" - for md_file in commands_dir.glob("speckit.*.md"): - content = md_file.read_text(encoding="utf-8") - assert "{SCRIPT}" not in content, f"{md_file.name} has unprocessed {{SCRIPT}}" - assert "__AGENT__" not in content, f"{md_file.name} has unprocessed __AGENT__" - assert "{ARGS}" not in content, f"{md_file.name} has unprocessed {{ARGS}}" - assert "__SPECKIT_COMMAND_" not in content, f"{md_file.name} has unprocessed __SPECKIT_COMMAND_*__" + created = bob.setup(tmp_path, m) + for f in created: + content = f.read_text(encoding="utf-8") + assert "{SCRIPT}" not in content, f"{f.name} has unprocessed {{SCRIPT}}" + assert "__AGENT__" not in content, f"{f.name} has unprocessed __AGENT__" + assert "{ARGS}" not in content, f"{f.name} has unprocessed {{ARGS}}" + assert "__SPECKIT_COMMAND_" not in content, f"{f.name} has unprocessed __SPECKIT_COMMAND_*__" + + def test_command_refs_use_hyphen_separator(self, tmp_path): + """Default skills layout must use /speckit-, not /speckit..""" + bob = get_integration("bob") + m = IntegrationManifest("bob", tmp_path) + created = bob.setup(tmp_path, m) + for f in created: + content = f.read_text(encoding="utf-8") + assert "/speckit." not in content, ( + f"{f.name} contains dot-notation /speckit. reference; " + "skills must use /speckit-" + ) def test_all_files_tracked_in_manifest(self, tmp_path): - from specify_cli.integrations.bob import BobIntegration - bob = BobIntegration() + bob = get_integration("bob") m = IntegrationManifest("bob", tmp_path) created = bob.setup(tmp_path, m) for f in created: @@ -118,8 +151,7 @@ def test_all_files_tracked_in_manifest(self, tmp_path): assert rel in m.files, f"{rel} not tracked in manifest" def test_install_uninstall_roundtrip(self, tmp_path): - from specify_cli.integrations.bob import BobIntegration - bob = BobIntegration() + bob = get_integration("bob") m = IntegrationManifest("bob", tmp_path) created = bob.install(tmp_path, m) assert len(created) > 0 @@ -131,77 +163,62 @@ def test_install_uninstall_roundtrip(self, tmp_path): assert skipped == [] -class TestBobSkillsMode: - """Skills mode: .bob/skills/speckit-/SKILL.md layout.""" +class TestBobLegacyCommandsMode: + """Legacy opt-in mode: .bob/commands/speckit..md layout.""" - def test_setup_skills_creates_skill_files(self, tmp_path): + def test_setup_legacy_creates_markdown_files(self, tmp_path): from specify_cli.integrations.bob import BobIntegration bob = BobIntegration() m = IntegrationManifest("bob", tmp_path) - created = bob.setup(tmp_path, m, parsed_options={"skills": True}) + created = bob.setup(tmp_path, m, parsed_options={"legacy_commands": True}) assert len(created) > 0 for f in created: assert f.exists() - assert f.name == "SKILL.md" - assert f.parent.name.startswith("speckit-") + assert f.suffix == ".md" + assert f.name.startswith("speckit.") + assert f.parent == tmp_path / ".bob" / "commands" - def test_setup_skills_does_not_warn_about_legacy(self, tmp_path): + def test_setup_legacy_warns_deprecated(self, tmp_path): from specify_cli.integrations.bob import BobIntegration bob = BobIntegration() m = IntegrationManifest("bob", tmp_path) + with pytest.warns(UserWarning, match="Bob legacy commands mode"): + bob.setup(tmp_path, m, parsed_options={"legacy_commands": True}) - with warnings.catch_warnings(record=True) as caught: - warnings.simplefilter("always") - bob.setup(tmp_path, m, parsed_options={"skills": True}) - - assert not any( - "Bob legacy markdown mode" in str(item.message) - for item in caught - ) - - def test_setup_skills_creates_correct_directory(self, tmp_path): + def test_setup_legacy_no_skills_dir(self, tmp_path): from specify_cli.integrations.bob import BobIntegration bob = BobIntegration() m = IntegrationManifest("bob", tmp_path) - bob.setup(tmp_path, m, parsed_options={"skills": True}) - skills_dir = tmp_path / ".bob" / "skills" - assert skills_dir.is_dir() + bob.setup(tmp_path, m, parsed_options={"legacy_commands": True}) + assert not (tmp_path / ".bob" / "skills").exists() - def test_setup_skills_no_commands_dir(self, tmp_path): + def test_setup_legacy_templates_are_processed(self, tmp_path): from specify_cli.integrations.bob import BobIntegration bob = BobIntegration() m = IntegrationManifest("bob", tmp_path) - bob.setup(tmp_path, m, parsed_options={"skills": True}) + bob.setup(tmp_path, m, parsed_options={"legacy_commands": True}) commands_dir = tmp_path / ".bob" / "commands" - assert not commands_dir.exists() - - def test_setup_skills_has_expected_commands(self, tmp_path): - from specify_cli.integrations.bob import BobIntegration - bob = BobIntegration() - m = IntegrationManifest("bob", tmp_path) - created = bob.setup(tmp_path, m, parsed_options={"skills": True}) - - expected_commands = { - "analyze", "clarify", "constitution", "converge", "implement", - "plan", "checklist", "specify", "tasks", "taskstoissues", - } - actual_commands = {f.parent.name.removeprefix("speckit-") for f in created} - assert actual_commands == expected_commands + for md_file in commands_dir.glob("speckit.*.md"): + content = md_file.read_text(encoding="utf-8") + assert "{SCRIPT}" not in content + assert "__AGENT__" not in content + assert "{ARGS}" not in content + assert "__SPECKIT_COMMAND_" not in content - def test_setup_skills_all_files_tracked(self, tmp_path): + def test_setup_legacy_all_files_tracked(self, tmp_path): from specify_cli.integrations.bob import BobIntegration bob = BobIntegration() m = IntegrationManifest("bob", tmp_path) - created = bob.setup(tmp_path, m, parsed_options={"skills": True}) + created = bob.setup(tmp_path, m, parsed_options={"legacy_commands": True}) for f in created: rel = f.resolve().relative_to(tmp_path.resolve()).as_posix() assert rel in m.files, f"{rel} not tracked in manifest" - def test_skills_uninstall_roundtrip(self, tmp_path): + def test_setup_legacy_uninstall_roundtrip(self, tmp_path): from specify_cli.integrations.bob import BobIntegration bob = BobIntegration() m = IntegrationManifest("bob", tmp_path) - created = bob.install(tmp_path, m, parsed_options={"skills": True}) + created = bob.install(tmp_path, m, parsed_options={"legacy_commands": True}) assert len(created) > 0 m.save() removed, skipped = bob.uninstall(tmp_path, m) @@ -210,9 +227,9 @@ def test_skills_uninstall_roundtrip(self, tmp_path): class TestBobInitFlowDefault: - """CLI init integration tests for default (markdown) mode.""" + """CLI init creates skills by default.""" - def test_init_default_creates_commands(self, tmp_path): + def test_init_default_creates_skills(self, tmp_path): from typer.testing import CliRunner from specify_cli import app @@ -222,11 +239,10 @@ def test_init_default_creates_commands(self, tmp_path): "--ignore-agent-tools", "--script", "sh", ]) assert result.exit_code == 0, f"init --integration bob failed: {result.output}" - assert (target / ".bob" / "commands" / "speckit.plan.md").exists() - assert not (target / ".bob" / "skills").exists() + assert (target / ".bob" / "skills" / "speckit-plan" / "SKILL.md").exists() + assert not (target / ".bob" / "commands").exists() def test_init_default_complete_file_inventory_sh(self, tmp_path): - """Default init creates .bob/commands/*.md files.""" from typer.testing import CliRunner from specify_cli import app @@ -248,24 +264,24 @@ def test_init_default_complete_file_inventory_sh(self, tmp_path): "plan", "checklist", "specify", "tasks", "taskstoissues", ] for cmd in commands: - assert (project / ".bob" / "commands" / f"speckit.{cmd}.md").exists(), ( - f"Missing .bob/commands/speckit.{cmd}.md" + assert (project / ".bob" / "skills" / f"speckit-{cmd}" / "SKILL.md").exists(), ( + f"Missing .bob/skills/speckit-{cmd}/SKILL.md" ) -class TestBobInitFlowSkills: - """CLI init integration tests for skills mode.""" +class TestBobInitFlowLegacy: + """CLI init with --legacy-commands produces .bob/commands/*.md.""" - def test_init_skills_creates_skill_files(self, tmp_path): + def test_init_legacy_creates_commands(self, tmp_path): from typer.testing import CliRunner from specify_cli import app target = tmp_path / "test-proj" result = CliRunner().invoke(app, [ "init", str(target), "--integration", "bob", - "--integration-options", "--skills", + "--integration-options", "--legacy-commands", "--ignore-agent-tools", "--script", "sh", ]) - assert result.exit_code == 0, f"init --integration bob --skills failed: {result.output}" - assert (target / ".bob" / "skills" / "speckit-plan" / "SKILL.md").exists() - assert not (target / ".bob" / "commands").exists() + assert result.exit_code == 0, f"init --integration bob --legacy-commands failed: {result.output}" + assert (target / ".bob" / "commands" / "speckit.plan.md").exists() + assert not (target / ".bob" / "skills").exists() From 71ba3b9f0a408fa80fcf41f4bfebe9209cb9bbe0 Mon Sep 17 00:00:00 2001 From: Davide Barletta Date: Tue, 14 Jul 2026 16:48:44 +0200 Subject: [PATCH 05/27] fix(bob): honor legacy_commands in ai_skills persistence and add bob to ALWAYS_SLASH_AGENTS - init.py: suppress ai_skills=True when --legacy-commands is passed so extensions and presets target .bob/commands, not .bob/skills - _invocation_style.py: add 'bob' to ALWAYS_SLASH_AGENTS so init next-steps and hook invocations always show /speckit- (skills is the default layout; no ai_skills flag required) --- src/specify_cli/_invocation_style.py | 3 ++- src/specify_cli/commands/init.py | 10 +++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/_invocation_style.py b/src/specify_cli/_invocation_style.py index 627967cfbd..09826e4af3 100644 --- a/src/specify_cli/_invocation_style.py +++ b/src/specify_cli/_invocation_style.py @@ -12,7 +12,8 @@ DOLLAR_SKILLS_AGENTS: frozenset[str] = frozenset({"codex", "zcode"}) # Agents that always render /speckit-, regardless of ai_skills. -ALWAYS_SLASH_AGENTS: frozenset[str] = frozenset({"devin", "trae", "zed"}) +# Bob is included because skills is its default layout (no flag required). +ALWAYS_SLASH_AGENTS: frozenset[str] = frozenset({"bob", "devin", "trae", "zed"}) # Agents that render /speckit- only when ai_skills is enabled. CONDITIONAL_SLASH_AGENTS: frozenset[str] = frozenset( diff --git a/src/specify_cli/commands/init.py b/src/specify_cli/commands/init.py index dd815b8c5d..3ec7f96054 100644 --- a/src/specify_cli/commands/init.py +++ b/src/specify_cli/commands/init.py @@ -498,9 +498,13 @@ def init( } from ..integrations.base import SkillsIntegration as _SkillsPersist - if isinstance(resolved_integration, _SkillsPersist) or getattr( - resolved_integration, "_skills_mode", False - ): + _legacy_commands = bool( + (integration_parsed_options or {}).get("legacy_commands") + ) + if ( + isinstance(resolved_integration, _SkillsPersist) + or getattr(resolved_integration, "_skills_mode", False) + ) and not _legacy_commands: init_opts["ai_skills"] = True save_init_options(project_path, init_opts) From d5e0e954097f84a48188db16aa042262520b73cb Mon Sep 17 00:00:00 2001 From: Davide Barletta <98974352+davidebibm@users.noreply.github.com> Date: Tue, 14 Jul 2026 17:14:55 +0200 Subject: [PATCH 06/27] Copilot suggestion Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- integrations/catalog.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integrations/catalog.json b/integrations/catalog.json index 221ba15d9d..fbdd666001 100644 --- a/integrations/catalog.json +++ b/integrations/catalog.json @@ -178,10 +178,10 @@ "id": "bob", "name": "IBM Bob", "version": "2.0.0", - "description": "IBM Bob 2.0 IDE integration", + "description": "IBM Bob 2.0 IDE skills-based integration", "author": "spec-kit-core", "repository": "https://github.com/github/spec-kit", - "tags": ["ide", "ibm"] + "tags": ["ide", "ibm", "skills"] }, "trae": { "id": "trae", From 3679697459fd65a10326e5c69b819a4fcf3af1e7 Mon Sep 17 00:00:00 2001 From: Davide Barletta Date: Tue, 14 Jul 2026 17:22:04 +0200 Subject: [PATCH 07/27] fix(bob): extend IntegrationBase directly to avoid false isinstance(SkillsIntegration) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - bob/__init__.py: switch BobIntegration base from SkillsIntegration to IntegrationBase; add _BobSkillsHelper for skills-mode delegation; set invoke_separator='-' explicitly; set _skills_mode flag in setup() so consumers can derive the effective mode without isinstance checks - _helpers.py: replace isinstance(integration, SkillsIntegration) guard with getattr(_skills_mode) so legacy-commands mode does not persist ai_skills=True - _invocation_style.py: remove 'bob' from ALWAYS_SLASH_AGENTS — Bob 2.0 skills are invoked via natural language, not /skill-name slash commands - integrations/catalog.json: advance updated_at to 2026-07-15 --- integrations/catalog.json | 2 +- src/specify_cli/_invocation_style.py | 3 +- src/specify_cli/integrations/_helpers.py | 2 +- src/specify_cli/integrations/bob/__init__.py | 49 +++++++++++++++++--- tests/integrations/test_integration_bob.py | 16 +++++-- 5 files changed, 58 insertions(+), 14 deletions(-) diff --git a/integrations/catalog.json b/integrations/catalog.json index fbdd666001..013d1be8b7 100644 --- a/integrations/catalog.json +++ b/integrations/catalog.json @@ -1,6 +1,6 @@ { "schema_version": "1.0", - "updated_at": "2026-06-23T00:00:00Z", + "updated_at": "2026-07-15T00:00:00Z", "catalog_url": "https://raw.githubusercontent.com/github/spec-kit/main/integrations/catalog.json", "integrations": { "claude": { diff --git a/src/specify_cli/_invocation_style.py b/src/specify_cli/_invocation_style.py index 09826e4af3..627967cfbd 100644 --- a/src/specify_cli/_invocation_style.py +++ b/src/specify_cli/_invocation_style.py @@ -12,8 +12,7 @@ DOLLAR_SKILLS_AGENTS: frozenset[str] = frozenset({"codex", "zcode"}) # Agents that always render /speckit-, regardless of ai_skills. -# Bob is included because skills is its default layout (no flag required). -ALWAYS_SLASH_AGENTS: frozenset[str] = frozenset({"bob", "devin", "trae", "zed"}) +ALWAYS_SLASH_AGENTS: frozenset[str] = frozenset({"devin", "trae", "zed"}) # Agents that render /speckit- only when ai_skills is enabled. CONDITIONAL_SLASH_AGENTS: frozenset[str] = frozenset( diff --git a/src/specify_cli/integrations/_helpers.py b/src/specify_cli/integrations/_helpers.py index d1bf051f77..a438505e37 100644 --- a/src/specify_cli/integrations/_helpers.py +++ b/src/specify_cli/integrations/_helpers.py @@ -270,7 +270,7 @@ def _update_init_options_for_integration( opts["speckit_version"] = _get_speckit_version() if script_type: opts["script"] = script_type - if isinstance(integration, SkillsIntegration) or getattr(integration, "_skills_mode", False): + if getattr(integration, "_skills_mode", False): opts["ai_skills"] = True else: opts.pop("ai_skills", None) diff --git a/src/specify_cli/integrations/bob/__init__.py b/src/specify_cli/integrations/bob/__init__.py index e47f3b4a28..e29ac0d60f 100644 --- a/src/specify_cli/integrations/bob/__init__.py +++ b/src/specify_cli/integrations/bob/__init__.py @@ -17,7 +17,7 @@ from pathlib import Path from typing import Any -from ..base import IntegrationOption, MarkdownIntegration, SkillsIntegration +from ..base import IntegrationBase, IntegrationOption, MarkdownIntegration, SkillsIntegration from ..manifest import IntegrationManifest @@ -55,7 +55,30 @@ class _BobMarkdownHelper(MarkdownIntegration): } -class BobIntegration(SkillsIntegration): +class _BobSkillsHelper(SkillsIntegration): + """Internal helper used when Bob is scaffolded in skills mode. + + Not registered in the integration registry — only used as a delegate + by ``BobIntegration`` for skills-mode setup. + """ + + key = "bob" + config = { + "name": "IBM Bob", + "folder": ".bob/", + "commands_subdir": "skills", + "install_url": None, + "requires_cli": False, + } + registrar_config = { + "dir": ".bob/skills", + "format": "markdown", + "args": "$ARGUMENTS", + "extension": "/SKILL.md", + } + + +class BobIntegration(IntegrationBase): """Integration for IBM Bob IDE. Default mode: installs ``.bob/skills/speckit-/SKILL.md`` files @@ -64,13 +87,17 @@ class BobIntegration(SkillsIntegration): Legacy mode (``--legacy-commands``): installs ``.bob/commands/speckit..md`` files (Bob 1.x layout — deprecated). - Inheriting ``SkillsIntegration`` ensures ``invoke_separator = "-"`` is - set at the class level so ``CommandRegistrar.AGENT_CONFIGS`` (which reads - the class attribute directly) generates correct hyphenated - ``/speckit-`` references for skills. + Extends ``IntegrationBase`` directly so that ``isinstance(integration, + SkillsIntegration)`` is ``False`` and consumers such as + ``_update_init_options_for_integration`` and the ``specify init`` + next-steps builder derive the effective mode from ``_skills_mode`` + rather than the class hierarchy. ``invoke_separator = "-"`` is set + explicitly to match the skills-default behaviour expected by + ``CommandRegistrar.AGENT_CONFIGS``. """ key = "bob" + invoke_separator = "-" config = { "name": "IBM Bob", "folder": ".bob/", @@ -85,6 +112,10 @@ class BobIntegration(SkillsIntegration): "extension": "/SKILL.md", } + # Set by setup() to reflect the active mode; read by _helpers.py and + # init.py via getattr(integration, "_skills_mode", False). + _skills_mode: bool = False + @classmethod def options(cls) -> list[IntegrationOption]: return [ @@ -115,9 +146,13 @@ def setup( """ parsed_options = parsed_options or {} if parsed_options.get("legacy_commands"): + self._skills_mode = False _warn_legacy_commands_deprecated() return self._setup_legacy(project_root, manifest, parsed_options, **opts) - return SkillsIntegration.setup(self, project_root, manifest, parsed_options, **opts) + self._skills_mode = True + return SkillsIntegration.setup( + _BobSkillsHelper(), project_root, manifest, parsed_options, **opts + ) def _setup_legacy( self, diff --git a/tests/integrations/test_integration_bob.py b/tests/integrations/test_integration_bob.py index 6d362ca772..eb52317f23 100644 --- a/tests/integrations/test_integration_bob.py +++ b/tests/integrations/test_integration_bob.py @@ -16,9 +16,19 @@ def test_registered(self): assert "bob" in INTEGRATION_REGISTRY assert get_integration("bob") is not None - def test_is_skills_integration(self): - """BobIntegration is a SkillsIntegration — skills are the default.""" - assert isinstance(get_integration("bob"), SkillsIntegration) + def test_is_integration_base_not_skills_integration(self): + """BobIntegration extends IntegrationBase directly — not SkillsIntegration. + + It must NOT be an instance of SkillsIntegration so that consumers + such as _update_init_options_for_integration and the init next-steps + builder derive the effective mode from _skills_mode rather than the + class hierarchy. invoke_separator='-' is set explicitly on the class. + """ + from specify_cli.integrations.base import IntegrationBase + bob = get_integration("bob") + assert isinstance(bob, IntegrationBase) + assert not isinstance(bob, SkillsIntegration) + assert bob.invoke_separator == "-" def test_key_and_config(self): bob = get_integration("bob") From 64c0b9d005a0c3cf3924df0a99eeb5c2948431ef Mon Sep 17 00:00:00 2001 From: Davide Barletta Date: Tue, 14 Jul 2026 17:25:37 +0200 Subject: [PATCH 08/27] fix(lint): remove unused SkillsIntegration import from _helpers.py --- src/specify_cli/integrations/_helpers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/specify_cli/integrations/_helpers.py b/src/specify_cli/integrations/_helpers.py index a438505e37..b18a793d9d 100644 --- a/src/specify_cli/integrations/_helpers.py +++ b/src/specify_cli/integrations/_helpers.py @@ -263,7 +263,6 @@ def _update_init_options_for_integration( load_init_options, save_init_options, ) - from .base import SkillsIntegration opts = load_init_options(project_root) opts["integration"] = integration.key opts["ai"] = integration.key From 6ab8d6109f0621910dd30a5e30b7f70ce9e8bb2c Mon Sep 17 00:00:00 2001 From: Davide Barletta <98974352+davidebibm@users.noreply.github.com> Date: Tue, 14 Jul 2026 18:02:02 +0200 Subject: [PATCH 09/27] Copilot suggested change Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/specify_cli/integrations/bob/__init__.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/specify_cli/integrations/bob/__init__.py b/src/specify_cli/integrations/bob/__init__.py index e29ac0d60f..a61c6c9782 100644 --- a/src/specify_cli/integrations/bob/__init__.py +++ b/src/specify_cli/integrations/bob/__init__.py @@ -98,6 +98,14 @@ class BobIntegration(IntegrationBase): key = "bob" invoke_separator = "-" + + def effective_invoke_separator( + self, parsed_options: dict[str, Any] | None = None + ) -> str: + """Return the invocation separator for the selected Bob layout.""" + if parsed_options and parsed_options.get("legacy_commands"): + return "." + return "-" config = { "name": "IBM Bob", "folder": ".bob/", From 7edf5b5e73ec35303c331ecda36b9cf73e506ad2 Mon Sep 17 00:00:00 2001 From: Davide Barletta Date: Wed, 15 Jul 2026 09:23:38 +0200 Subject: [PATCH 10/27] feat(bob): add bob skills integration with registrar-based mode detection --- src/specify_cli/_invocation_style.py | 1 + src/specify_cli/commands/init.py | 2 ++ src/specify_cli/integrations/_helpers.py | 3 ++- src/specify_cli/integrations/bob/__init__.py | 19 ++++++++++++++----- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/specify_cli/_invocation_style.py b/src/specify_cli/_invocation_style.py index 627967cfbd..438e3b738d 100644 --- a/src/specify_cli/_invocation_style.py +++ b/src/specify_cli/_invocation_style.py @@ -18,6 +18,7 @@ CONDITIONAL_SLASH_AGENTS: frozenset[str] = frozenset( { "agy", + "bob", "claude", "copilot", "cursor-agent", diff --git a/src/specify_cli/commands/init.py b/src/specify_cli/commands/init.py index 3ec7f96054..8e33acf8a9 100644 --- a/src/specify_cli/commands/init.py +++ b/src/specify_cli/commands/init.py @@ -658,6 +658,7 @@ def init( kimi_skill_mode = selected_ai == "kimi" agy_skill_mode = selected_ai == "agy" and _is_skills_integration trae_skill_mode = selected_ai == "trae" + bob_skill_mode = selected_ai == "bob" and _is_skills_integration cursor_agent_skill_mode = ( selected_ai == "cursor-agent" and _is_skills_integration ) @@ -672,6 +673,7 @@ def init( or kimi_skill_mode or agy_skill_mode or trae_skill_mode + or bob_skill_mode or cursor_agent_skill_mode or copilot_skill_mode or devin_skill_mode diff --git a/src/specify_cli/integrations/_helpers.py b/src/specify_cli/integrations/_helpers.py index b18a793d9d..d1bf051f77 100644 --- a/src/specify_cli/integrations/_helpers.py +++ b/src/specify_cli/integrations/_helpers.py @@ -263,13 +263,14 @@ def _update_init_options_for_integration( load_init_options, save_init_options, ) + from .base import SkillsIntegration opts = load_init_options(project_root) opts["integration"] = integration.key opts["ai"] = integration.key opts["speckit_version"] = _get_speckit_version() if script_type: opts["script"] = script_type - if getattr(integration, "_skills_mode", False): + if isinstance(integration, SkillsIntegration) or getattr(integration, "_skills_mode", False): opts["ai_skills"] = True else: opts.pop("ai_skills", None) diff --git a/src/specify_cli/integrations/bob/__init__.py b/src/specify_cli/integrations/bob/__init__.py index a61c6c9782..ed2da49f38 100644 --- a/src/specify_cli/integrations/bob/__init__.py +++ b/src/specify_cli/integrations/bob/__init__.py @@ -106,6 +106,7 @@ def effective_invoke_separator( if parsed_options and parsed_options.get("legacy_commands"): return "." return "-" + config = { "name": "IBM Bob", "folder": ".bob/", @@ -120,9 +121,17 @@ def effective_invoke_separator( "extension": "/SKILL.md", } - # Set by setup() to reflect the active mode; read by _helpers.py and - # init.py via getattr(integration, "_skills_mode", False). - _skills_mode: bool = False + @property + def _skills_mode(self) -> bool: + """True when the instance is configured in skills (default) mode. + + Derived from the current ``registrar_config`` so that the value + reflects whichever mode ``setup()`` last activated, without relying + on mutable instance state that is unavailable in a fresh process + (e.g. ``specify integration use bob`` or ``_set_default_integration``). + """ + rc = self.registrar_config or {} + return rc.get("extension") == "/SKILL.md" @classmethod def options(cls) -> list[IntegrationOption]: @@ -154,10 +163,10 @@ def setup( """ parsed_options = parsed_options or {} if parsed_options.get("legacy_commands"): - self._skills_mode = False + self.registrar_config = dict(_BobMarkdownHelper.registrar_config) _warn_legacy_commands_deprecated() return self._setup_legacy(project_root, manifest, parsed_options, **opts) - self._skills_mode = True + self.registrar_config = dict(_BobSkillsHelper.registrar_config) return SkillsIntegration.setup( _BobSkillsHelper(), project_root, manifest, parsed_options, **opts ) From 68bb87642662f87b911aaa4578f65618ae0fb1f1 Mon Sep 17 00:00:00 2001 From: Davide Barletta Date: Wed, 15 Jul 2026 16:15:14 +0200 Subject: [PATCH 11/27] address 3 comments from copilot --- src/specify_cli/commands/init.py | 18 ++++++++----- src/specify_cli/integrations/_helpers.py | 12 +++++++-- .../integrations/_install_commands.py | 4 ++- .../integrations/_migrate_commands.py | 4 ++- src/specify_cli/integrations/bob/__init__.py | 25 +++++++++---------- 5 files changed, 40 insertions(+), 23 deletions(-) diff --git a/src/specify_cli/commands/init.py b/src/specify_cli/commands/init.py index 8e33acf8a9..d2338d64cc 100644 --- a/src/specify_cli/commands/init.py +++ b/src/specify_cli/commands/init.py @@ -501,10 +501,13 @@ def init( _legacy_commands = bool( (integration_parsed_options or {}).get("legacy_commands") ) - if ( + _skills_mode_attr = getattr(resolved_integration, "_skills_mode", None) + _is_skills = ( isinstance(resolved_integration, _SkillsPersist) - or getattr(resolved_integration, "_skills_mode", False) - ) and not _legacy_commands: + or (callable(_skills_mode_attr) and _skills_mode_attr(integration_parsed_options)) + or (not callable(_skills_mode_attr) and bool(_skills_mode_attr)) + ) + if _is_skills and not _legacy_commands: init_opts["ai_skills"] = True save_init_options(project_path, init_opts) @@ -648,9 +651,12 @@ def init( from ..integrations.base import SkillsIntegration as _SkillsInt - _is_skills_integration = isinstance( - resolved_integration, _SkillsInt - ) or getattr(resolved_integration, "_skills_mode", False) + _skills_mode_attr = getattr(resolved_integration, "_skills_mode", None) + _is_skills_integration = ( + isinstance(resolved_integration, _SkillsInt) + or (callable(_skills_mode_attr) and _skills_mode_attr(integration_parsed_options)) + or (not callable(_skills_mode_attr) and bool(_skills_mode_attr)) + ) codex_skill_mode = selected_ai == "codex" and _is_skills_integration zcode_skill_mode = selected_ai == "zcode" and _is_skills_integration diff --git a/src/specify_cli/integrations/_helpers.py b/src/specify_cli/integrations/_helpers.py index d1bf051f77..506a74935e 100644 --- a/src/specify_cli/integrations/_helpers.py +++ b/src/specify_cli/integrations/_helpers.py @@ -252,6 +252,7 @@ def _update_init_options_for_integration( project_root: Path, integration: Any, script_type: str | None = None, + parsed_options: dict[str, Any] | None = None, ) -> None: """Update init-options.json to reflect *integration* as the active one. @@ -270,7 +271,12 @@ def _update_init_options_for_integration( opts["speckit_version"] = _get_speckit_version() if script_type: opts["script"] = script_type - if isinstance(integration, SkillsIntegration) or getattr(integration, "_skills_mode", False): + _skills_mode_attr = getattr(integration, "_skills_mode", None) + if callable(_skills_mode_attr): + is_skills = _skills_mode_attr(parsed_options) + else: + is_skills = bool(_skills_mode_attr) or isinstance(integration, SkillsIntegration) + if is_skills: opts["ai_skills"] = True else: opts.pop("ai_skills", None) @@ -326,7 +332,9 @@ def _set_default_integration( ) from exc _write_integration_json(project_root, key, installed_keys, settings) - _update_init_options_for_integration(project_root, integration, script_type=resolved_script) + _update_init_options_for_integration( + project_root, integration, script_type=resolved_script, parsed_options=parsed_options + ) def _set_default_integration_or_exit(*args: Any, **kwargs: Any) -> None: diff --git a/src/specify_cli/integrations/_install_commands.py b/src/specify_cli/integrations/_install_commands.py index 66fd2b2d26..7337a38d10 100644 --- a/src/specify_cli/integrations/_install_commands.py +++ b/src/specify_cli/integrations/_install_commands.py @@ -158,7 +158,9 @@ def integration_install( ) _write_integration_json(project_root, new_default, new_installed, settings) if new_default == integration.key: - _update_init_options_for_integration(project_root, integration, script_type=selected_script) + _update_init_options_for_integration( + project_root, integration, script_type=selected_script, parsed_options=parsed_options + ) else: _refresh_init_options_speckit_version(project_root) diff --git a/src/specify_cli/integrations/_migrate_commands.py b/src/specify_cli/integrations/_migrate_commands.py index 6568d1af18..ce8820d161 100644 --- a/src/specify_cli/integrations/_migrate_commands.py +++ b/src/specify_cli/integrations/_migrate_commands.py @@ -463,7 +463,9 @@ def integration_upgrade( new_manifest.save() _write_integration_json(project_root, installed_key, installed_keys, settings) if installed_key == key: - _update_init_options_for_integration(project_root, integration, script_type=selected_script) + _update_init_options_for_integration( + project_root, integration, script_type=selected_script, parsed_options=parsed_options + ) else: _refresh_init_options_speckit_version(project_root) except Exception as exc: diff --git a/src/specify_cli/integrations/bob/__init__.py b/src/specify_cli/integrations/bob/__init__.py index ed2da49f38..3c4c123b96 100644 --- a/src/specify_cli/integrations/bob/__init__.py +++ b/src/specify_cli/integrations/bob/__init__.py @@ -77,6 +77,10 @@ class _BobSkillsHelper(SkillsIntegration): "extension": "/SKILL.md", } + def post_process_skill_content(self, content: str) -> str: + """Bob skills are intent-activated; no slash-command note is needed.""" + return content + class BobIntegration(IntegrationBase): """Integration for IBM Bob IDE. @@ -121,17 +125,16 @@ def effective_invoke_separator( "extension": "/SKILL.md", } - @property - def _skills_mode(self) -> bool: + def _skills_mode(self, parsed_options: dict[str, Any] | None = None) -> bool: """True when the instance is configured in skills (default) mode. - Derived from the current ``registrar_config`` so that the value - reflects whichever mode ``setup()`` last activated, without relying - on mutable instance state that is unavailable in a fresh process - (e.g. ``specify integration use bob`` or ``_set_default_integration``). + Derived from *parsed_options* so that the value is correct both + during ``setup()`` and in fresh-process contexts where ``setup()`` + has not been called (e.g. ``specify integration use bob``). """ - rc = self.registrar_config or {} - return rc.get("extension") == "/SKILL.md" + if parsed_options is None: + parsed_options = {} + return not parsed_options.get("legacy_commands", False) @classmethod def options(cls) -> list[IntegrationOption]: @@ -163,13 +166,9 @@ def setup( """ parsed_options = parsed_options or {} if parsed_options.get("legacy_commands"): - self.registrar_config = dict(_BobMarkdownHelper.registrar_config) _warn_legacy_commands_deprecated() return self._setup_legacy(project_root, manifest, parsed_options, **opts) - self.registrar_config = dict(_BobSkillsHelper.registrar_config) - return SkillsIntegration.setup( - _BobSkillsHelper(), project_root, manifest, parsed_options, **opts - ) + return _BobSkillsHelper().setup(project_root, manifest, parsed_options, **opts) def _setup_legacy( self, From f47ab611960f377cff3ca01b3491ce5533bea27f Mon Sep 17 00:00:00 2001 From: Davide Barletta Date: Wed, 15 Jul 2026 16:41:31 +0200 Subject: [PATCH 12/27] feat(bob): update registrar config to use legacy commands layout --- src/specify_cli/integrations/bob/__init__.py | 23 ++++-- tests/integrations/test_integration_bob.py | 84 +++++++++++++++++++- 2 files changed, 99 insertions(+), 8 deletions(-) diff --git a/src/specify_cli/integrations/bob/__init__.py b/src/specify_cli/integrations/bob/__init__.py index 3c4c123b96..261aec0364 100644 --- a/src/specify_cli/integrations/bob/__init__.py +++ b/src/specify_cli/integrations/bob/__init__.py @@ -96,8 +96,18 @@ class BobIntegration(IntegrationBase): ``_update_init_options_for_integration`` and the ``specify init`` next-steps builder derive the effective mode from ``_skills_mode`` rather than the class hierarchy. ``invoke_separator = "-"`` is set - explicitly to match the skills-default behaviour expected by - ``CommandRegistrar.AGENT_CONFIGS``. + explicitly so that ``effective_invoke_separator()`` returns ``"-"`` + for skills-mode invocations. + + ``registrar_config`` intentionally mirrors the legacy commands layout + (``extension: ".md"``, ``dir: ".bob/commands"``) so that + ``CommandRegistrar.AGENT_CONFIGS["bob"]`` follows the same pattern as + Copilot: extension/preset registration writes to ``.bob/commands/`` + for legacy-mode projects, and is transparently skipped for skills-mode + projects (``skills_mode_active`` becomes ``True`` because + ``ai_skills=True`` and ``extension != "/SKILL.md"``). The + ``_BobSkillsHelper`` class owns the SKILL.md layout used exclusively + during ``setup()`` (``specify init`` / ``specify integration install``). """ key = "bob" @@ -114,15 +124,18 @@ def effective_invoke_separator( config = { "name": "IBM Bob", "folder": ".bob/", - "commands_subdir": "skills", + "commands_subdir": "commands", "install_url": None, "requires_cli": False, } registrar_config = { - "dir": ".bob/skills", + "dir": ".bob/commands", "format": "markdown", "args": "$ARGUMENTS", - "extension": "/SKILL.md", + "extension": ".md", + # Legacy commands use dot-notation (/speckit.plan); the class-level + # invoke_separator="-" applies to skills-mode invocations only. + "invoke_separator": ".", } def _skills_mode(self, parsed_options: dict[str, Any] | None = None) -> bool: diff --git a/tests/integrations/test_integration_bob.py b/tests/integrations/test_integration_bob.py index eb52317f23..ef1be7ef7e 100644 --- a/tests/integrations/test_integration_bob.py +++ b/tests/integrations/test_integration_bob.py @@ -34,9 +34,13 @@ def test_key_and_config(self): bob = get_integration("bob") assert bob.key == "bob" assert bob.config["folder"] == ".bob/" - assert bob.config["commands_subdir"] == "skills" - assert bob.registrar_config["dir"] == ".bob/skills" - assert bob.registrar_config["extension"] == "/SKILL.md" + # registrar_config mirrors the legacy commands layout so that + # CommandRegistrar.AGENT_CONFIGS["bob"] follows the Copilot pattern: + # extension registration writes to .bob/commands/ for legacy-mode + # projects and is skipped for skills-mode projects (skills_mode_active). + assert bob.config["commands_subdir"] == "commands" + assert bob.registrar_config["dir"] == ".bob/commands" + assert bob.registrar_config["extension"] == ".md" def test_invoke_separator_is_hyphen(self): """Class-level invoke_separator must be '-' so CommandRegistrar.AGENT_CONFIGS @@ -295,3 +299,77 @@ def test_init_legacy_creates_commands(self, tmp_path): assert result.exit_code == 0, f"init --integration bob --legacy-commands failed: {result.output}" assert (target / ".bob" / "commands" / "speckit.plan.md").exists() assert not (target / ".bob" / "skills").exists() + + +class TestBobRegistrarConfig: + """Verify AGENT_CONFIGS["bob"] follows the Copilot pattern for extension registration.""" + + def test_registrar_config_uses_commands_layout(self): + """AGENT_CONFIGS["bob"] must use the legacy .md layout (not /SKILL.md). + + This mirrors Copilot: the static registrar config targets the non-skills + format so that: + - skills_mode_active becomes True when ai_skills=True, preventing + extension registration from writing SKILL.md files into .bob/skills/ + on projects that never asked for legacy files. + - legacy-mode projects receive extension .md files in .bob/commands/. + """ + from specify_cli.agents import CommandRegistrar + registrar = CommandRegistrar() + bob_cfg = registrar.AGENT_CONFIGS.get("bob") + assert bob_cfg is not None, "bob must be in AGENT_CONFIGS" + assert bob_cfg["extension"] == ".md", ( + "AGENT_CONFIGS['bob']['extension'] must be '.md' so that " + "skills_mode_active=True suppresses extension registration on " + "skills-mode projects (mirrors the Copilot pattern)" + ) + assert bob_cfg["dir"] == ".bob/commands" + + def test_skills_mode_project_extension_registration_skipped(self, tmp_path): + """Extension registrar skips Bob on skills-mode projects (no .bob/commands dir).""" + from specify_cli.agents import CommandRegistrar + # Simulate a skills-mode Bob project: .bob/skills exists, .bob/commands does not + (tmp_path / ".bob" / "skills").mkdir(parents=True) + + registrar = CommandRegistrar() + results = registrar.register_commands_for_all_agents( + commands=[{"name": "speckit.test-cmd", "file": "test.md"}], + source_id="test", + source_dir=tmp_path, + project_root=tmp_path, + ) + # Bob must not appear in results — .bob/commands doesn't exist + assert "bob" not in results + + def test_legacy_mode_project_extension_registration_runs(self, tmp_path): + """Extension registrar writes to .bob/commands/ for legacy-mode projects.""" + import textwrap + from specify_cli.agents import CommandRegistrar + + # Simulate a legacy-mode Bob project: .bob/commands exists, .bob/skills does not + commands_dir = tmp_path / ".bob" / "commands" + commands_dir.mkdir(parents=True) + + # Provide a minimal command source file + cmd_file = tmp_path / "test.md" + cmd_file.write_text( + textwrap.dedent("""\ + --- + description: "Test command" + --- + Test body. + """), + encoding="utf-8", + ) + + registrar = CommandRegistrar() + results = registrar.register_commands_for_all_agents( + commands=[{"name": "speckit.test-cmd", "file": "test.md"}], + source_id="test", + source_dir=tmp_path, + project_root=tmp_path, + ) + assert "bob" in results, "bob must appear in results for legacy-mode project" + registered_file = commands_dir / "speckit.test-cmd.md" + assert registered_file.exists(), f"Expected {registered_file} to be written" + From d85128901da735ae9c0397fb1800cd4e338ba3df Mon Sep 17 00:00:00 2001 From: Davide Barletta Date: Wed, 15 Jul 2026 16:45:27 +0200 Subject: [PATCH 13/27] fix lint --- tests/integrations/test_integration_bob.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integrations/test_integration_bob.py b/tests/integrations/test_integration_bob.py index ef1be7ef7e..727e76c4b0 100644 --- a/tests/integrations/test_integration_bob.py +++ b/tests/integrations/test_integration_bob.py @@ -372,4 +372,3 @@ def test_legacy_mode_project_extension_registration_runs(self, tmp_path): assert "bob" in results, "bob must appear in results for legacy-mode project" registered_file = commands_dir / "speckit.test-cmd.md" assert registered_file.exists(), f"Expected {registered_file} to be written" - From 73725c5856ab802565c718a3550b127ba7b85479 Mon Sep 17 00:00:00 2001 From: Davide Barletta <98974352+davidebibm@users.noreply.github.com> Date: Thu, 16 Jul 2026 08:34:44 +0200 Subject: [PATCH 14/27] Suggested fix from Copilot Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/specify_cli/integrations/_helpers.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/specify_cli/integrations/_helpers.py b/src/specify_cli/integrations/_helpers.py index 506a74935e..c2a007e182 100644 --- a/src/specify_cli/integrations/_helpers.py +++ b/src/specify_cli/integrations/_helpers.py @@ -276,6 +276,14 @@ def _update_init_options_for_integration( is_skills = _skills_mode_attr(parsed_options) else: is_skills = bool(_skills_mode_attr) or isinstance(integration, SkillsIntegration) + + if ( + integration.key == "bob" + and parsed_options is None + and (project_root / ".bob" / "commands").exists() + and not (project_root / ".bob" / "skills").exists() + ): + is_skills = False if is_skills: opts["ai_skills"] = True else: From c6edc66d8f272a4f123d24e556893e0bd36585d3 Mon Sep 17 00:00:00 2001 From: Davide Barletta Date: Thu, 16 Jul 2026 14:16:33 +0200 Subject: [PATCH 15/27] fix pr comment --- src/specify_cli/integrations/_helpers.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/specify_cli/integrations/_helpers.py b/src/specify_cli/integrations/_helpers.py index ccca2096f8..07a62efeed 100644 --- a/src/specify_cli/integrations/_helpers.py +++ b/src/specify_cli/integrations/_helpers.py @@ -279,19 +279,6 @@ def _update_init_options_for_integration( opts["speckit_version"] = _get_speckit_version() if script_type: opts["script"] = script_type - _skills_mode_attr = getattr(integration, "_skills_mode", None) - if callable(_skills_mode_attr): - is_skills = _skills_mode_attr(parsed_options) - else: - is_skills = bool(_skills_mode_attr) or isinstance(integration, SkillsIntegration) - - if ( - integration.key == "bob" - and parsed_options is None - and (project_root / ".bob" / "commands").exists() - and not (project_root / ".bob" / "skills").exists() - ): - is_skills = False # Skills mode is either intrinsic (SkillsIntegration), set on the instance # during setup() (_skills_mode), or requested via parsed options (e.g. # Copilot's --skills, persisted as parsed_options["skills"]). The latter is From ef2cb53c281a71cc2ba2c32b115f2c5cbe79dbfa Mon Sep 17 00:00:00 2001 From: Davide Barletta Date: Thu, 16 Jul 2026 14:59:42 +0200 Subject: [PATCH 16/27] fix pr comment --- src/specify_cli/integrations/_helpers.py | 16 +++++++++------ tests/integrations/test_integration_bob.py | 24 ++++++++++++++++++++++ 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/src/specify_cli/integrations/_helpers.py b/src/specify_cli/integrations/_helpers.py index 07a62efeed..dfbab4f482 100644 --- a/src/specify_cli/integrations/_helpers.py +++ b/src/specify_cli/integrations/_helpers.py @@ -279,14 +279,18 @@ def _update_init_options_for_integration( opts["speckit_version"] = _get_speckit_version() if script_type: opts["script"] = script_type - # Skills mode is either intrinsic (SkillsIntegration), set on the instance - # during setup() (_skills_mode), or requested via parsed options (e.g. - # Copilot's --skills, persisted as parsed_options["skills"]). The latter is - # the only signal available on the `use` path, where no setup() runs and a - # fresh integration instance has _skills_mode == False (issue #3550). + # Skills mode is either intrinsic (SkillsIntegration), derived from + # parsed_options via a callable _skills_mode method (e.g. Bob), set on + # the instance during setup() as a bool attribute (e.g. Copilot), or + # requested via parsed options (e.g. Copilot's --skills, persisted as + # parsed_options["skills"]). The latter is the only signal available on + # the `use` path, where no setup() runs and a fresh integration instance + # has _skills_mode == False (issue #3550). + _skills_mode_attr = getattr(integration, "_skills_mode", None) skills_mode = ( isinstance(integration, SkillsIntegration) - or getattr(integration, "_skills_mode", False) + or (callable(_skills_mode_attr) and _skills_mode_attr(parsed_options)) + or (not callable(_skills_mode_attr) and bool(_skills_mode_attr)) or bool((parsed_options or {}).get("skills")) ) if skills_mode: diff --git a/tests/integrations/test_integration_bob.py b/tests/integrations/test_integration_bob.py index 727e76c4b0..850fb27c7e 100644 --- a/tests/integrations/test_integration_bob.py +++ b/tests/integrations/test_integration_bob.py @@ -300,6 +300,30 @@ def test_init_legacy_creates_commands(self, tmp_path): assert (target / ".bob" / "commands" / "speckit.plan.md").exists() assert not (target / ".bob" / "skills").exists() + def test_init_legacy_does_not_set_ai_skills(self, tmp_path): + """Legacy install must NOT write ai_skills=True to init-options.json. + + Regression test: _update_init_options_for_integration previously called + getattr(integration, "_skills_mode", False) which returned the bound method + object (always truthy) instead of calling it, so legacy projects incorrectly + got ai_skills=True and used hyphenated skill invocations. + """ + from typer.testing import CliRunner + from specify_cli import app + from specify_cli import load_init_options + + target = tmp_path / "test-proj" + result = CliRunner().invoke(app, [ + "init", str(target), "--integration", "bob", + "--integration-options", "--legacy-commands", + "--ignore-agent-tools", "--script", "sh", + ]) + assert result.exit_code == 0, f"init failed: {result.output}" + init_opts = load_init_options(target) + assert init_opts.get("ai_skills") is not True, ( + "Legacy Bob project must not have ai_skills=True in init-options.json" + ) + class TestBobRegistrarConfig: """Verify AGENT_CONFIGS["bob"] follows the Copilot pattern for extension registration.""" From cad76d37b2136e67ddbad2b7f9be75e98f60f5b4 Mon Sep 17 00:00:00 2001 From: Davide Barletta Date: Thu, 16 Jul 2026 17:36:02 +0200 Subject: [PATCH 17/27] fix pr comment --- src/specify_cli/integrations/bob/__init__.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/specify_cli/integrations/bob/__init__.py b/src/specify_cli/integrations/bob/__init__.py index 261aec0364..f02178570c 100644 --- a/src/specify_cli/integrations/bob/__init__.py +++ b/src/specify_cli/integrations/bob/__init__.py @@ -133,9 +133,6 @@ def effective_invoke_separator( "format": "markdown", "args": "$ARGUMENTS", "extension": ".md", - # Legacy commands use dot-notation (/speckit.plan); the class-level - # invoke_separator="-" applies to skills-mode invocations only. - "invoke_separator": ".", } def _skills_mode(self, parsed_options: dict[str, Any] | None = None) -> bool: From 5a8cb614d64888ebf8ab950d0fa74b1b23ab4233 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Thu, 16 Jul 2026 11:45:01 -0500 Subject: [PATCH 18/27] refactor(bob): resolve skills mode via base-class hooks + fix command-ref separators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rework the dual-mode handling introduced for Bob 2.0 so an integration's internal representation never leaks into shared init/install/upgrade code, and fix the legacy command-reference separator surfaced in review. Base-class contract: - Add IntegrationBase.is_skills_mode(parsed_options) — the single hook the shared machinery consults to decide whether to persist ai_skills and render skill invocations. SkillsIntegration returns True; Copilot honors --skills / self._skills_mode; Bob returns `not legacy_commands`. - Add IntegrationBase.invoke_separator_for_mode(skills_enabled) — resolves the command-ref separator from a project's persisted mode for registration paths that only have the ai_skills flag (no CLI parsed_options). Default is behavior-preserving; Bob maps skills->"-", legacy->".". - BobIntegration stays on IntegrationBase (mirroring Copilot, the other dual-mode agent) and delegates setup() to internal _BobSkillsHelper / _BobMarkdownHelper. Removes the _skills_mode method and all isinstance(SkillsIntegration) / callable(_skills_mode) probing from _helpers.py and init.py. Fix legacy separator (review feedback): CommandRegistrar.register_commands and PresetManager._resolve_skill_command_refs previously read the single static AGENT_CONFIGS[key]["invoke_separator"], so legacy .bob/commands/ extension and preset command refs rendered /speckit- instead of Bob 1.x /speckit.. Both now resolve the separator per project mode via invoke_separator_for_mode. Tests: add regression coverage for the is_skills_mode / invoke_separator_for_mode hooks and legacy extension command-ref separators; normalize a width-sensitive workflow assertion to match its siblings. Full suite green. 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 --- src/specify_cli/agents.py | 25 ++- src/specify_cli/commands/init.py | 33 ++-- src/specify_cli/integrations/_helpers.py | 23 +-- .../integrations/_install_commands.py | 5 +- .../integrations/_migrate_commands.py | 5 +- src/specify_cli/integrations/base.py | 41 +++++ src/specify_cli/integrations/bob/__init__.py | 173 +++++++++--------- .../integrations/copilot/__init__.py | 11 ++ src/specify_cli/presets/__init__.py | 39 +++- tests/integrations/test_integration_bob.py | 90 ++++++++- tests/test_workflows.py | 4 +- 11 files changed, 298 insertions(+), 151 deletions(-) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index e4d09ffe99..e5e91d7f21 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -637,6 +637,23 @@ def register_commands( is_cline_ext = agent_name == "cline" and source_id != "core" source_root = source_dir.resolve() + # Resolve the command-reference separator once for this agent/project. + # Dual-layout agents (e.g. Bob) use different separators for their + # skills vs command layouts, so we ask the integration to map the + # project's persisted skills state to the correct separator rather than + # relying on the single static AGENT_CONFIGS value. + _sep = agent_config.get("invoke_separator", ".") + try: + from specify_cli.integrations import get_integration # noqa: PLC0415 + + _integ = get_integration(agent_name) + if _integ is not None: + _sep = _integ.invoke_separator_for_mode( + is_ai_skills_enabled(load_init_options(project_root)) + ) + except Exception: + pass + for cmd_info in commands: cmd_name = cmd_info["name"] aliases = cmd_info.get("aliases", []) @@ -709,13 +726,15 @@ def register_commands( ) # Resolve __SPECKIT_COMMAND_*__ tokens using the agent's invoke separator. - # The separator is sourced from agent_config (populated by _build_agent_configs, - # which propagates each integration's invoke_separator class attribute). + # For dual-layout agents (e.g. Bob) the separator differs between the + # skills and command layouts, so a single static AGENT_CONFIGS value is + # insufficient. Resolve it from the integration using the project's + # persisted skills state; single-layout agents fall back to the static + # AGENT_CONFIGS value unchanged (invoke_separator_for_mode default). # Deferred import of IntegrationBase avoids a circular import at module load # (base.py itself imports CommandRegistrar lazily). from specify_cli.integrations.base import IntegrationBase # noqa: PLC0415 - _sep = agent_config.get("invoke_separator", ".") body = IntegrationBase.resolve_command_refs(body, _sep) output_name = self._compute_output_name(agent_name, cmd_name, agent_config) diff --git a/src/specify_cli/commands/init.py b/src/specify_cli/commands/init.py index 2779fa4bd3..88ea5afd5a 100644 --- a/src/specify_cli/commands/init.py +++ b/src/specify_cli/commands/init.py @@ -532,18 +532,9 @@ def init( "feature_numbering": "sequential", "speckit_version": get_speckit_version(), } - from ..integrations.base import SkillsIntegration as _SkillsPersist - - _legacy_commands = bool( - (integration_parsed_options or {}).get("legacy_commands") - ) - _skills_mode_attr = getattr(resolved_integration, "_skills_mode", None) - _is_skills = ( - isinstance(resolved_integration, _SkillsPersist) - or (callable(_skills_mode_attr) and _skills_mode_attr(integration_parsed_options)) - or (not callable(_skills_mode_attr) and bool(_skills_mode_attr)) - ) - if _is_skills and not _legacy_commands: + if resolved_integration.is_skills_mode( + integration_parsed_options or None + ): init_opts["ai_skills"] = True save_init_options(project_path, init_opts) @@ -690,13 +681,8 @@ def init( steps_lines.append("1. You're already in the project directory!") step_num = 2 - from ..integrations.base import SkillsIntegration as _SkillsInt - - _skills_mode_attr = getattr(resolved_integration, "_skills_mode", None) - _is_skills_integration = ( - isinstance(resolved_integration, _SkillsInt) - or (callable(_skills_mode_attr) and _skills_mode_attr(integration_parsed_options)) - or (not callable(_skills_mode_attr) and bool(_skills_mode_attr)) + _is_skills_integration = resolved_integration.is_skills_mode( + integration_parsed_options or None ) codex_skill_mode = selected_ai == "codex" and _is_skills_integration @@ -705,7 +691,6 @@ def init( kimi_skill_mode = selected_ai == "kimi" agy_skill_mode = selected_ai == "agy" and _is_skills_integration trae_skill_mode = selected_ai == "trae" - bob_skill_mode = selected_ai == "bob" and _is_skills_integration cursor_agent_skill_mode = ( selected_ai == "cursor-agent" and _is_skills_integration ) @@ -714,6 +699,7 @@ def init( zed_skill_mode = selected_ai == "zed" and _is_skills_integration grok_skill_mode = selected_ai == "grok" and _is_skills_integration cline_skill_mode = selected_ai == "cline" + bob_skill_mode = selected_ai == "bob" and _is_skills_integration native_skill_mode = ( codex_skill_mode or zcode_skill_mode @@ -721,12 +707,12 @@ def init( or kimi_skill_mode or agy_skill_mode or trae_skill_mode - or bob_skill_mode or cursor_agent_skill_mode or copilot_skill_mode or devin_skill_mode or zed_skill_mode or grok_skill_mode + or bob_skill_mode ) if codex_skill_mode: @@ -764,6 +750,11 @@ def init( f"{step_num}. Start Grok Build in this project directory; spec-kit skills were installed to [cyan].grok/skills[/cyan]" ) step_num += 1 + if bob_skill_mode: + steps_lines.append( + f"{step_num}. Start Bob in this project directory; spec-kit skills were installed to [cyan].bob/skills[/cyan]" + ) + step_num += 1 usage_label = "skills" if native_skill_mode else "slash commands" from .._invocation_style import ( diff --git a/src/specify_cli/integrations/_helpers.py b/src/specify_cli/integrations/_helpers.py index dfbab4f482..bfbbc04448 100644 --- a/src/specify_cli/integrations/_helpers.py +++ b/src/specify_cli/integrations/_helpers.py @@ -272,28 +272,19 @@ def _update_init_options_for_integration( load_init_options, save_init_options, ) - from .base import SkillsIntegration opts = load_init_options(project_root) opts["integration"] = integration.key opts["ai"] = integration.key opts["speckit_version"] = _get_speckit_version() if script_type: opts["script"] = script_type - # Skills mode is either intrinsic (SkillsIntegration), derived from - # parsed_options via a callable _skills_mode method (e.g. Bob), set on - # the instance during setup() as a bool attribute (e.g. Copilot), or - # requested via parsed options (e.g. Copilot's --skills, persisted as - # parsed_options["skills"]). The latter is the only signal available on - # the `use` path, where no setup() runs and a fresh integration instance - # has _skills_mode == False (issue #3550). - _skills_mode_attr = getattr(integration, "_skills_mode", None) - skills_mode = ( - isinstance(integration, SkillsIntegration) - or (callable(_skills_mode_attr) and _skills_mode_attr(parsed_options)) - or (not callable(_skills_mode_attr) and bool(_skills_mode_attr)) - or bool((parsed_options or {}).get("skills")) - ) - if skills_mode: + # Whether skills mode is active is owned by each integration via the + # ``is_skills_mode`` hook (base default honors ``--skills``; + # SkillsIntegration returns True; skills-first integrations with a legacy + # opt-out such as Bob override it). This keeps shared code free of + # ``isinstance`` / ``_skills_mode`` probing. Passing parsed_options lets it + # work on the ``use``/``install`` path where no setup() runs (issue #3550). + if integration.is_skills_mode(parsed_options): opts["ai_skills"] = True else: opts.pop("ai_skills", None) diff --git a/src/specify_cli/integrations/_install_commands.py b/src/specify_cli/integrations/_install_commands.py index 7337a38d10..f608a342b4 100644 --- a/src/specify_cli/integrations/_install_commands.py +++ b/src/specify_cli/integrations/_install_commands.py @@ -159,7 +159,10 @@ def integration_install( _write_integration_json(project_root, new_default, new_installed, settings) if new_default == integration.key: _update_init_options_for_integration( - project_root, integration, script_type=selected_script, parsed_options=parsed_options + project_root, + integration, + script_type=selected_script, + parsed_options=parsed_options, ) else: _refresh_init_options_speckit_version(project_root) diff --git a/src/specify_cli/integrations/_migrate_commands.py b/src/specify_cli/integrations/_migrate_commands.py index ce8820d161..64fb574e08 100644 --- a/src/specify_cli/integrations/_migrate_commands.py +++ b/src/specify_cli/integrations/_migrate_commands.py @@ -464,7 +464,10 @@ def integration_upgrade( _write_integration_json(project_root, installed_key, installed_keys, settings) if installed_key == key: _update_init_options_for_integration( - project_root, integration, script_type=selected_script, parsed_options=parsed_options + project_root, + integration, + script_type=selected_script, + parsed_options=parsed_options, ) else: _refresh_init_options_speckit_version(project_root) diff --git a/src/specify_cli/integrations/base.py b/src/specify_cli/integrations/base.py index a425d4e012..ab8b28c763 100644 --- a/src/specify_cli/integrations/base.py +++ b/src/specify_cli/integrations/base.py @@ -171,6 +171,43 @@ def effective_invoke_separator( """ return self.invoke_separator + def invoke_separator_for_mode(self, skills_enabled: bool) -> str: + """Command-ref separator given the project's *resolved* skills state. + + Registration paths (extension / preset command rendering) have no CLI + ``parsed_options`` — only the persisted ``ai_skills`` flag — so they + resolve the command-reference separator through this hook rather than + the static ``AGENT_CONFIGS[key]["invoke_separator"]`` value, which + cannot represent an agent whose separator differs between its skills + and command layouts. + + The default is mode-independent and returns exactly what + ``_build_agent_configs`` would place in ``AGENT_CONFIGS`` (the + ``registrar_config`` override if present, else the class-level + ``invoke_separator``), so single-layout agents are unaffected. + Dual-mode agents whose separator depends on the layout (e.g. Bob: + ``-`` for skills, ``.`` for legacy commands) override this. + """ + cfg = self.registrar_config or {} + return cfg.get("invoke_separator", self.invoke_separator) + + def is_skills_mode(self, parsed_options: dict[str, Any] | None = None) -> bool: + """Return whether this integration scaffolds skills for these options. + + This is the single, well-defined hook the shared init/install/upgrade + machinery consults to decide whether to persist ``ai_skills=True`` and + render skill invocations. It replaces ad-hoc ``isinstance`` / + ``getattr(self, "_skills_mode", ...)`` probing so an integration's + internal representation never has to leak into shared dispatch code. + + The default (command-first integrations, e.g. Copilot's default + layout) is skills mode only when ``--skills`` was requested. + ``SkillsIntegration`` overrides this to return ``True`` by default; + skills-first integrations that expose a legacy opt-out (e.g. Bob) + override it to honor their own flag. + """ + return bool((parsed_options or {}).get("skills")) + def build_exec_args( self, prompt: str, @@ -1376,6 +1413,10 @@ class SkillsIntegration(IntegrationBase): invoke_separator = "-" + def is_skills_mode(self, parsed_options: dict[str, Any] | None = None) -> bool: + """Skills-native integrations scaffold skills unconditionally.""" + return True + def build_exec_args( self, prompt: str, diff --git a/src/specify_cli/integrations/bob/__init__.py b/src/specify_cli/integrations/bob/__init__.py index f02178570c..985da67dda 100644 --- a/src/specify_cli/integrations/bob/__init__.py +++ b/src/specify_cli/integrations/bob/__init__.py @@ -1,13 +1,19 @@ """IBM Bob integration. -Bob 2.0 uses the ``.bob/skills/speckit-/SKILL.md`` layout. -The legacy ``.bob/commands/*.md`` layout (Bob 1.x) is available as an -opt-in for projects that have not yet migrated, via -``--integration-options "--legacy-commands"``. +Bob 2.0 uses the ``.bob/skills/speckit-/SKILL.md`` layout by default. +The legacy ``.bob/commands/*.md`` layout (Bob 1.x) remains available as an +opt-in via ``--integration-options "--legacy-commands"``. + +Bob is a *dual-mode* integration: whether it scaffolds skills or commands is +a per-project **configuration** decision (the ``--legacy-commands`` option, +persisted as ``ai_skills`` in init-options), not a property of the class. +It therefore extends :class:`IntegrationBase` (like Copilot, the other +dual-mode agent) and resolves the mode through the ``is_skills_mode`` hook, +delegating the actual scaffolding to a per-layout helper. Deprecation cycle: - This release: Skills layout is the default; legacy ``.bob/commands/`` - is opt-in via ``--legacy-commands``. + This release: Skills layout is the default; legacy ``.bob/commands/`` is + opt-in via ``--legacy-commands``. Next cycle: ``--legacy-commands`` flag removed. """ @@ -17,12 +23,16 @@ from pathlib import Path from typing import Any -from ..base import IntegrationBase, IntegrationOption, MarkdownIntegration, SkillsIntegration +from ..base import ( + IntegrationBase, + IntegrationOption, + MarkdownIntegration, + SkillsIntegration, +) from ..manifest import IntegrationManifest def _warn_legacy_commands_deprecated() -> None: - """Warn that Bob's legacy markdown layout is being phased out.""" warnings.warn( "Bob legacy commands mode (.bob/commands/) is deprecated and will be " "removed in a future Spec Kit release. Omit --legacy-commands to use " @@ -32,95 +42,79 @@ def _warn_legacy_commands_deprecated() -> None: ) -class _BobMarkdownHelper(MarkdownIntegration): - """Internal helper used when Bob is scaffolded in legacy commands mode. +class _BobSkillsHelper(SkillsIntegration): + """Default-mode helper: ``.bob/skills/speckit-/SKILL.md``. - Not registered in the integration registry — only used as a delegate - by ``BobIntegration`` when ``--legacy-commands`` is passed. + Not registered in the integration registry — used only as a delegate by + :class:`BobIntegration` for skills-mode ``setup()``. """ key = "bob" config = { "name": "IBM Bob", "folder": ".bob/", - "commands_subdir": "commands", + "commands_subdir": "skills", "install_url": None, "requires_cli": False, } registrar_config = { - "dir": ".bob/commands", + "dir": ".bob/skills", "format": "markdown", "args": "$ARGUMENTS", - "extension": ".md", + "extension": "/SKILL.md", } + def post_process_skill_content(self, content: str) -> str: + """Bob skills are intent-activated; no slash-command note is needed.""" + return content + -class _BobSkillsHelper(SkillsIntegration): - """Internal helper used when Bob is scaffolded in skills mode. +class _BobMarkdownHelper(MarkdownIntegration): + """Legacy-mode helper: ``.bob/commands/speckit..md`` (Bob 1.x). - Not registered in the integration registry — only used as a delegate - by ``BobIntegration`` for skills-mode setup. + Not registered in the integration registry — used only as a delegate by + :class:`BobIntegration` when ``--legacy-commands`` is passed. Declares + ``invoke_separator="."`` so command-reference tokens render as Bob 1.x + ``/speckit.`` invocations. """ key = "bob" + invoke_separator = "." config = { "name": "IBM Bob", "folder": ".bob/", - "commands_subdir": "skills", + "commands_subdir": "commands", "install_url": None, "requires_cli": False, } registrar_config = { - "dir": ".bob/skills", + "dir": ".bob/commands", "format": "markdown", "args": "$ARGUMENTS", - "extension": "/SKILL.md", + "extension": ".md", + "invoke_separator": ".", } - def post_process_skill_content(self, content: str) -> str: - """Bob skills are intent-activated; no slash-command note is needed.""" - return content - class BobIntegration(IntegrationBase): - """Integration for IBM Bob IDE. - - Default mode: installs ``.bob/skills/speckit-/SKILL.md`` files - (Bob 2.0 skills layout). - - Legacy mode (``--legacy-commands``): installs - ``.bob/commands/speckit..md`` files (Bob 1.x layout — deprecated). - - Extends ``IntegrationBase`` directly so that ``isinstance(integration, - SkillsIntegration)`` is ``False`` and consumers such as - ``_update_init_options_for_integration`` and the ``specify init`` - next-steps builder derive the effective mode from ``_skills_mode`` - rather than the class hierarchy. ``invoke_separator = "-"`` is set - explicitly so that ``effective_invoke_separator()`` returns ``"-"`` - for skills-mode invocations. - - ``registrar_config`` intentionally mirrors the legacy commands layout - (``extension: ".md"``, ``dir: ".bob/commands"``) so that - ``CommandRegistrar.AGENT_CONFIGS["bob"]`` follows the same pattern as - Copilot: extension/preset registration writes to ``.bob/commands/`` - for legacy-mode projects, and is transparently skipped for skills-mode - projects (``skills_mode_active`` becomes ``True`` because - ``ai_skills=True`` and ``extension != "/SKILL.md"``). The - ``_BobSkillsHelper`` class owns the SKILL.md layout used exclusively - during ``setup()`` (``specify init`` / ``specify integration install``). + """Integration for IBM Bob IDE (dual-mode; skills by default). + + Whether a project uses the skills or the legacy commands layout is a + configuration choice resolved by :meth:`is_skills_mode`, not the class + hierarchy. ``setup()`` delegates to the matching helper. + + ``registrar_config`` mirrors the *commands* layout (``extension: ".md"``, + ``dir: ".bob/commands"``) — the same pattern Copilot uses — so that + ``CommandRegistrar.AGENT_CONFIGS["bob"]`` drives extension/preset + registration into ``.bob/commands/`` for legacy-mode projects, while + skills-mode projects have that command registration transparently skipped + (``skills_mode_active`` becomes ``True`` because ``ai_skills=True`` and + ``extension != "/SKILL.md"``) and receive extension skills instead. + ``invoke_separator = "-"`` matches the default (skills) layout. """ key = "bob" invoke_separator = "-" - - def effective_invoke_separator( - self, parsed_options: dict[str, Any] | None = None - ) -> str: - """Return the invocation separator for the selected Bob layout.""" - if parsed_options and parsed_options.get("legacy_commands"): - return "." - return "-" - config = { "name": "IBM Bob", "folder": ".bob/", @@ -135,17 +129,6 @@ def effective_invoke_separator( "extension": ".md", } - def _skills_mode(self, parsed_options: dict[str, Any] | None = None) -> bool: - """True when the instance is configured in skills (default) mode. - - Derived from *parsed_options* so that the value is correct both - during ``setup()`` and in fresh-process contexts where ``setup()`` - has not been called (e.g. ``specify integration use bob``). - """ - if parsed_options is None: - parsed_options = {} - return not parsed_options.get("legacy_commands", False) - @classmethod def options(cls) -> list[IntegrationOption]: return [ @@ -161,32 +144,40 @@ def options(cls) -> list[IntegrationOption]: ), ] - def setup( - self, - project_root: Path, - manifest: IntegrationManifest, - parsed_options: dict[str, Any] | None = None, - **opts: Any, - ) -> list[Path]: - """Install Bob commands. + def is_skills_mode(self, parsed_options: dict[str, Any] | None = None) -> bool: + """Bob is skills-first; ``--legacy-commands`` opts out.""" + return not (parsed_options or {}).get("legacy_commands", False) + + def effective_invoke_separator( + self, parsed_options: dict[str, Any] | None = None + ) -> str: + """``"."`` for the legacy commands layout, ``"-"`` for skills.""" + return "-" if self.is_skills_mode(parsed_options) else "." + + def invoke_separator_for_mode(self, skills_enabled: bool) -> str: + """Resolve the command-ref separator from a project's persisted mode. - Default: skills layout (``.bob/skills/speckit-/SKILL.md``). - When ``parsed_options["legacy_commands"]`` is truthy, falls back to - the deprecated ``.bob/commands/speckit..md`` layout. + Skills projects render ``/speckit-``; legacy command projects + render Bob 1.x ``/speckit.``. Extension/preset registration + consults this (via the persisted ``ai_skills`` flag) so both layouts + get the correct separator despite sharing one static ``AGENT_CONFIGS`` + entry. """ - parsed_options = parsed_options or {} - if parsed_options.get("legacy_commands"): - _warn_legacy_commands_deprecated() - return self._setup_legacy(project_root, manifest, parsed_options, **opts) - return _BobSkillsHelper().setup(project_root, manifest, parsed_options, **opts) + return "-" if skills_enabled else "." - def _setup_legacy( + def setup( self, project_root: Path, manifest: IntegrationManifest, parsed_options: dict[str, Any] | None = None, **opts: Any, ) -> list[Path]: - """Legacy mode: ``.bob/commands/speckit..md`` layout.""" - helper = _BobMarkdownHelper() - return MarkdownIntegration.setup(helper, project_root, manifest, parsed_options, **opts) + parsed_options = parsed_options or {} + if self.is_skills_mode(parsed_options): + return _BobSkillsHelper().setup( + project_root, manifest, parsed_options, **opts + ) + _warn_legacy_commands_deprecated() + return MarkdownIntegration.setup( + _BobMarkdownHelper(), project_root, manifest, parsed_options, **opts + ) diff --git a/src/specify_cli/integrations/copilot/__init__.py b/src/specify_cli/integrations/copilot/__init__.py index 44bd47f353..85f5a292bc 100644 --- a/src/specify_cli/integrations/copilot/__init__.py +++ b/src/specify_cli/integrations/copilot/__init__.py @@ -131,6 +131,17 @@ def effective_invoke_separator( return "-" return self.invoke_separator + def is_skills_mode(self, parsed_options: dict[str, Any] | None = None) -> bool: + """Copilot is skills mode when ``--skills`` was requested. + + On the init path ``setup()`` has already recorded the choice in + ``self._skills_mode``; on the ``use``/``install`` path (where no + ``setup()`` runs) the signal comes from *parsed_options* (#3550). + """ + if parsed_options and parsed_options.get("skills"): + return True + return self._skills_mode + @classmethod def options(cls) -> list[IntegrationOption]: return [ diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 98bba08fd4..5a1aecc2d8 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -1171,7 +1171,7 @@ def _reconcile_skills(self, command_names: List[str]) -> None: selected_ai, fm, body, self.project_root ) body = self._resolve_skill_command_refs( - body, registrar, selected_ai + body, registrar, selected_ai, self.project_root ) from ..integrations import get_integration integration = get_integration(selected_ai) if isinstance(selected_ai, str) else None @@ -1252,7 +1252,10 @@ def _skill_title_from_command(cmd_name: str) -> str: @staticmethod def _resolve_skill_command_refs( - body: str, registrar: "CommandRegistrar", selected_ai: str + body: str, + registrar: "CommandRegistrar", + selected_ai: str, + project_root: "Path | None" = None, ) -> str: """Render ``__SPECKIT_COMMAND_*__`` tokens in a skill body as invocations. @@ -1261,10 +1264,30 @@ def _resolve_skill_command_refs( slash-command invocation — ``/speckit-`` for a ``-`` separator, ``/speckit.`` for ``.`` — the same rendering the command layer applies via ``CommandRegistrar.register_commands()``. + + For dual-layout agents (e.g. Bob) the separator depends on the + project's persisted skills state, so — when *project_root* is provided + — the separator is resolved from the integration via + ``invoke_separator_for_mode`` rather than the single static + ``AGENT_CONFIGS`` value. """ - separator = registrar.AGENT_CONFIGS.get(selected_ai, {}).get( - "invoke_separator", "." - ) + separator = None + if project_root is not None and isinstance(selected_ai, str): + try: + from .. import load_init_options + from ..integrations import get_integration + + integration = get_integration(selected_ai) + if integration is not None: + separator = integration.invoke_separator_for_mode( + is_ai_skills_enabled(load_init_options(project_root)) + ) + except Exception: + separator = None + if separator is None: + separator = registrar.AGENT_CONFIGS.get(selected_ai, {}).get( + "invoke_separator", "." + ) return IntegrationBase.resolve_command_refs(body, separator) def _build_extension_skill_restore_index(self) -> Dict[str, Dict[str, Any]]: @@ -1445,7 +1468,7 @@ def _register_skills( body = registrar.resolve_skill_placeholders( selected_ai, frontmatter, body, self.project_root ) - body = self._resolve_skill_command_refs(body, registrar, selected_ai) + body = self._resolve_skill_command_refs(body, registrar, selected_ai, self.project_root) for target_skill_name in target_skill_names: skill_subdir = skills_dir / target_skill_name @@ -1540,7 +1563,7 @@ def _unregister_skills(self, skill_names: List[str], preset_dir: Path) -> None: selected_ai, frontmatter, body, self.project_root ) body = self._resolve_skill_command_refs( - body, registrar, selected_ai + body, registrar, selected_ai, self.project_root ) original_desc = frontmatter.get("description", "") @@ -1592,7 +1615,7 @@ def _unregister_skills(self, skill_names: List[str], preset_dir: Path) -> None: selected_ai, frontmatter, body, self.project_root ) body = self._resolve_skill_command_refs( - body, registrar, selected_ai + body, registrar, selected_ai, self.project_root ) command_name = extension_restore["command_name"] diff --git a/tests/integrations/test_integration_bob.py b/tests/integrations/test_integration_bob.py index 850fb27c7e..92b1c9c40d 100644 --- a/tests/integrations/test_integration_bob.py +++ b/tests/integrations/test_integration_bob.py @@ -19,10 +19,14 @@ def test_registered(self): def test_is_integration_base_not_skills_integration(self): """BobIntegration extends IntegrationBase directly — not SkillsIntegration. - It must NOT be an instance of SkillsIntegration so that consumers - such as _update_init_options_for_integration and the init next-steps - builder derive the effective mode from _skills_mode rather than the - class hierarchy. invoke_separator='-' is set explicitly on the class. + Bob is dual-mode (skills by default, legacy commands via + ``--legacy-commands``), so its skills-ness is a per-project config + decision resolved by the ``is_skills_mode`` hook — not a class-hierarchy + property. It therefore must NOT be a ``SkillsIntegration`` (which is + reserved for statically skills-only agents); shared code consults + ``is_skills_mode(parsed_options)`` instead of ``isinstance``. + ``invoke_separator='-'`` is set explicitly on the class to match the + default (skills) layout. """ from specify_cli.integrations.base import IntegrationBase bob = get_integration("bob") @@ -68,6 +72,35 @@ def test_no_skills_flag(self): assert len(skills_opts) == 0 +class TestBobIsSkillsModeHook: + """The is_skills_mode hook is the single source of truth for the mode.""" + + def test_default_is_skills(self): + bob = get_integration("bob") + assert bob.is_skills_mode(None) is True + assert bob.is_skills_mode({}) is True + + def test_legacy_commands_disables_skills(self): + bob = get_integration("bob") + assert bob.is_skills_mode({"legacy_commands": True}) is False + + def test_effective_invoke_separator_tracks_mode(self): + bob = get_integration("bob") + assert bob.effective_invoke_separator(None) == "-" + assert bob.effective_invoke_separator({"legacy_commands": True}) == "." + + def test_invoke_separator_for_mode_tracks_persisted_state(self): + """Registration paths resolve the separator from persisted ai_skills.""" + bob = get_integration("bob") + assert bob.invoke_separator_for_mode(True) == "-" + assert bob.invoke_separator_for_mode(False) == "." + + def test_no_skills_mode_method_leaks(self): + """The old callable _skills_mode method must be gone; consumers use the hook.""" + bob = get_integration("bob") + assert not callable(getattr(bob, "_skills_mode", None)) + + class TestBobDefaultSkillsMode: """Default mode: .bob/skills/speckit-/SKILL.md layout.""" @@ -303,10 +336,12 @@ def test_init_legacy_creates_commands(self, tmp_path): def test_init_legacy_does_not_set_ai_skills(self, tmp_path): """Legacy install must NOT write ai_skills=True to init-options.json. - Regression test: _update_init_options_for_integration previously called - getattr(integration, "_skills_mode", False) which returned the bound method - object (always truthy) instead of calling it, so legacy projects incorrectly - got ai_skills=True and used hyphenated skill invocations. + Behavioral guard for the dual-mode contract: with --legacy-commands, + BobIntegration.is_skills_mode(parsed_options) returns False, so + _update_init_options_for_integration must not persist ai_skills=True. + (Regression origin: shared code previously probed a bound _skills_mode + method object, which is always truthy, and wrongly enabled skills for + legacy projects.) """ from typer.testing import CliRunner from specify_cli import app @@ -396,3 +431,42 @@ def test_legacy_mode_project_extension_registration_runs(self, tmp_path): assert "bob" in results, "bob must appear in results for legacy-mode project" registered_file = commands_dir / "speckit.test-cmd.md" assert registered_file.exists(), f"Expected {registered_file} to be written" + + def test_legacy_extension_command_refs_use_dot_separator(self, tmp_path): + """Regression (review #3415): legacy .bob/commands/ extension commands must + render Bob 1.x ``/speckit.`` refs, not the skills-layout ``/speckit-``. + + The single static AGENT_CONFIGS["bob"]["invoke_separator"] is "-" (the + default skills layout); register_commands must instead resolve the + separator from the project's persisted mode via + BobIntegration.invoke_separator_for_mode(False) -> ".". + """ + import textwrap + from specify_cli.agents import CommandRegistrar + + # Legacy-mode project: .bob/commands exists, ai_skills is NOT set. + commands_dir = tmp_path / ".bob" / "commands" + commands_dir.mkdir(parents=True) + cmd_file = tmp_path / "test.md" + cmd_file.write_text( + textwrap.dedent("""\ + --- + description: "Test command" + --- + See __SPECKIT_COMMAND_SPECIFY__ for details. + """), + encoding="utf-8", + ) + + registrar = CommandRegistrar() + registrar.register_commands_for_all_agents( + commands=[{"name": "speckit.test-cmd", "file": "test.md"}], + source_id="test", + source_dir=tmp_path, + project_root=tmp_path, + ) + rendered = (commands_dir / "speckit.test-cmd.md").read_text(encoding="utf-8") + assert "/speckit.specify" in rendered, ( + "legacy Bob extension commands must render /speckit.specify (dot)" + ) + assert "/speckit-specify" not in rendered diff --git a/tests/test_workflows.py b/tests/test_workflows.py index d60f45d176..2eb56e43e0 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -8882,8 +8882,8 @@ def unlink_boom(self_path, *args, **kwargs): # Original download error remains present. assert "exceedsthe100-byteworkflowsizelimit" in "".join(result.output.split()) # Cleanup failure is reported too, not silently swallowed / crashing. - assert "cleanup denied" in result.output - assert "Warning" in result.output + assert "cleanupdenied" in "".join(result.output.split()) + assert "Warning" in "".join(result.output.split()) assert not WorkflowRegistry(project_dir).is_installed("align-wf") def test_add_from_url_installs(self, project_dir, monkeypatch): From d929bfb5d7f3b872bd0fb37568fa8f1130d6bb00 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Thu, 16 Jul 2026 12:31:26 -0500 Subject: [PATCH 19/27] =?UTF-8?q?fix(bob,copilot):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20preserve=20legacy=20layout,=20dual-mode=20separator?= =?UTF-8?q?s,=20extension-skill=20token=20resolution?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses PR review 4716036212 (3 comments): 1. Bob legacy-install regression: `use`/`switch`/`upgrade` on an existing Bob 1.x project (only `.bob/commands/` on disk, no stored `legacy_commands`) called `is_skills_mode(None)` -> True and rewrote `ai_skills=True`, silently switching extension/command-reference handling to the skills layout. `is_skills_mode` now takes an optional `project_root`; Bob preserves an already-installed legacy layout until an explicit upgrade creates `.bob/skills/`. A fresh project still defaults to skills. 2. Copilot dual-mode separator: `invoke_separator_for_mode` was inherited from the base (mode-independent) and returned Copilot's static `.`, so preset/extension command refs in a Copilot skills project rendered `/speckit.` instead of `/speckit-`. Override it on Copilot to track the persisted `ai_skills` state, consistent with `build_command_invocation` and `effective_invoke_separator`. 3. Bob extension-skill command-ref tokens: verified that merging main's generic `_resolve_command_ref_tokens` (#3544) resolves Bob's tokens via the `CONDITIONAL_SLASH_AGENTS` path (`/speckit-`); added Bob to the command-ref regression parametrize plus dedicated Bob use-path tests. All tests pass (full suite green; merged with current main incl. #3544). 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 --- src/specify_cli/commands/init.py | 4 +- src/specify_cli/integrations/_helpers.py | 2 +- src/specify_cli/integrations/base.py | 18 ++++- src/specify_cli/integrations/bob/__init__.py | 28 ++++++- .../integrations/copilot/__init__.py | 20 ++++- tests/integrations/test_integration_bob.py | 75 +++++++++++++++++++ .../integrations/test_integration_copilot.py | 11 +++ tests/test_extension_skills.py | 1 + 8 files changed, 149 insertions(+), 10 deletions(-) diff --git a/src/specify_cli/commands/init.py b/src/specify_cli/commands/init.py index 88ea5afd5a..bccf28052f 100644 --- a/src/specify_cli/commands/init.py +++ b/src/specify_cli/commands/init.py @@ -533,7 +533,7 @@ def init( "speckit_version": get_speckit_version(), } if resolved_integration.is_skills_mode( - integration_parsed_options or None + integration_parsed_options or None, project_root=project_path ): init_opts["ai_skills"] = True save_init_options(project_path, init_opts) @@ -682,7 +682,7 @@ def init( step_num = 2 _is_skills_integration = resolved_integration.is_skills_mode( - integration_parsed_options or None + integration_parsed_options or None, project_root=project_path ) codex_skill_mode = selected_ai == "codex" and _is_skills_integration diff --git a/src/specify_cli/integrations/_helpers.py b/src/specify_cli/integrations/_helpers.py index bfbbc04448..fae3afd756 100644 --- a/src/specify_cli/integrations/_helpers.py +++ b/src/specify_cli/integrations/_helpers.py @@ -284,7 +284,7 @@ def _update_init_options_for_integration( # opt-out such as Bob override it). This keeps shared code free of # ``isinstance`` / ``_skills_mode`` probing. Passing parsed_options lets it # work on the ``use``/``install`` path where no setup() runs (issue #3550). - if integration.is_skills_mode(parsed_options): + if integration.is_skills_mode(parsed_options, project_root=project_root): opts["ai_skills"] = True else: opts.pop("ai_skills", None) diff --git a/src/specify_cli/integrations/base.py b/src/specify_cli/integrations/base.py index ab8b28c763..44c3e13e90 100644 --- a/src/specify_cli/integrations/base.py +++ b/src/specify_cli/integrations/base.py @@ -191,7 +191,11 @@ def invoke_separator_for_mode(self, skills_enabled: bool) -> str: cfg = self.registrar_config or {} return cfg.get("invoke_separator", self.invoke_separator) - def is_skills_mode(self, parsed_options: dict[str, Any] | None = None) -> bool: + def is_skills_mode( + self, + parsed_options: dict[str, Any] | None = None, + project_root: Path | None = None, + ) -> bool: """Return whether this integration scaffolds skills for these options. This is the single, well-defined hook the shared init/install/upgrade @@ -200,6 +204,12 @@ def is_skills_mode(self, parsed_options: dict[str, Any] | None = None) -> bool: ``getattr(self, "_skills_mode", ...)`` probing so an integration's internal representation never has to leak into shared dispatch code. + *project_root* is optional context for the ``use`` / ``switch`` / + ``upgrade`` path, where no ``setup()`` runs and *parsed_options* may be + empty: dual-mode integrations can consult the already-installed + on-disk layout to avoid silently migrating an existing project to a + different mode. The default ignores it. + The default (command-first integrations, e.g. Copilot's default layout) is skills mode only when ``--skills`` was requested. ``SkillsIntegration`` overrides this to return ``True`` by default; @@ -1413,7 +1423,11 @@ class SkillsIntegration(IntegrationBase): invoke_separator = "-" - def is_skills_mode(self, parsed_options: dict[str, Any] | None = None) -> bool: + def is_skills_mode( + self, + parsed_options: dict[str, Any] | None = None, + project_root: Path | None = None, + ) -> bool: """Skills-native integrations scaffold skills unconditionally.""" return True diff --git a/src/specify_cli/integrations/bob/__init__.py b/src/specify_cli/integrations/bob/__init__.py index 985da67dda..daaca48b75 100644 --- a/src/specify_cli/integrations/bob/__init__.py +++ b/src/specify_cli/integrations/bob/__init__.py @@ -144,9 +144,31 @@ def options(cls) -> list[IntegrationOption]: ), ] - def is_skills_mode(self, parsed_options: dict[str, Any] | None = None) -> bool: - """Bob is skills-first; ``--legacy-commands`` opts out.""" - return not (parsed_options or {}).get("legacy_commands", False) + def is_skills_mode( + self, + parsed_options: dict[str, Any] | None = None, + project_root: Path | None = None, + ) -> bool: + """Bob is skills-first; ``--legacy-commands`` opts out. + + On ``use`` / ``switch`` / ``upgrade`` no ``setup()`` runs and + *parsed_options* is typically empty (existing Bob 1.x installs never + stored ``legacy_commands``). Defaulting to skills there would rewrite + such a project's ``ai_skills`` flag to ``True`` even though it still + only contains ``.bob/commands/`` — silently switching its extension / + command-reference handling to the skills layout. So when a + *project_root* is supplied, an already-installed legacy layout + (``.bob/commands/`` present, ``.bob/skills/`` absent) is preserved + until an explicit upgrade actually creates ``.bob/skills/``. A fresh + project (no ``.bob/`` layout yet) still defaults to skills. + """ + if (parsed_options or {}).get("legacy_commands", False): + return False + if project_root is not None: + bob_dir = Path(project_root) / ".bob" + if (bob_dir / "commands").is_dir() and not (bob_dir / "skills").is_dir(): + return False + return True def effective_invoke_separator( self, parsed_options: dict[str, Any] | None = None diff --git a/src/specify_cli/integrations/copilot/__init__.py b/src/specify_cli/integrations/copilot/__init__.py index 85f5a292bc..822f966019 100644 --- a/src/specify_cli/integrations/copilot/__init__.py +++ b/src/specify_cli/integrations/copilot/__init__.py @@ -131,17 +131,33 @@ def effective_invoke_separator( return "-" return self.invoke_separator - def is_skills_mode(self, parsed_options: dict[str, Any] | None = None) -> bool: + def is_skills_mode( + self, + parsed_options: dict[str, Any] | None = None, + project_root: Path | None = None, + ) -> bool: """Copilot is skills mode when ``--skills`` was requested. On the init path ``setup()`` has already recorded the choice in ``self._skills_mode``; on the ``use``/``install`` path (where no - ``setup()`` runs) the signal comes from *parsed_options* (#3550). + ``setup()`` runs) the signal comes from *parsed_options* (#3550), which + round-trips because ``--skills`` is persisted in the stored options. """ if parsed_options and parsed_options.get("skills"): return True return self._skills_mode + def invoke_separator_for_mode(self, skills_enabled: bool) -> str: + """Skills projects render ``/speckit-``; default markdown ``.``. + + Copilot is dual-layout, so — like Bob — the command-reference + separator depends on the persisted ``ai_skills`` state rather than a + single static value. This keeps preset/extension command refs in a + Copilot skills project consistent with ``build_command_invocation`` + (which emits ``/speckit-``). + """ + return "-" if skills_enabled else self.invoke_separator + @classmethod def options(cls) -> list[IntegrationOption]: return [ diff --git a/tests/integrations/test_integration_bob.py b/tests/integrations/test_integration_bob.py index 92b1c9c40d..e4875a1fb5 100644 --- a/tests/integrations/test_integration_bob.py +++ b/tests/integrations/test_integration_bob.py @@ -84,6 +84,38 @@ def test_legacy_commands_disables_skills(self): bob = get_integration("bob") assert bob.is_skills_mode({"legacy_commands": True}) is False + def test_existing_commands_layout_preserved_on_use(self, tmp_path): + """Regression (review #3415): an existing Bob 1.x project (only + ``.bob/commands/`` on disk, no stored ``legacy_commands``) must NOT be + treated as skills mode when re-resolved with a project_root, so + ``use``/``switch``/``upgrade`` never silently migrate it to skills. + """ + bob = get_integration("bob") + (tmp_path / ".bob" / "commands").mkdir(parents=True) + # No parsed options at all — the pre-existing-install scenario. + assert bob.is_skills_mode(None, project_root=tmp_path) is False + assert bob.is_skills_mode({}, project_root=tmp_path) is False + + def test_existing_skills_layout_stays_skills_on_use(self, tmp_path): + """A ``.bob/skills/`` project resolves to skills mode.""" + bob = get_integration("bob") + (tmp_path / ".bob" / "skills").mkdir(parents=True) + assert bob.is_skills_mode(None, project_root=tmp_path) is True + + def test_fresh_project_defaults_to_skills_with_project_root(self, tmp_path): + """A project with no ``.bob/`` layout yet still defaults to skills.""" + bob = get_integration("bob") + assert bob.is_skills_mode(None, project_root=tmp_path) is True + + def test_explicit_legacy_flag_wins_over_disk_layout(self, tmp_path): + """An explicit ``--legacy-commands`` overrides on-disk detection.""" + bob = get_integration("bob") + (tmp_path / ".bob" / "skills").mkdir(parents=True) + assert ( + bob.is_skills_mode({"legacy_commands": True}, project_root=tmp_path) + is False + ) + def test_effective_invoke_separator_tracks_mode(self): bob = get_integration("bob") assert bob.effective_invoke_separator(None) == "-" @@ -470,3 +502,46 @@ def test_legacy_extension_command_refs_use_dot_separator(self, tmp_path): "legacy Bob extension commands must render /speckit.specify (dot)" ) assert "/speckit-specify" not in rendered + + +class TestBobUseFlowPreservesLegacyLayout: + """Regression (review #3415): re-activating an existing Bob 1.x project + must not silently migrate it to the skills layout. + """ + + def test_update_init_options_preserves_legacy_commands_project(self, tmp_path): + """``use``/``switch``/``upgrade`` on a ``.bob/commands``-only project + (no stored ``legacy_commands``) must not write ``ai_skills=True``. + """ + from specify_cli.integrations._helpers import ( + _update_init_options_for_integration, + ) + from specify_cli import load_init_options + + # Existing Bob 1.x project: legacy commands dir on disk, no ai_skills. + (tmp_path / ".bob" / "commands").mkdir(parents=True) + bob = get_integration("bob") + + # Simulate the use/switch path: no parsed options were stored. + _update_init_options_for_integration(tmp_path, bob, parsed_options=None) + + opts = load_init_options(tmp_path) + assert opts.get("ai") == "bob" + assert opts.get("ai_skills") is not True, ( + "an existing .bob/commands project must stay legacy on re-activation" + ) + + def test_update_init_options_keeps_skills_project_as_skills(self, tmp_path): + """A ``.bob/skills`` project stays skills on re-activation.""" + from specify_cli.integrations._helpers import ( + _update_init_options_for_integration, + ) + from specify_cli import load_init_options + + (tmp_path / ".bob" / "skills").mkdir(parents=True) + bob = get_integration("bob") + + _update_init_options_for_integration(tmp_path, bob, parsed_options=None) + + opts = load_init_options(tmp_path) + assert opts.get("ai_skills") is True diff --git a/tests/integrations/test_integration_copilot.py b/tests/integrations/test_integration_copilot.py index 5b3a5712ad..e9cb33f66d 100644 --- a/tests/integrations/test_integration_copilot.py +++ b/tests/integrations/test_integration_copilot.py @@ -575,6 +575,17 @@ def test_skills_mode_invoke_separator(self): assert copilot.effective_invoke_separator({"skills": True}) == "-" assert copilot.effective_invoke_separator({"skills": False}) == "." + def test_invoke_separator_for_mode_tracks_persisted_state(self): + """Regression (review #3415): registration paths (preset/extension + command refs) must resolve the separator from the persisted ai_skills + state. A Copilot skills project renders ``/speckit-`` (hyphen), + matching ``build_command_invocation``; the default markdown layout + renders ``/speckit.`` (dot). + """ + copilot = self._make_copilot() + assert copilot.invoke_separator_for_mode(True) == "-" + assert copilot.invoke_separator_for_mode(False) == "." + def test_skill_body_has_content(self, tmp_path): """Each SKILL.md body should contain template content.""" copilot = self._make_copilot() diff --git a/tests/test_extension_skills.py b/tests/test_extension_skills.py index dea42a3852..799629b076 100644 --- a/tests/test_extension_skills.py +++ b/tests/test_extension_skills.py @@ -978,6 +978,7 @@ def test_skill_registration_rewrites_extension_subdir_paths(self, project_dir, t ("codex", "$speckit-plan"), ("kimi", "/skill:speckit-plan"), ("zcode", "$speckit-plan"), + ("bob", "/speckit-plan"), ], ) def test_skill_registration_resolves_command_ref_tokens( From 8a162b35ed400650e6745fb3cd50f9f4bf43af16 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Thu, 16 Jul 2026 12:56:00 -0500 Subject: [PATCH 20/27] fix(bob): resolve command-ref separator with project-aware mode before shared-infra refresh (review #3415) The `use`/`switch` paths refresh shared infrastructure via `_with_integration_setting()` / `_invoke_separator_for_integration()`, which previously resolved the invoke separator through `effective_invoke_separator` / `is_skills_mode` WITHOUT a project_root. For a pre-PR Bob 1.x project (.bob/commands/ on disk, no stored options), this defaulted to the skills "-" separator and rewrote rendered shared-template command refs to /speckit-*, even though ai_skills stayed false. Thread project_root through effective_invoke_separator, the two runtime helpers, and every call site so Bob's on-disk legacy detection governs the separator before shared infra is refreshed. Add a rendered-shared-template regression test covering `use --force`. 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 --- src/specify_cli/commands/init.py | 3 +- src/specify_cli/integration_runtime.py | 12 ++- src/specify_cli/integrations/_helpers.py | 4 +- .../integrations/_install_commands.py | 4 +- .../integrations/_migrate_commands.py | 10 ++- src/specify_cli/integrations/base.py | 8 +- src/specify_cli/integrations/bob/__init__.py | 15 +++- .../integrations/copilot/__init__.py | 4 +- tests/integrations/test_integration_bob.py | 77 +++++++++++++++++++ 9 files changed, 120 insertions(+), 17 deletions(-) diff --git a/src/specify_cli/commands/init.py b/src/specify_cli/commands/init.py index bccf28052f..f820d6646f 100644 --- a/src/specify_cli/commands/init.py +++ b/src/specify_cli/commands/init.py @@ -458,6 +458,7 @@ def init( script_type=selected_script, raw_options=integration_options, parsed_options=integration_parsed_options or None, + project_root=project_path, ) _write_integration_json( project_path, @@ -478,7 +479,7 @@ def init( tracker=tracker, force=force, invoke_separator=resolved_integration.effective_invoke_separator( - integration_parsed_options + integration_parsed_options, project_root=project_path ), ) tracker.complete( diff --git a/src/specify_cli/integration_runtime.py b/src/specify_cli/integration_runtime.py index a36dcc672c..9bf8aeac96 100644 --- a/src/specify_cli/integration_runtime.py +++ b/src/specify_cli/integration_runtime.py @@ -46,6 +46,7 @@ def with_integration_setting( script_type: str | None = None, raw_options: str | None = None, parsed_options: dict[str, Any] | None = None, + project_root: Any = None, ) -> dict[str, dict[str, Any]]: """Return integration settings with *key* updated.""" settings = integration_settings(state) @@ -63,7 +64,9 @@ def with_integration_setting( elif raw_options is not None: current.pop("parsed_options", None) - current["invoke_separator"] = integration.effective_invoke_separator(parsed_options) + current["invoke_separator"] = integration.effective_invoke_separator( + parsed_options, project_root + ) settings[key] = current return settings @@ -73,10 +76,11 @@ def invoke_separator_for_integration( state: dict[str, Any], key: str, parsed_options: dict[str, Any] | None = None, + project_root: Any = None, ) -> str: """Resolve the invocation separator for stored/default integration state.""" if parsed_options is not None: - return integration.effective_invoke_separator(parsed_options) + return integration.effective_invoke_separator(parsed_options, project_root) setting = integration_setting(state, key) stored_separator = setting.get("invoke_separator") @@ -85,6 +89,6 @@ def invoke_separator_for_integration( stored_parsed = setting.get("parsed_options") if isinstance(stored_parsed, dict): - return integration.effective_invoke_separator(stored_parsed) + return integration.effective_invoke_separator(stored_parsed, project_root) - return integration.effective_invoke_separator(None) + return integration.effective_invoke_separator(None, project_root) diff --git a/src/specify_cli/integrations/_helpers.py b/src/specify_cli/integrations/_helpers.py index fae3afd756..d27ca7ef4d 100644 --- a/src/specify_cli/integrations/_helpers.py +++ b/src/specify_cli/integrations/_helpers.py @@ -320,6 +320,7 @@ def _set_default_integration( script_type=resolved_script, raw_options=raw_options, parsed_options=parsed_options, + project_root=project_root, ) if refresh_templates: @@ -328,7 +329,8 @@ def _set_default_integration( project_root, resolved_script, invoke_separator=_invoke_separator_for_integration( - integration, {"integration_settings": settings}, key, parsed_options + integration, {"integration_settings": settings}, key, parsed_options, + project_root=project_root, ), force=refresh_templates_force, refresh_managed=True, diff --git a/src/specify_cli/integrations/_install_commands.py b/src/specify_cli/integrations/_install_commands.py index f608a342b4..740d8de708 100644 --- a/src/specify_cli/integrations/_install_commands.py +++ b/src/specify_cli/integrations/_install_commands.py @@ -127,7 +127,8 @@ def integration_install( project_root, selected_script, invoke_separator=_invoke_separator_for_integration( - infra_integration, current, infra_key, infra_parsed + infra_integration, current, infra_key, infra_parsed, + project_root=project_root, ), ) if os.name != "nt": @@ -155,6 +156,7 @@ def integration_install( script_type=selected_script, raw_options=raw_options, parsed_options=parsed_options, + project_root=project_root, ) _write_integration_json(project_root, new_default, new_installed, settings) if new_default == integration.key: diff --git a/src/specify_cli/integrations/_migrate_commands.py b/src/specify_cli/integrations/_migrate_commands.py index 64fb574e08..baea424f04 100644 --- a/src/specify_cli/integrations/_migrate_commands.py +++ b/src/specify_cli/integrations/_migrate_commands.py @@ -236,7 +236,8 @@ def integration_switch( force=refresh_shared_infra, refresh_managed=True, invoke_separator=_invoke_separator_for_integration( - target_integration, current, target, parsed_options + target_integration, current, target, parsed_options, + project_root=project_root, ), refresh_hint=( "To overwrite customizations, re-run with " @@ -415,7 +416,8 @@ def integration_upgrade( selected_script, force=force, invoke_separator=_invoke_separator_for_integration( - infra_integration, current, infra_key, infra_parsed + infra_integration, current, infra_key, infra_parsed, + project_root=project_root, ), ) if os.name != "nt": @@ -441,6 +443,7 @@ def integration_upgrade( script_type=selected_script, raw_options=raw_options, parsed_options=parsed_options, + project_root=project_root, ) if installed_key == key: try: @@ -448,7 +451,8 @@ def integration_upgrade( project_root, selected_script, invoke_separator=_invoke_separator_for_integration( - integration, {"integration_settings": settings}, key, parsed_options + integration, {"integration_settings": settings}, key, parsed_options, + project_root=project_root, ), force=force, refresh_managed=True, diff --git a/src/specify_cli/integrations/base.py b/src/specify_cli/integrations/base.py index 44c3e13e90..3d316cc158 100644 --- a/src/specify_cli/integrations/base.py +++ b/src/specify_cli/integrations/base.py @@ -160,14 +160,16 @@ def options(cls) -> list[IntegrationOption]: return [] def effective_invoke_separator( - self, parsed_options: dict[str, Any] | None = None + self, + parsed_options: dict[str, Any] | None = None, + project_root: Path | None = None, ) -> str: """Return the invoke separator for the given options. Subclasses whose separator depends on runtime options (e.g. Copilot in ``--skills`` mode) should override this method. - The default implementation ignores *parsed_options* and returns - the class-level ``invoke_separator``. + The default implementation ignores *parsed_options* and + *project_root* and returns the class-level ``invoke_separator``. """ return self.invoke_separator diff --git a/src/specify_cli/integrations/bob/__init__.py b/src/specify_cli/integrations/bob/__init__.py index daaca48b75..321130a7cf 100644 --- a/src/specify_cli/integrations/bob/__init__.py +++ b/src/specify_cli/integrations/bob/__init__.py @@ -171,10 +171,19 @@ def is_skills_mode( return True def effective_invoke_separator( - self, parsed_options: dict[str, Any] | None = None + self, + parsed_options: dict[str, Any] | None = None, + project_root: Path | None = None, ) -> str: - """``"."`` for the legacy commands layout, ``"-"`` for skills.""" - return "-" if self.is_skills_mode(parsed_options) else "." + """``"."`` for the legacy commands layout, ``"-"`` for skills. + + *project_root* lets the ``use`` / ``switch`` / ``upgrade`` path — which + refreshes shared infrastructure *before* persisting init-options — + detect an already-installed legacy layout, so core command references + are rendered with the correct separator instead of defaulting to the + skills ``-``. + """ + return "-" if self.is_skills_mode(parsed_options, project_root) else "." def invoke_separator_for_mode(self, skills_enabled: bool) -> str: """Resolve the command-ref separator from a project's persisted mode. diff --git a/src/specify_cli/integrations/copilot/__init__.py b/src/specify_cli/integrations/copilot/__init__.py index 822f966019..ce41c00d6f 100644 --- a/src/specify_cli/integrations/copilot/__init__.py +++ b/src/specify_cli/integrations/copilot/__init__.py @@ -122,7 +122,9 @@ class CopilotIntegration(IntegrationBase): _skills_mode: bool = False def effective_invoke_separator( - self, parsed_options: dict[str, Any] | None = None + self, + parsed_options: dict[str, Any] | None = None, + project_root: Path | None = None, ) -> str: """Return ``"-"`` when skills mode is requested, ``"."`` otherwise.""" if parsed_options and parsed_options.get("skills"): diff --git a/tests/integrations/test_integration_bob.py b/tests/integrations/test_integration_bob.py index e4875a1fb5..2a559fe036 100644 --- a/tests/integrations/test_integration_bob.py +++ b/tests/integrations/test_integration_bob.py @@ -545,3 +545,80 @@ def test_update_init_options_keeps_skills_project_as_skills(self, tmp_path): opts = load_init_options(tmp_path) assert opts.get("ai_skills") is True + + def test_with_integration_setting_stores_dot_separator_for_legacy(self, tmp_path): + """Regression (review #3415): shared-infra refresh on the use/switch + path resolves the command-ref separator *before* init-options are + rewritten, via ``effective_invoke_separator``. For an existing + ``.bob/commands`` project with no stored options this must resolve to + ``"."`` (project-aware), not the skills-layout ``"-"``; otherwise core + command references get rewritten to ``/speckit-*``. + """ + from specify_cli.integration_runtime import with_integration_setting + + (tmp_path / ".bob" / "commands").mkdir(parents=True) + bob = get_integration("bob") + + # Simulate the use/switch path: no parsed options stored. + settings = with_integration_setting( + {}, "bob", bob, parsed_options=None, project_root=tmp_path + ) + assert settings["bob"]["invoke_separator"] == ".", ( + "legacy .bob/commands project must persist the dot separator so " + "shared templates render Bob 1.x /speckit. references" + ) + + def test_use_force_keeps_legacy_command_refs_in_shared_templates(self, tmp_path): + """End-to-end (review #3415): ``integration use bob --force`` on an + existing Bob 1.x project (legacy layout on disk, stored options + stripped as a pre-PR install would be) must re-render shared templates + with ``/speckit.`` (dot), not ``/speckit-``. + """ + import json + from typer.testing import CliRunner + from specify_cli import app + + # Create a real legacy Bob project (renders shared templates). + target = tmp_path / "proj" + runner = CliRunner() + result = runner.invoke(app, [ + "init", str(target), "--integration", "bob", + "--integration-options", "--legacy-commands", + "--ignore-agent-tools", "--script", "sh", + ]) + assert result.exit_code == 0, f"init failed: {result.output}" + + template = target / ".specify" / "templates" / "plan-template.md" + assert template.is_file(), "expected a rendered shared plan template" + assert "/speckit.plan" in template.read_text(encoding="utf-8") + + # Simulate a pre-PR Bob 1.x install: no stored options/separator. + integ_json = target / ".specify" / "integration.json" + data = json.loads(integ_json.read_text(encoding="utf-8")) + bob_settings = data["integration_settings"]["bob"] + for stale in ("raw_options", "parsed_options", "invoke_separator"): + bob_settings.pop(stale, None) + integ_json.write_text(json.dumps(data, indent=2), encoding="utf-8") + + # Re-activate with --force so shared templates are re-rendered. + import os + old_cwd = os.getcwd() + try: + os.chdir(target) + result = runner.invoke( + app, ["integration", "use", "bob", "--force"] + ) + finally: + os.chdir(old_cwd) + assert result.exit_code == 0, f"use failed: {result.output}" + + rendered = template.read_text(encoding="utf-8") + assert "/speckit.plan" in rendered, ( + "legacy Bob project must keep /speckit.plan (dot) after refresh" + ) + assert "/speckit-plan" not in rendered, ( + "shared templates must not be rewritten to the skills /speckit-plan" + ) + # And the persisted separator must reflect the legacy layout. + data = json.loads(integ_json.read_text(encoding="utf-8")) + assert data["integration_settings"]["bob"].get("invoke_separator") == "." From eeaf85f3156908a2fa6e1f05fbf33d94be22a69a Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 17 Jul 2026 08:43:18 -0500 Subject: [PATCH 21/27] fix(bob): scope persisted ai_skills flag to active agent when resolving command-ref separator (review #3415) `register_commands` runs once per detected agent, but the persisted `ai_skills` flag describes only the active integration (`opts["ai"]`). When another agent (e.g. Copilot) is active in skills mode while a legacy `.bob/commands` layout is also present, the previous code passed that global `True` to Bob's `invoke_separator_for_mode`, rewriting Bob 1.x command refs to `/speckit-*` instead of `/speckit.*`. Only consult the persisted flag for the agent it describes (`opts["ai"] == agent_name`); otherwise resolve the separator from the agent's own project-aware `effective_invoke_separator(None, project_root)`. Add regression tests covering the mismatched-active-agent case and a control for Bob-active skills mode. 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 --- src/specify_cli/agents.py | 18 +++++- tests/integrations/test_integration_bob.py | 72 ++++++++++++++++++++++ 2 files changed, 87 insertions(+), 3 deletions(-) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index e5e91d7f21..63a7fd5dbf 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -648,9 +648,21 @@ def register_commands( _integ = get_integration(agent_name) if _integ is not None: - _sep = _integ.invoke_separator_for_mode( - is_ai_skills_enabled(load_init_options(project_root)) - ) + _opts = load_init_options(project_root) + # The persisted ``ai_skills`` flag describes only the active + # integration (``opts["ai"]``). ``register_commands_for_all_agents`` + # calls this for every detected agent, so trusting that flag for a + # different agent would, e.g., render a legacy ``.bob/commands`` + # project's refs as ``/speckit-*`` just because Copilot is active in + # skills mode. Only consult the flag for the agent it describes; + # otherwise resolve this agent's separator from its own project-aware + # detection. + if _opts.get("ai") == agent_name: + _sep = _integ.invoke_separator_for_mode( + is_ai_skills_enabled(_opts) + ) + else: + _sep = _integ.effective_invoke_separator(None, project_root) except Exception: pass diff --git a/tests/integrations/test_integration_bob.py b/tests/integrations/test_integration_bob.py index 2a559fe036..26e4ee711b 100644 --- a/tests/integrations/test_integration_bob.py +++ b/tests/integrations/test_integration_bob.py @@ -622,3 +622,75 @@ def test_use_force_keeps_legacy_command_refs_in_shared_templates(self, tmp_path) # And the persisted separator must reflect the legacy layout. data = json.loads(integ_json.read_text(encoding="utf-8")) assert data["integration_settings"]["bob"].get("invoke_separator") == "." + + +class TestBobCommandRefScopedToActiveAgent: + """Regression (review #3415, 4716424313). + + ``CommandRegistrar.register_commands`` runs once per detected agent, but the + persisted ``ai_skills`` flag describes only the *active* integration + (``opts["ai"]``). When another agent (e.g. Copilot) is active in skills + mode while a legacy ``.bob/commands`` layout is also present, Bob's command + references must still render with the ``.`` separator (Bob 1.x + ``/speckit.``) rather than inheriting Copilot's ``ai_skills=True`` and + rendering ``/speckit-``. + """ + + def _write_command_ref_ext(self, source_dir): + source_dir.mkdir(parents=True, exist_ok=True) + cmd = source_dir / "run.md" + cmd.write_text( + "---\ndescription: Run\n---\n\nUse __SPECKIT_COMMAND_PLAN__ first.\n", + encoding="utf-8", + ) + return [{"name": "speckit.ext.run", "file": "run.md"}] + + def test_legacy_bob_ref_not_rewritten_when_other_agent_active_in_skills( + self, tmp_path + ): + from specify_cli._init_options import save_init_options + from specify_cli.agents import CommandRegistrar + + # Legacy Bob layout on disk; skills layout absent. + (tmp_path / ".bob" / "commands").mkdir(parents=True) + # A different agent (Copilot) is the active integration, in skills mode. + save_init_options(tmp_path, {"ai": "copilot", "ai_skills": True}) + + source_dir = tmp_path / "ext-src" + commands = self._write_command_ref_ext(source_dir) + + registrar = CommandRegistrar() + registered = registrar.register_commands( + "bob", commands, "ext", source_dir, tmp_path, + ) + assert "speckit.ext.run" in registered + + written = list((tmp_path / ".bob" / "commands").glob("*.md")) + assert written, "expected a rendered Bob command file" + content = written[0].read_text(encoding="utf-8") + assert "__SPECKIT_COMMAND_PLAN__" not in content + assert "/speckit.plan" in content, ( + "legacy Bob command refs must use the dot separator even when " + "another agent is active in skills mode" + ) + assert "/speckit-plan" not in content + + def test_active_bob_skills_still_uses_hyphen(self, tmp_path): + """Control: when Bob itself is the active skills agent, refs use ``-``.""" + from specify_cli._init_options import save_init_options + from specify_cli.agents import CommandRegistrar + + (tmp_path / ".bob" / "skills").mkdir(parents=True) + save_init_options(tmp_path, {"ai": "bob", "ai_skills": True}) + + source_dir = tmp_path / "ext-src" + commands = self._write_command_ref_ext(source_dir) + + registrar = CommandRegistrar() + registrar.register_commands("bob", commands, "ext", source_dir, tmp_path) + + written = list((tmp_path / ".bob" / "commands").glob("*.md")) + assert written, "expected a rendered Bob command file" + content = written[0].read_text(encoding="utf-8") + assert "/speckit-plan" in content + assert "/speckit.plan" not in content From 21922f8cf19747c024b751f1ddc133999651dca0 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 17 Jul 2026 09:25:10 -0500 Subject: [PATCH 22/27] fix(bob): detect Spec Kit layout from managed artifacts, not any skills dir (review #3415) Two related mis-detections from review 4723246468: 1. `BobIntegration.is_skills_mode` treated the mere presence of a `.bob/skills/` directory as proof the project is skills-based. A legacy Spec Kit install (managed `.bob/commands/speckit.*.md`) that also carried unrelated Bob 2 skills would be misclassified as skills, so `integration use bob` persisted `ai_skills` and rewrote shared refs. Now the layout is inferred from managed Spec Kit artifacts: legacy/command mode only when managed `speckit.*.md` command files exist and no managed `speckit-*` skill dirs do. 2. The `register_commands` separator for an inactive agent used a disk-based `effective_invoke_separator(None, project_root)` fallback that could pick the skills separator even though the registrar writes the static command layout (`.bob/commands/*.md`). Inactive agents now resolve the separator from the registrar's actual output layout (`extension == "/SKILL.md"`), so command-layout files keep `/speckit.*` refs regardless of sibling dirs. Update the affected hook/E2E tests to use managed artifacts and add regression tests for the mixed-layout and inactive-registrar scenarios. 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 --- src/specify_cli/agents.py | 16 +++- src/specify_cli/integrations/bob/__init__.py | 21 +++-- tests/integrations/test_integration_bob.py | 92 +++++++++++++++++--- 3 files changed, 109 insertions(+), 20 deletions(-) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 63a7fd5dbf..3dc2ba9274 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -662,7 +662,21 @@ def register_commands( is_ai_skills_enabled(_opts) ) else: - _sep = _integ.effective_invoke_separator(None, project_root) + # Inactive agent: the reference separator must match the + # layout THIS registrar writes into — determined by the + # agent's static output config (its command dir + file + # extension), not by unrelated sibling directories on disk. + # A skill-scaffold output ("/SKILL.md") uses the skills + # separator; a command-layout output uses the command + # separator. This avoids mislabeling a Bob command-layout + # write as skills just because an unrelated .bob/skills + # directory happens to exist. + registrar_writes_skills = ( + agent_config.get("extension") == "/SKILL.md" + ) + _sep = _integ.invoke_separator_for_mode( + registrar_writes_skills + ) except Exception: pass diff --git a/src/specify_cli/integrations/bob/__init__.py b/src/specify_cli/integrations/bob/__init__.py index 321130a7cf..904a01cb97 100644 --- a/src/specify_cli/integrations/bob/__init__.py +++ b/src/specify_cli/integrations/bob/__init__.py @@ -155,18 +155,25 @@ def is_skills_mode( *parsed_options* is typically empty (existing Bob 1.x installs never stored ``legacy_commands``). Defaulting to skills there would rewrite such a project's ``ai_skills`` flag to ``True`` even though it still - only contains ``.bob/commands/`` — silently switching its extension / - command-reference handling to the skills layout. So when a - *project_root* is supplied, an already-installed legacy layout - (``.bob/commands/`` present, ``.bob/skills/`` absent) is preserved - until an explicit upgrade actually creates ``.bob/skills/``. A fresh - project (no ``.bob/`` layout yet) still defaults to skills. + only contains a command layout — silently switching its extension / + command-reference handling to the skills layout. + + So when a *project_root* is supplied, the Spec Kit layout is inferred + from **managed Spec Kit artifacts**, not from the mere presence of a + ``.bob/skills/`` directory: a user may keep unrelated Bob 2 skills in + ``.bob/skills/`` while their Spec Kit commands still live in + ``.bob/commands/speckit.*.md``. We therefore treat the project as + legacy (command) mode only when managed Spec Kit command files exist + and no managed Spec Kit skills (``speckit-*`` skill dirs) do. A fresh + project (no managed artifacts yet) still defaults to skills. """ if (parsed_options or {}).get("legacy_commands", False): return False if project_root is not None: bob_dir = Path(project_root) / ".bob" - if (bob_dir / "commands").is_dir() and not (bob_dir / "skills").is_dir(): + has_managed_skills = any((bob_dir / "skills").glob("speckit-*")) + has_managed_commands = any((bob_dir / "commands").glob("speckit.*.md")) + if has_managed_commands and not has_managed_skills: return False return True diff --git a/tests/integrations/test_integration_bob.py b/tests/integrations/test_integration_bob.py index 26e4ee711b..3f8acdb29a 100644 --- a/tests/integrations/test_integration_bob.py +++ b/tests/integrations/test_integration_bob.py @@ -85,32 +85,63 @@ def test_legacy_commands_disables_skills(self): assert bob.is_skills_mode({"legacy_commands": True}) is False def test_existing_commands_layout_preserved_on_use(self, tmp_path): - """Regression (review #3415): an existing Bob 1.x project (only - ``.bob/commands/`` on disk, no stored ``legacy_commands``) must NOT be - treated as skills mode when re-resolved with a project_root, so - ``use``/``switch``/``upgrade`` never silently migrate it to skills. + """Regression (review #3415): an existing Bob 1.x project (managed + ``.bob/commands/speckit.*.md`` on disk, no stored ``legacy_commands``) + must NOT be treated as skills mode when re-resolved with a + project_root, so ``use``/``switch``/``upgrade`` never silently migrate + it to skills. """ bob = get_integration("bob") - (tmp_path / ".bob" / "commands").mkdir(parents=True) + cmds = tmp_path / ".bob" / "commands" + cmds.mkdir(parents=True) + (cmds / "speckit.plan.md").write_text("# plan", encoding="utf-8") # No parsed options at all — the pre-existing-install scenario. assert bob.is_skills_mode(None, project_root=tmp_path) is False assert bob.is_skills_mode({}, project_root=tmp_path) is False def test_existing_skills_layout_stays_skills_on_use(self, tmp_path): - """A ``.bob/skills/`` project resolves to skills mode.""" + """A project with managed ``speckit-*`` skills resolves to skills mode.""" bob = get_integration("bob") - (tmp_path / ".bob" / "skills").mkdir(parents=True) + (tmp_path / ".bob" / "skills" / "speckit-plan").mkdir(parents=True) + assert bob.is_skills_mode(None, project_root=tmp_path) is True + + def test_managed_commands_with_unrelated_skills_dir_stays_legacy( + self, tmp_path + ): + """Regression (review #3415, 4723246468): a legacy Spec Kit install + (managed ``.bob/commands/speckit.*.md``) that *also* carries unrelated + Bob 2 skills (a ``.bob/skills/`` dir with no managed ``speckit-*`` + skills) must stay in command mode — the mere presence of a skills + directory is not evidence that Spec Kit is skills-based. + """ + bob = get_integration("bob") + cmds = tmp_path / ".bob" / "commands" + cmds.mkdir(parents=True) + (cmds / "speckit.plan.md").write_text("# plan", encoding="utf-8") + # An unrelated (non-Spec-Kit) skill the user authored. + (tmp_path / ".bob" / "skills" / "my-own-skill").mkdir(parents=True) + assert bob.is_skills_mode(None, project_root=tmp_path) is False + assert bob.effective_invoke_separator(None, project_root=tmp_path) == "." + + def test_managed_skills_win_when_both_layouts_present(self, tmp_path): + """When managed Spec Kit skills exist, skills mode wins even if a stale + managed command file is still on disk (upgrade leftover).""" + bob = get_integration("bob") + cmds = tmp_path / ".bob" / "commands" + cmds.mkdir(parents=True) + (cmds / "speckit.plan.md").write_text("# plan", encoding="utf-8") + (tmp_path / ".bob" / "skills" / "speckit-plan").mkdir(parents=True) assert bob.is_skills_mode(None, project_root=tmp_path) is True def test_fresh_project_defaults_to_skills_with_project_root(self, tmp_path): - """A project with no ``.bob/`` layout yet still defaults to skills.""" + """A project with no managed ``.bob/`` artifacts yet defaults to skills.""" bob = get_integration("bob") assert bob.is_skills_mode(None, project_root=tmp_path) is True def test_explicit_legacy_flag_wins_over_disk_layout(self, tmp_path): """An explicit ``--legacy-commands`` overrides on-disk detection.""" bob = get_integration("bob") - (tmp_path / ".bob" / "skills").mkdir(parents=True) + (tmp_path / ".bob" / "skills" / "speckit-plan").mkdir(parents=True) assert ( bob.is_skills_mode({"legacy_commands": True}, project_root=tmp_path) is False @@ -519,7 +550,9 @@ def test_update_init_options_preserves_legacy_commands_project(self, tmp_path): from specify_cli import load_init_options # Existing Bob 1.x project: legacy commands dir on disk, no ai_skills. - (tmp_path / ".bob" / "commands").mkdir(parents=True) + cmds = tmp_path / ".bob" / "commands" + cmds.mkdir(parents=True) + (cmds / "speckit.plan.md").write_text("# plan", encoding="utf-8") bob = get_integration("bob") # Simulate the use/switch path: no parsed options were stored. @@ -538,7 +571,7 @@ def test_update_init_options_keeps_skills_project_as_skills(self, tmp_path): ) from specify_cli import load_init_options - (tmp_path / ".bob" / "skills").mkdir(parents=True) + (tmp_path / ".bob" / "skills" / "speckit-plan").mkdir(parents=True) bob = get_integration("bob") _update_init_options_for_integration(tmp_path, bob, parsed_options=None) @@ -556,7 +589,9 @@ def test_with_integration_setting_stores_dot_separator_for_legacy(self, tmp_path """ from specify_cli.integration_runtime import with_integration_setting - (tmp_path / ".bob" / "commands").mkdir(parents=True) + cmds = tmp_path / ".bob" / "commands" + cmds.mkdir(parents=True) + (cmds / "speckit.plan.md").write_text("# plan", encoding="utf-8") bob = get_integration("bob") # Simulate the use/switch path: no parsed options stored. @@ -694,3 +729,36 @@ def test_active_bob_skills_still_uses_hyphen(self, tmp_path): content = written[0].read_text(encoding="utf-8") assert "/speckit-plan" in content assert "/speckit.plan" not in content + + def test_inactive_bob_command_output_uses_dot_even_with_skills_dir( + self, tmp_path + ): + """Regression (review #3415, 4723246468): for an inactive Bob install + the registrar's separator must match the layout it is actually writing + (``.bob/commands/*.md`` — command layout), not on-disk sibling dirs. + Even when a ``.bob/skills/`` directory (with managed ``speckit-*`` + skills) coexists, command-layout files must keep ``/speckit.``. + """ + from specify_cli._init_options import save_init_options + from specify_cli.agents import CommandRegistrar + + # Both layouts on disk; the active agent is something else entirely. + (tmp_path / ".bob" / "commands").mkdir(parents=True) + (tmp_path / ".bob" / "skills" / "speckit-plan").mkdir(parents=True) + save_init_options(tmp_path, {"ai": "claude", "ai_skills": True}) + + source_dir = tmp_path / "ext-src" + commands = self._write_command_ref_ext(source_dir) + + registrar = CommandRegistrar() + registrar.register_commands("bob", commands, "ext", source_dir, tmp_path) + + written = list((tmp_path / ".bob" / "commands").glob("*.md")) + assert written, "expected a rendered Bob command file" + content = written[0].read_text(encoding="utf-8") + assert "__SPECKIT_COMMAND_PLAN__" not in content + assert "/speckit.plan" in content, ( + "Bob command-layout output must use the dot separator regardless " + "of a coexisting .bob/skills directory" + ) + assert "/speckit-plan" not in content From 13db5f39efd57c66461bc72b9da10c3911269d65 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 17 Jul 2026 10:42:55 -0500 Subject: [PATCH 23/27] fix(bob): apply managed-artifact detection on upgrade + consistent skill post-processing (review #3415) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two issues from review 4723782860: 1. `BobIntegration.setup()` resolved the layout via `is_skills_mode(parsed_options)` WITHOUT `project_root`, so `integration upgrade bob` on a Bob 1.x install (managed `.bob/commands/speckit.*.md`, no stored options) ignored the existing command files, generated skills, and stale-deleted the legacy commands — silently migrating the project. Pass `project_root` so the same managed-artifact detection used by `use` also governs upgrades. 2. Only `_BobSkillsHelper` overrode `post_process_skill_content` to suppress the shared slash-command hook note. Preset/extension skill generators call that hook on the registered `BobIntegration`, which inherited `IntegrationBase`'s note-injecting default. Repeat the no-op (delegating to the skills helper) on the registered class so every Bob skill-generation path is consistent with intent-activated core Bob skills. Add regression tests for the upgrade-preservation and post-processing paths. 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 --- src/specify_cli/integrations/bob/__init__.py | 13 +++- tests/integrations/test_integration_bob.py | 80 ++++++++++++++++++++ 2 files changed, 92 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/integrations/bob/__init__.py b/src/specify_cli/integrations/bob/__init__.py index 904a01cb97..6ba2efd365 100644 --- a/src/specify_cli/integrations/bob/__init__.py +++ b/src/specify_cli/integrations/bob/__init__.py @@ -203,6 +203,17 @@ def invoke_separator_for_mode(self, skills_enabled: bool) -> str: """ return "-" if skills_enabled else "." + def post_process_skill_content(self, content: str) -> str: + """Bob skills are intent-activated; no slash-command note is injected. + + Preset/extension skill generators call this on the *registered* + ``BobIntegration`` instance, not on :class:`_BobSkillsHelper`, so the + no-op must be repeated here (delegating to the helper) — otherwise + those paths would inherit ``IntegrationBase``'s default and inject + ``/speckit-*`` hook guidance that core Bob skills intentionally omit. + """ + return _BobSkillsHelper().post_process_skill_content(content) + def setup( self, project_root: Path, @@ -211,7 +222,7 @@ def setup( **opts: Any, ) -> list[Path]: parsed_options = parsed_options or {} - if self.is_skills_mode(parsed_options): + if self.is_skills_mode(parsed_options, project_root): return _BobSkillsHelper().setup( project_root, manifest, parsed_options, **opts ) diff --git a/tests/integrations/test_integration_bob.py b/tests/integrations/test_integration_bob.py index 3f8acdb29a..1a709bc966 100644 --- a/tests/integrations/test_integration_bob.py +++ b/tests/integrations/test_integration_bob.py @@ -762,3 +762,83 @@ def test_inactive_bob_command_output_uses_dot_even_with_skills_dir( "of a coexisting .bob/skills directory" ) assert "/speckit-plan" not in content + + +class TestBobSetupPreservesLegacyOnUpgrade: + """Regression (review #3415, 4723782860, comment 1). + + ``setup()`` must apply the same managed-artifact detection as ``use`` so + that ``integration upgrade bob`` on a Bob 1.x install (managed + ``.bob/commands/speckit.*.md`` on disk, no stored options) preserves the + command layout instead of silently generating skills and stale-deleting + the legacy commands. + """ + + def test_setup_without_options_preserves_existing_command_layout( + self, tmp_path + ): + from specify_cli.integrations.bob import BobIntegration + + # Pre-existing Bob 1.x install: managed command files, no options. + cmds = tmp_path / ".bob" / "commands" + cmds.mkdir(parents=True) + (cmds / "speckit.plan.md").write_text("# plan", encoding="utf-8") + + bob = BobIntegration() + m = IntegrationManifest("bob", tmp_path) + with pytest.warns(UserWarning, match="Bob legacy commands mode"): + created = bob.setup(tmp_path, m, parsed_options=None) + + # Command layout regenerated; no skills layout introduced. + assert not (tmp_path / ".bob" / "skills").exists(), ( + "upgrade must not migrate an existing legacy Bob project to skills" + ) + assert created, "expected command files to be regenerated" + for f in created: + assert f.parent == tmp_path / ".bob" / "commands" + assert f.suffix == ".md" + + def test_setup_fresh_project_still_defaults_to_skills(self, tmp_path): + """A fresh project (no managed artifacts) still defaults to skills.""" + from specify_cli.integrations.bob import BobIntegration + + bob = BobIntegration() + m = IntegrationManifest("bob", tmp_path) + created = bob.setup(tmp_path, m, parsed_options=None) + + assert (tmp_path / ".bob" / "skills").is_dir() + assert not (tmp_path / ".bob" / "commands").exists() + assert created + + +class TestBobPostProcessSkillContent: + """Regression (review #3415, 4723782860, comment 2). + + Preset/extension skill generators call ``post_process_skill_content`` on + the *registered* ``BobIntegration`` instance. Core Bob skills are + intent-activated and intentionally omit the shared slash-command hook note, + so the registered class must expose the same no-op the skills helper does + (not inherit a note-injecting default) to keep every skill path consistent. + """ + + def test_registered_bob_has_post_process_hook(self): + bob = get_integration("bob") + assert hasattr(bob, "post_process_skill_content") + + def test_post_process_is_noop_no_hook_note_injected(self): + bob = get_integration("bob") + sample = ( + "---\nname: speckit-plan\n---\n\n" + "Run /speckit.plan then /speckit.tasks.\n" + ) + assert bob.post_process_skill_content(sample) == sample + + def test_post_process_matches_skills_helper(self): + from specify_cli.integrations.bob import _BobSkillsHelper + + bob = get_integration("bob") + sample = "---\nname: speckit-analyze\n---\n\nSome body with /speckit.plan.\n" + assert ( + bob.post_process_skill_content(sample) + == _BobSkillsHelper().post_process_skill_content(sample) + ) From ba6655c1e55c10865c8c2a71ffe0c013f27d9922 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 17 Jul 2026 11:36:48 -0500 Subject: [PATCH 24/27] feat(bob): add --skills migration opt-in; fix separator + manifest loss (review #3415) Address review #3415 (4724160183): - Comment 1: Add an explicit `--skills` opt-in to BobIntegration. It forces the skills layout over on-disk auto-detection, giving legacy Bob 1.x installs a supported migration path (`integration upgrade bob --integration-options="--skills"`). `--skills` and `--legacy-commands` are mutually exclusive (clean exit-1 error). - Comment 2: In CommandRegistrar.register_commands, derive the command-ref separator from the output layout (agent_config["extension"]) for the active agent too, not the persisted ai_skills flag. A command-layout file (.bob/commands/*.md, .github/agents/*.agent.md) always renders /speckit.*; only a /SKILL.md scaffold uses /speckit-*. Dual-layout agents (Bob, Copilot) write skills via their own setup()/skills path, so register_commands only ever emits their command-layout files. - Comment 3: Update docs/reference/integrations.md Bob entry to document the skills-based default (.bob/skills/), the deprecated --legacy-commands opt-out, and the --skills migration path. Also fix a latent manifest-loss bug surfaced by the migration path: the upgrade Phase 2 stale-file cleanup built a throwaway manifest sharing the integration key and called uninstall(), which always deleted {key}.manifest.json. Any layout-shrinking upgrade (e.g. legacy->skills) thus wiped the freshly-saved manifest, leaving the project untracked and un-upgradeable. uninstall() now takes remove_manifest (default True); the stale-cleanup pass passes False. Adds regression tests for the --skills opt-in, mutual exclusion, corrected active-agent separator, remove_manifest=False, and an end-to-end legacy->skills migration that verifies the manifest survives and the project remains upgradeable. Full suite: 4555 passed, 5 skipped. 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 --- docs/reference/integrations.md | 2 +- src/specify_cli/agents.py | 56 +++++------ .../integrations/_migrate_commands.py | 10 +- src/specify_cli/integrations/bob/__init__.py | 73 +++++++++++--- src/specify_cli/integrations/manifest.py | 12 ++- tests/integrations/test_integration_bob.py | 97 +++++++++++++++++-- .../test_integration_subcommand.py | 53 ++++++++++ tests/integrations/test_manifest.py | 22 +++++ 8 files changed, 266 insertions(+), 59 deletions(-) diff --git a/docs/reference/integrations.md b/docs/reference/integrations.md index 72044ec684..67596f6b58 100644 --- 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-` | | [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` | IDE-based agent | +| [IBM Bob](https://www.ibm.com/products/bob) | `bob` | Skills-based integration by default; installs skills as `speckit-/SKILL.md` under `.bob/skills/` and invokes them as `/speckit-`. 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. | | [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/agents.py b/src/specify_cli/agents.py index 3dc2ba9274..b46464348e 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -637,46 +637,34 @@ def register_commands( is_cline_ext = agent_name == "cline" and source_id != "core" source_root = source_dir.resolve() - # Resolve the command-reference separator once for this agent/project. - # Dual-layout agents (e.g. Bob) use different separators for their - # skills vs command layouts, so we ask the integration to map the - # project's persisted skills state to the correct separator rather than - # relying on the single static AGENT_CONFIGS value. + # Resolve the command-reference separator for the file THIS registrar + # is about to write. The separator must match the *output layout* the + # registrar produces for this agent — not the project's persisted + # ``ai_skills`` flag, and not unrelated sibling directories on disk. A + # skill scaffold ("/SKILL.md") uses the skills separator; any + # command-layout output (".md", ".agent.md", ".toml", …) uses the + # command separator. + # + # This holds for the *active* agent too. Dual-layout agents (Bob, + # Copilot) write their skills via their own setup()/skills path, so + # ``register_commands`` only ever emits their command-layout files. + # Deriving the separator from ``ai_skills`` would render such a + # ``.bob/commands/*.md`` (or ``.github/agents/*.agent.md``) file with + # ``/speckit-*`` whenever that agent is active in skills mode — even + # though a command-layout file must use ``/speckit.*``. Deriving it + # from the agent's static output config avoids that mismatch and stays + # correct when a stale ``.bob/skills`` directory coexists with + # ``.bob/commands``. _sep = agent_config.get("invoke_separator", ".") try: from specify_cli.integrations import get_integration # noqa: PLC0415 _integ = get_integration(agent_name) if _integ is not None: - _opts = load_init_options(project_root) - # The persisted ``ai_skills`` flag describes only the active - # integration (``opts["ai"]``). ``register_commands_for_all_agents`` - # calls this for every detected agent, so trusting that flag for a - # different agent would, e.g., render a legacy ``.bob/commands`` - # project's refs as ``/speckit-*`` just because Copilot is active in - # skills mode. Only consult the flag for the agent it describes; - # otherwise resolve this agent's separator from its own project-aware - # detection. - if _opts.get("ai") == agent_name: - _sep = _integ.invoke_separator_for_mode( - is_ai_skills_enabled(_opts) - ) - else: - # Inactive agent: the reference separator must match the - # layout THIS registrar writes into — determined by the - # agent's static output config (its command dir + file - # extension), not by unrelated sibling directories on disk. - # A skill-scaffold output ("/SKILL.md") uses the skills - # separator; a command-layout output uses the command - # separator. This avoids mislabeling a Bob command-layout - # write as skills just because an unrelated .bob/skills - # directory happens to exist. - registrar_writes_skills = ( - agent_config.get("extension") == "/SKILL.md" - ) - _sep = _integ.invoke_separator_for_mode( - registrar_writes_skills - ) + registrar_writes_skills = ( + agent_config.get("extension") == "/SKILL.md" + ) + _sep = _integ.invoke_separator_for_mode(registrar_writes_skills) except Exception: pass diff --git a/src/specify_cli/integrations/_migrate_commands.py b/src/specify_cli/integrations/_migrate_commands.py index baea424f04..6518db8fdc 100644 --- a/src/specify_cli/integrations/_migrate_commands.py +++ b/src/specify_cli/integrations/_migrate_commands.py @@ -496,7 +496,15 @@ def integration_upgrade( if stale_keys: stale_manifest = IntegrationManifest(key, project_root, version="stale-cleanup") stale_manifest._files = {k: old_files[k] for k in stale_keys} - stale_removed, _ = stale_manifest.uninstall(project_root, force=True) + # remove_manifest=False: this throwaway manifest shares ``key`` with the + # real one just saved above (new_manifest.save()). Letting uninstall() + # delete ``{key}.manifest.json`` would wipe the freshly-written manifest + # whenever an upgrade shrinks the tracked file set (e.g. Bob migrating + # from the legacy commands layout to skills), leaving the integration + # untracked and un-upgradeable. + stale_removed, _ = stale_manifest.uninstall( + project_root, force=True, remove_manifest=False + ) if stale_removed: console.print(f" Removed {len(stale_removed)} stale file(s) from previous install") diff --git a/src/specify_cli/integrations/bob/__init__.py b/src/specify_cli/integrations/bob/__init__.py index 6ba2efd365..f0c85f3b45 100644 --- a/src/specify_cli/integrations/bob/__init__.py +++ b/src/specify_cli/integrations/bob/__init__.py @@ -23,6 +23,8 @@ from pathlib import Path from typing import Any +import typer + from ..base import ( IntegrationBase, IntegrationOption, @@ -32,6 +34,24 @@ from ..manifest import IntegrationManifest +def _validate_mode_options(parsed_options: dict[str, Any] | None) -> None: + """Reject ``--skills`` and ``--legacy-commands`` used together. + + The two flags select opposite layouts, so combining them is ambiguous. + Fail fast with the same clean exit-1 UX as other bad-option paths rather + than silently letting one win. + """ + opts = parsed_options or {} + if opts.get("skills") and opts.get("legacy_commands"): + from ..._console import console + + console.print( + "[red]Error:[/red] --skills and --legacy-commands are mutually " + "exclusive; pass only one." + ) + raise typer.Exit(1) + + def _warn_legacy_commands_deprecated() -> None: warnings.warn( "Bob legacy commands mode (.bob/commands/) is deprecated and will be " @@ -132,6 +152,17 @@ class BobIntegration(IntegrationBase): @classmethod def options(cls) -> list[IntegrationOption]: return [ + IntegrationOption( + "--skills", + is_flag=True, + default=False, + help=( + "Force the default skills layout (.bob/skills/), overriding " + "on-disk auto-detection. Use this to migrate a legacy " + "commands install to skills, e.g. " + "`integration upgrade bob --integration-options \"--skills\"`" + ), + ), IntegrationOption( "--legacy-commands", is_flag=True, @@ -151,23 +182,39 @@ def is_skills_mode( ) -> bool: """Bob is skills-first; ``--legacy-commands`` opts out. - On ``use`` / ``switch`` / ``upgrade`` no ``setup()`` runs and - *parsed_options* is typically empty (existing Bob 1.x installs never - stored ``legacy_commands``). Defaulting to skills there would rewrite + Precedence: + + 1. Explicit ``--skills`` wins — it *forces* skills mode regardless of + what is already on disk. This is the supported migration / opt-in + path: ``integration upgrade bob --integration-options "--skills"`` + converts a legacy commands install to the skills layout (setup() + scaffolds ``.bob/skills`` and the upgrade's stale-file pass removes + the old ``.bob/commands`` files). + 2. Explicit ``--legacy-commands`` opts out to the Bob 1.x layout. + 3. Otherwise, when a *project_root* is supplied, the layout is inferred + from **managed Spec Kit artifacts** (see below). + 4. A fresh project (no managed artifacts, no flags) defaults to skills. + + The disk-detection fallback exists because on ``use`` / ``switch`` / + ``upgrade`` (without ``--skills``) no ``setup()`` runs and + *parsed_options* is typically empty — existing Bob 1.x installs never + stored ``legacy_commands``. Defaulting to skills there would rewrite such a project's ``ai_skills`` flag to ``True`` even though it still - only contains a command layout — silently switching its extension / - command-reference handling to the skills layout. - - So when a *project_root* is supplied, the Spec Kit layout is inferred - from **managed Spec Kit artifacts**, not from the mere presence of a - ``.bob/skills/`` directory: a user may keep unrelated Bob 2 skills in - ``.bob/skills/`` while their Spec Kit commands still live in + only contains a command layout, silently switching its extension / + command-reference handling. So the layout is inferred from managed + Spec Kit artifacts, not the mere presence of a ``.bob/skills/`` + directory: a user may keep unrelated Bob 2 skills in ``.bob/skills/`` + while their Spec Kit commands still live in ``.bob/commands/speckit.*.md``. We therefore treat the project as legacy (command) mode only when managed Spec Kit command files exist - and no managed Spec Kit skills (``speckit-*`` skill dirs) do. A fresh - project (no managed artifacts yet) still defaults to skills. + and no managed Spec Kit skills (``speckit-*`` skill dirs) do. Passing + ``--skills`` overrides this so users are never trapped in legacy mode. """ - if (parsed_options or {}).get("legacy_commands", False): + opts = parsed_options or {} + _validate_mode_options(opts) + if opts.get("skills", False): + return True + if opts.get("legacy_commands", False): return False if project_root is not None: bob_dir = Path(project_root) / ".bob" diff --git a/src/specify_cli/integrations/manifest.py b/src/specify_cli/integrations/manifest.py index 8c98243c9a..a318b990cb 100644 --- a/src/specify_cli/integrations/manifest.py +++ b/src/specify_cli/integrations/manifest.py @@ -327,12 +327,18 @@ def uninstall( project_root: Path | None = None, *, force: bool = False, + remove_manifest: bool = True, ) -> tuple[list[Path], list[Path]]: """Remove tracked files whose hash still matches. Parameters: - project_root: Override for the project root. - force: If ``True``, remove files even if modified. + project_root: Override for the project root. + force: If ``True``, remove files even if modified. + remove_manifest: If ``True`` (default), also delete this + integration's ``{key}.manifest.json``. Set ``False`` for + *partial* cleanups (e.g. the upgrade stale-file pass, which + builds a throwaway manifest over a subset of files) so the + real, freshly-saved manifest for the same key is not destroyed. Returns: ``(removed, skipped)`` — absolute paths. @@ -393,7 +399,7 @@ def uninstall( # Remove the manifest file itself manifest = root / ".specify" / "integrations" / f"{self.key}.manifest.json" - if manifest.exists(): + if remove_manifest and manifest.exists(): manifest.unlink() parent = manifest.parent while parent != root: diff --git a/tests/integrations/test_integration_bob.py b/tests/integrations/test_integration_bob.py index 1a709bc966..52a25ae2c6 100644 --- a/tests/integrations/test_integration_bob.py +++ b/tests/integrations/test_integration_bob.py @@ -64,12 +64,20 @@ def test_options_include_legacy_commands_flag(self): # Legacy must be OPT-IN (default=False) — skills are the default assert opt.default is False - def test_no_skills_flag(self): - """The old --skills flag must be gone; it has been replaced by the default.""" + def test_options_include_skills_migration_flag(self): + """Review #3415, 4724160183, comment 1: a ``--skills`` opt-in exists as + the supported migration path from legacy commands to the skills layout. + It is distinct from the pre-skills-default ``--skills`` flag: here it + *forces* skills mode over on-disk auto-detection. + """ bob = get_integration("bob") opts = bob.options() skills_opts = [o for o in opts if o.name == "--skills"] - assert len(skills_opts) == 0 + assert len(skills_opts) == 1 + opt = skills_opts[0] + assert opt.is_flag is True + # Opt-in: disk auto-detection remains the default behavior. + assert opt.default is False class TestBobIsSkillsModeHook: @@ -147,10 +155,40 @@ def test_explicit_legacy_flag_wins_over_disk_layout(self, tmp_path): is False ) + def test_explicit_skills_flag_forces_skills_over_legacy_disk_layout( + self, tmp_path + ): + """Regression (review #3415, 4724160183, comment 1). + + ``--skills`` is the supported migration / opt-in: it must force skills + mode even when a managed legacy ``.bob/commands`` layout is on disk + (which otherwise auto-detects to legacy). This gives + ``integration upgrade bob --integration-options="--skills"`` a path out + of legacy mode instead of being trapped by disk detection. + """ + bob = get_integration("bob") + cmds = tmp_path / ".bob" / "commands" + cmds.mkdir(parents=True) + (cmds / "speckit.plan.md").write_text("# plan", encoding="utf-8") + assert bob.is_skills_mode({"skills": True}, project_root=tmp_path) is True + assert ( + bob.effective_invoke_separator({"skills": True}, project_root=tmp_path) + == "-" + ) + + def test_skills_and_legacy_flags_are_mutually_exclusive(self): + """Passing both ``--skills`` and ``--legacy-commands`` exits cleanly.""" + import typer + + bob = get_integration("bob") + with pytest.raises(typer.Exit): + bob.is_skills_mode({"skills": True, "legacy_commands": True}) + def test_effective_invoke_separator_tracks_mode(self): bob = get_integration("bob") assert bob.effective_invoke_separator(None) == "-" assert bob.effective_invoke_separator({"legacy_commands": True}) == "." + assert bob.effective_invoke_separator({"skills": True}) == "-" def test_invoke_separator_for_mode_tracks_persisted_state(self): """Registration paths resolve the separator from persisted ai_skills.""" @@ -710,8 +748,19 @@ def test_legacy_bob_ref_not_rewritten_when_other_agent_active_in_skills( ) assert "/speckit-plan" not in content - def test_active_bob_skills_still_uses_hyphen(self, tmp_path): - """Control: when Bob itself is the active skills agent, refs use ``-``.""" + def test_active_bob_skills_command_output_uses_dot(self, tmp_path): + """Regression (review #3415, 4724160183, comment 2). + + The separator must match the *output layout* the registrar writes, not + the project's persisted ``ai_skills`` flag. Even when Bob itself is the + active agent in skills mode, a ``.bob/commands/*.md`` file is a + command-layout artifact and must render Bob 1.x ``/speckit.``. + Rendering ``/speckit-`` into a command file (as the old + ``ai_skills``-driven active-agent branch did) produced an invocation the + command layout can't resolve. Bob skills are written via its own + skills path, so ``register_commands`` only ever emits command-layout + files for Bob. + """ from specify_cli._init_options import save_init_options from specify_cli.agents import CommandRegistrar @@ -727,8 +776,12 @@ def test_active_bob_skills_still_uses_hyphen(self, tmp_path): written = list((tmp_path / ".bob" / "commands").glob("*.md")) assert written, "expected a rendered Bob command file" content = written[0].read_text(encoding="utf-8") - assert "/speckit-plan" in content - assert "/speckit.plan" not in content + assert "__SPECKIT_COMMAND_PLAN__" not in content + assert "/speckit.plan" in content, ( + "a .bob/commands/*.md command-layout file must use the dot " + "separator even when Bob is the active agent in skills mode" + ) + assert "/speckit-plan" not in content def test_inactive_bob_command_output_uses_dot_even_with_skills_dir( self, tmp_path @@ -810,6 +863,36 @@ def test_setup_fresh_project_still_defaults_to_skills(self, tmp_path): assert not (tmp_path / ".bob" / "commands").exists() assert created + def test_setup_with_skills_flag_migrates_legacy_to_skills(self, tmp_path): + """Review #3415, 4724160183, comment 1: ``--skills`` on an existing + legacy install forces the skills layout (the migration opt-in), instead + of preserving the auto-detected legacy layout. ``setup()`` scaffolds the + skills layout; the ``integration upgrade`` stale-file pass removes the + old command files. + """ + from specify_cli.integrations.bob import BobIntegration + + # Pre-existing Bob 1.x install on disk. + cmds = tmp_path / ".bob" / "commands" + cmds.mkdir(parents=True) + (cmds / "speckit.plan.md").write_text("# plan", encoding="utf-8") + + bob = BobIntegration() + m = IntegrationManifest("bob", tmp_path) + # No deprecation warning — the user opted into skills, not legacy. + with warnings.catch_warnings(): + warnings.simplefilter("error", UserWarning) + created = bob.setup(tmp_path, m, parsed_options={"skills": True}) + + assert (tmp_path / ".bob" / "skills").is_dir(), ( + "--skills must force the skills layout even when a legacy commands " + "layout is already on disk" + ) + assert created + for f in created: + assert f.name == "SKILL.md" + assert f.parent.name.startswith("speckit-") + class TestBobPostProcessSkillContent: """Regression (review #3415, 4723782860, comment 2). diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index a6a3807498..7c189e23d9 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -2477,6 +2477,59 @@ def test_upgrade_migrates_opencode_legacy_dir(self, tmp_path): f"found: {[f.name for f in core_remaining]}" ) + def test_upgrade_bob_skills_migration_preserves_manifest(self, tmp_path): + """Regression (review #3415, 4724160183, comment 1). + + ``integration upgrade bob --integration-options="--skills"`` migrates a + legacy Bob 1.x install (``.bob/commands/*.md``) to the skills layout + (``.bob/skills/speckit-*/SKILL.md``) and stale-removes the old command + files. Because that stale-file pass shrinks the tracked set, the + upgrade's Phase 2 must NOT delete the freshly-saved ``bob.manifest.json`` + — otherwise the migrated project is left untracked and un-upgradeable. + """ + project = _init_project( + tmp_path, "bob", integration_options="--legacy-commands" + ) + + commands = project / ".bob" / "commands" + skills = project / ".bob" / "skills" + manifest_path = ( + project / ".specify" / "integrations" / "bob.manifest.json" + ) + assert commands.is_dir() and sorted(commands.glob("speckit.*.md")) + assert not skills.exists() + assert manifest_path.is_file() + + result = _run_in_project(project, [ + "integration", "upgrade", "bob", + "--integration-options", "--skills", + "--script", "sh", "--force", + ]) + assert result.exit_code == 0, f"migration upgrade failed: {result.output}" + + # Skills layout scaffolded; legacy core command files removed. + assert skills.is_dir(), ".bob/skills/ must exist after --skills migration" + assert sorted(skills.glob("speckit-*")), "expected migrated skill dirs" + core_commands = [ + f for f in commands.glob("speckit.*.md") + if "agent-context" not in f.name + ] if commands.exists() else [] + assert core_commands == [], ( + f"legacy core command files should be removed, found: " + f"{[f.name for f in core_commands]}" + ) + + # The manifest must survive so the project stays tracked/upgradeable. + assert manifest_path.is_file(), ( + "bob.manifest.json must survive a layout-shrinking migration" + ) + reupgrade = _run_in_project(project, [ + "integration", "upgrade", "bob", "--script", "sh", "--force", + ]) + assert reupgrade.exit_code == 0, ( + f"migrated project must remain upgradeable: {reupgrade.output}" + ) + def test_upgrade_preserves_existing_vscode_settings(self, tmp_path): """Regression: copilot upgrade must not stale-delete .vscode/settings.json. diff --git a/tests/integrations/test_manifest.py b/tests/integrations/test_manifest.py index 06c1fd398d..32b769769a 100644 --- a/tests/integrations/test_manifest.py +++ b/tests/integrations/test_manifest.py @@ -220,6 +220,28 @@ def test_removes_manifest_file(self, tmp_path): m.uninstall() assert not m.manifest_path.exists() + def test_remove_manifest_false_preserves_manifest_file(self, tmp_path): + """Regression (review #3415, 4724160183): a partial cleanup must not + delete ``{key}.manifest.json``. + + The upgrade stale-file pass builds a throwaway manifest sharing the + integration's key over a subset of files and uninstalls it. With + ``remove_manifest=False`` the tracked files are still removed but the + real, freshly-saved manifest for that key survives — otherwise a + layout-shrinking upgrade (e.g. Bob migrating legacy commands → skills) + would leave the integration untracked and un-upgradeable. + """ + m = IntegrationManifest("test", tmp_path, version="1.0") + m.record_file("f.txt", "content") + m.save() + assert m.manifest_path.exists() + removed, skipped = m.uninstall(remove_manifest=False) + assert len(removed) == 1 + assert not (tmp_path / "f.txt").exists() + assert m.manifest_path.exists(), ( + "remove_manifest=False must keep the manifest file on disk" + ) + def test_cleans_empty_parent_dirs(self, tmp_path): m = IntegrationManifest("test", tmp_path) m.record_file("a/b/c/f.txt", "content") From d3408e6ff828827cad5ddd64679f76ebfed97d11 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 17 Jul 2026 14:50:14 -0500 Subject: [PATCH 25/27] docs(agents): align token-resolution comment with output-layout separator rule (review #3415) Address review #3415 (4725516805). The comment above resolve_command_refs still described the removed state-based behavior ("resolve it from the integration using the project's persisted skills state"). Update it to describe the output-layout rule that register_commands now uses: _sep is derived from the layout this registrar writes (a /SKILL.md scaffold uses the skills separator; a command-layout file uses the command separator), not the persisted ai_skills state. Comment-only change; no behavior change. 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 --- src/specify_cli/agents.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index b46464348e..9c9a17e2b9 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -742,9 +742,12 @@ def register_commands( # Resolve __SPECKIT_COMMAND_*__ tokens using the agent's invoke separator. # For dual-layout agents (e.g. Bob) the separator differs between the # skills and command layouts, so a single static AGENT_CONFIGS value is - # insufficient. Resolve it from the integration using the project's - # persisted skills state; single-layout agents fall back to the static - # AGENT_CONFIGS value unchanged (invoke_separator_for_mode default). + # insufficient. ``_sep`` (resolved above) is derived from the *output + # layout* this registrar writes — a "/SKILL.md" scaffold uses the skills + # separator, any command-layout file uses the command separator — not + # the project's persisted ai_skills state. Single-layout agents fall back + # to the static AGENT_CONFIGS value unchanged (invoke_separator_for_mode + # default). # Deferred import of IntegrationBase avoids a circular import at module load # (base.py itself imports CommandRegistrar lazily). from specify_cli.integrations.base import IntegrationBase # noqa: PLC0415 From a3dd14173f38d9172fc94c15e4911213e0c2fbba Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 17 Jul 2026 16:07:20 -0500 Subject: [PATCH 26/27] fix(bob): reconcile extension artifacts on layout change (review #3415) When a dual-mode agent (Bob) flips between the legacy commands layout and the skills layout during `integration upgrade` (via `--skills` / `--legacy-commands`), the old layout's extension command/skill files were left orphaned: Phase 2 stale cleanup only removes files tracked by the *integration* manifest, while extension artifacts are tracked in the extension registry. Detect the layout flip by comparing whether the old vs new manifest tracks a `/SKILL.md` scaffold, and when it changed, unregister the agent's extension artifacts before the existing re-registration so they are recreated in the new layout (and the per-agent registry is updated). Preset artifacts are documented as a known, pre-existing cross-cutting gap: no agent-scoped preset re-registration exists in use/switch/upgrade for any agent, so reconciling them is out of scope for this Bob migration. 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 --- .../integrations/_migrate_commands.py | 46 +++++++++++ .../test_integration_subcommand.py | 76 +++++++++++++++++++ 2 files changed, 122 insertions(+) diff --git a/src/specify_cli/integrations/_migrate_commands.py b/src/specify_cli/integrations/_migrate_commands.py index 6518db8fdc..fbf1b9fe59 100644 --- a/src/specify_cli/integrations/_migrate_commands.py +++ b/src/specify_cli/integrations/_migrate_commands.py @@ -40,6 +40,19 @@ ) +def _manifest_tracks_skill_layout(manifest) -> bool: + """Return True when *manifest* tracks any skills-layout artifact. + + A skill scaffold is written as ``.../speckit-/SKILL.md``, so a + manifest whose tracked files include a ``/SKILL.md`` key is in the skills + layout; otherwise it is in the command layout. Used by ``upgrade`` to + detect a dual-mode agent (e.g. Bob) flipping between the legacy commands + layout and the skills layout so orphaned extension artifacts from the old + layout can be reconciled. + """ + return any(str(rel).endswith("/SKILL.md") for rel in manifest.files) + + @integration_app.command("switch") def integration_switch( target: str = typer.Argument(help="Integration key to switch to"), @@ -514,6 +527,39 @@ def integration_upgrade( # Done after the upgrade has fully settled (Phase 2 included) and outside # the try/except above so this best-effort step cannot affect upgrade # success. + # + # Layout-change reconciliation: a dual-mode agent (e.g. Bob) can flip + # between the legacy commands layout and the skills layout across an + # upgrade (``upgrade bob --integration-options "--skills"`` / reverse + # ``--legacy-commands``). Phase 2 above only removes stale files tracked by + # the *integration* manifest (core commands); extension artifacts are + # tracked separately in the extension registry, so the old layout's + # extension command/skill files would otherwise linger as orphans. When the + # layout actually changed, first unregister the agent's extension artifacts + # (removing old-layout files and clearing per-agent registry entries) so the + # re-registration below recreates them in the new layout. ``upgrade``s that + # don't change layout skip this to avoid needless remove/re-add churn. + # + # Known limitation: preset command/skill artifacts are NOT reconciled on a + # 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. + if _manifest_tracks_skill_layout(old_manifest) != _manifest_tracks_skill_layout( + new_manifest + ): + _unregister_extensions_for_agent( + project_root, + key, + continuing=( + "The integration layout changed, but old-layout extension " + "artifacts may need manual cleanup." + ), + ) _register_extensions_for_agent( project_root, key, diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index 7c189e23d9..d07e29dbc7 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -2530,6 +2530,82 @@ def test_upgrade_bob_skills_migration_preserves_manifest(self, tmp_path): f"migrated project must remain upgradeable: {reupgrade.output}" ) + def test_upgrade_bob_layout_change_reconciles_extension_artifacts(self, tmp_path): + """Regression (review #3415, 4725829110). + + When a dual-mode agent (Bob) flips layout across an upgrade, the old + layout's *extension* artifacts must be reconciled — not left orphaned. + A legacy Bob install renders enabled extensions as ``.bob/commands/`` + command files; migrating to skills via ``--skills`` must remove those + command files, recreate the extension as ``.bob/skills/`` skills, and + update the extension registry accordingly (and vice-versa for the + reverse ``--legacy-commands`` migration). + """ + project = _init_project( + tmp_path, "bob", integration_options="--legacy-commands" + ) + + result = _run_in_project(project, ["extension", "add", "git"]) + assert result.exit_code == 0, f"extension add failed: {result.output}" + + commands = project / ".bob" / "commands" + skills = project / ".bob" / "skills" + registry_path = project / ".specify" / "extensions" / ".registry" + + def _git_registry(): + data = json.loads(registry_path.read_text(encoding="utf-8")) + g = data["extensions"]["git"] + return list(g.get("registered_commands", {})), g.get( + "registered_skills", [] + ) + + # Legacy precondition: git renders as command files under .bob/commands. + assert sorted(commands.glob("speckit.git.*.md")), ( + "legacy Bob should render the git extension as command files" + ) + assert not list(skills.glob("speckit-git-*")) if skills.exists() else True + cmds_agents, skill_names = _git_registry() + assert "bob" in cmds_agents and not skill_names + + # Migrate legacy -> skills. + result = _run_in_project(project, [ + "integration", "upgrade", "bob", + "--integration-options", "--skills", + "--script", "sh", "--force", + ]) + assert result.exit_code == 0, f"--skills migration failed: {result.output}" + + # Old-layout git command files removed; skills recreated. + assert not sorted(commands.glob("speckit.git.*.md")), ( + "git extension command files must be removed after --skills migration" + ) + assert sorted(skills.glob("speckit-git-*")), ( + "git extension must be recreated as skills after --skills migration" + ) + cmds_agents, skill_names = _git_registry() + assert "bob" not in cmds_agents, ( + "extension registry must drop the stale bob command entry" + ) + assert skill_names, "extension registry must record the migrated skills" + + # Migrate skills -> legacy: the reverse reconciliation must also hold. + result = _run_in_project(project, [ + "integration", "upgrade", "bob", + "--integration-options", "--legacy-commands", + "--script", "sh", "--force", + ]) + assert result.exit_code == 0, ( + f"--legacy-commands migration failed: {result.output}" + ) + assert not sorted(skills.glob("speckit-git-*")), ( + "git extension skills must be removed after --legacy-commands migration" + ) + assert sorted(commands.glob("speckit.git.*.md")), ( + "git extension command files must be recreated in legacy layout" + ) + cmds_agents, skill_names = _git_registry() + assert "bob" in cmds_agents and not skill_names + def test_upgrade_preserves_existing_vscode_settings(self, tmp_path): """Regression: copilot upgrade must not stale-delete .vscode/settings.json. From d2321b2afcd60023ff28b92ef8145e01d5043f4e Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 17 Jul 2026 16:33:39 -0500 Subject: [PATCH 27/27] fix(bob): reject layout migration when preset overrides are installed (review #3415) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf --- docs/reference/integrations.md | 2 +- .../integrations/_migrate_commands.py | 83 +++++++++++++++++-- .../test_integration_subcommand.py | 56 +++++++++++++ 3 files changed, 135 insertions(+), 6 deletions(-) diff --git a/docs/reference/integrations.md b/docs/reference/integrations.md index f460d24806..62dcc5c083 100644 --- 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-` | | [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-/SKILL.md` under `.bob/skills/` and invokes them as `/speckit-`. 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-/SKILL.md` under `.bob/skills/` and invokes them as `/speckit-`. 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 index fbf1b9fe59..e9330c69b0 100644 --- 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 [/cyan]\n" + f" [cyan]specify integration upgrade {key} " + f"--integration-options \"...\"[/cyan]\n" + f" [cyan]specify preset add [/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 index d07e29dbc7..672e9c10da 100644 --- 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.