Skip to content

Commit 5a8cb61

Browse files
mnriemCopilot
andcommitted
refactor(bob): resolve skills mode via base-class hooks + fix command-ref separators
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-<cmd> instead of Bob 1.x /speckit.<cmd>. 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
1 parent cad76d3 commit 5a8cb61

11 files changed

Lines changed: 298 additions & 151 deletions

File tree

src/specify_cli/agents.py

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -637,6 +637,23 @@ def register_commands(
637637
is_cline_ext = agent_name == "cline" and source_id != "core"
638638
source_root = source_dir.resolve()
639639

640+
# Resolve the command-reference separator once for this agent/project.
641+
# Dual-layout agents (e.g. Bob) use different separators for their
642+
# skills vs command layouts, so we ask the integration to map the
643+
# project's persisted skills state to the correct separator rather than
644+
# relying on the single static AGENT_CONFIGS value.
645+
_sep = agent_config.get("invoke_separator", ".")
646+
try:
647+
from specify_cli.integrations import get_integration # noqa: PLC0415
648+
649+
_integ = get_integration(agent_name)
650+
if _integ is not None:
651+
_sep = _integ.invoke_separator_for_mode(
652+
is_ai_skills_enabled(load_init_options(project_root))
653+
)
654+
except Exception:
655+
pass
656+
640657
for cmd_info in commands:
641658
cmd_name = cmd_info["name"]
642659
aliases = cmd_info.get("aliases", [])
@@ -709,13 +726,15 @@ def register_commands(
709726
)
710727

711728
# Resolve __SPECKIT_COMMAND_*__ tokens using the agent's invoke separator.
712-
# The separator is sourced from agent_config (populated by _build_agent_configs,
713-
# which propagates each integration's invoke_separator class attribute).
729+
# For dual-layout agents (e.g. Bob) the separator differs between the
730+
# skills and command layouts, so a single static AGENT_CONFIGS value is
731+
# insufficient. Resolve it from the integration using the project's
732+
# persisted skills state; single-layout agents fall back to the static
733+
# AGENT_CONFIGS value unchanged (invoke_separator_for_mode default).
714734
# Deferred import of IntegrationBase avoids a circular import at module load
715735
# (base.py itself imports CommandRegistrar lazily).
716736
from specify_cli.integrations.base import IntegrationBase # noqa: PLC0415
717737

718-
_sep = agent_config.get("invoke_separator", ".")
719738
body = IntegrationBase.resolve_command_refs(body, _sep)
720739

721740
output_name = self._compute_output_name(agent_name, cmd_name, agent_config)

src/specify_cli/commands/init.py

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -532,18 +532,9 @@ def init(
532532
"feature_numbering": "sequential",
533533
"speckit_version": get_speckit_version(),
534534
}
535-
from ..integrations.base import SkillsIntegration as _SkillsPersist
536-
537-
_legacy_commands = bool(
538-
(integration_parsed_options or {}).get("legacy_commands")
539-
)
540-
_skills_mode_attr = getattr(resolved_integration, "_skills_mode", None)
541-
_is_skills = (
542-
isinstance(resolved_integration, _SkillsPersist)
543-
or (callable(_skills_mode_attr) and _skills_mode_attr(integration_parsed_options))
544-
or (not callable(_skills_mode_attr) and bool(_skills_mode_attr))
545-
)
546-
if _is_skills and not _legacy_commands:
535+
if resolved_integration.is_skills_mode(
536+
integration_parsed_options or None
537+
):
547538
init_opts["ai_skills"] = True
548539
save_init_options(project_path, init_opts)
549540

@@ -690,13 +681,8 @@ def init(
690681
steps_lines.append("1. You're already in the project directory!")
691682
step_num = 2
692683

693-
from ..integrations.base import SkillsIntegration as _SkillsInt
694-
695-
_skills_mode_attr = getattr(resolved_integration, "_skills_mode", None)
696-
_is_skills_integration = (
697-
isinstance(resolved_integration, _SkillsInt)
698-
or (callable(_skills_mode_attr) and _skills_mode_attr(integration_parsed_options))
699-
or (not callable(_skills_mode_attr) and bool(_skills_mode_attr))
684+
_is_skills_integration = resolved_integration.is_skills_mode(
685+
integration_parsed_options or None
700686
)
701687

702688
codex_skill_mode = selected_ai == "codex" and _is_skills_integration
@@ -705,7 +691,6 @@ def init(
705691
kimi_skill_mode = selected_ai == "kimi"
706692
agy_skill_mode = selected_ai == "agy" and _is_skills_integration
707693
trae_skill_mode = selected_ai == "trae"
708-
bob_skill_mode = selected_ai == "bob" and _is_skills_integration
709694
cursor_agent_skill_mode = (
710695
selected_ai == "cursor-agent" and _is_skills_integration
711696
)
@@ -714,19 +699,20 @@ def init(
714699
zed_skill_mode = selected_ai == "zed" and _is_skills_integration
715700
grok_skill_mode = selected_ai == "grok" and _is_skills_integration
716701
cline_skill_mode = selected_ai == "cline"
702+
bob_skill_mode = selected_ai == "bob" and _is_skills_integration
717703
native_skill_mode = (
718704
codex_skill_mode
719705
or zcode_skill_mode
720706
or claude_skill_mode
721707
or kimi_skill_mode
722708
or agy_skill_mode
723709
or trae_skill_mode
724-
or bob_skill_mode
725710
or cursor_agent_skill_mode
726711
or copilot_skill_mode
727712
or devin_skill_mode
728713
or zed_skill_mode
729714
or grok_skill_mode
715+
or bob_skill_mode
730716
)
731717

732718
if codex_skill_mode:
@@ -764,6 +750,11 @@ def init(
764750
f"{step_num}. Start Grok Build in this project directory; spec-kit skills were installed to [cyan].grok/skills[/cyan]"
765751
)
766752
step_num += 1
753+
if bob_skill_mode:
754+
steps_lines.append(
755+
f"{step_num}. Start Bob in this project directory; spec-kit skills were installed to [cyan].bob/skills[/cyan]"
756+
)
757+
step_num += 1
767758
usage_label = "skills" if native_skill_mode else "slash commands"
768759

769760
from .._invocation_style import (

src/specify_cli/integrations/_helpers.py

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -272,28 +272,19 @@ def _update_init_options_for_integration(
272272
load_init_options,
273273
save_init_options,
274274
)
275-
from .base import SkillsIntegration
276275
opts = load_init_options(project_root)
277276
opts["integration"] = integration.key
278277
opts["ai"] = integration.key
279278
opts["speckit_version"] = _get_speckit_version()
280279
if script_type:
281280
opts["script"] = script_type
282-
# Skills mode is either intrinsic (SkillsIntegration), derived from
283-
# parsed_options via a callable _skills_mode method (e.g. Bob), set on
284-
# the instance during setup() as a bool attribute (e.g. Copilot), or
285-
# requested via parsed options (e.g. Copilot's --skills, persisted as
286-
# parsed_options["skills"]). The latter is the only signal available on
287-
# the `use` path, where no setup() runs and a fresh integration instance
288-
# has _skills_mode == False (issue #3550).
289-
_skills_mode_attr = getattr(integration, "_skills_mode", None)
290-
skills_mode = (
291-
isinstance(integration, SkillsIntegration)
292-
or (callable(_skills_mode_attr) and _skills_mode_attr(parsed_options))
293-
or (not callable(_skills_mode_attr) and bool(_skills_mode_attr))
294-
or bool((parsed_options or {}).get("skills"))
295-
)
296-
if skills_mode:
281+
# Whether skills mode is active is owned by each integration via the
282+
# ``is_skills_mode`` hook (base default honors ``--skills``;
283+
# SkillsIntegration returns True; skills-first integrations with a legacy
284+
# opt-out such as Bob override it). This keeps shared code free of
285+
# ``isinstance`` / ``_skills_mode`` probing. Passing parsed_options lets it
286+
# work on the ``use``/``install`` path where no setup() runs (issue #3550).
287+
if integration.is_skills_mode(parsed_options):
297288
opts["ai_skills"] = True
298289
else:
299290
opts.pop("ai_skills", None)

src/specify_cli/integrations/_install_commands.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,10 @@ def integration_install(
159159
_write_integration_json(project_root, new_default, new_installed, settings)
160160
if new_default == integration.key:
161161
_update_init_options_for_integration(
162-
project_root, integration, script_type=selected_script, parsed_options=parsed_options
162+
project_root,
163+
integration,
164+
script_type=selected_script,
165+
parsed_options=parsed_options,
163166
)
164167
else:
165168
_refresh_init_options_speckit_version(project_root)

src/specify_cli/integrations/_migrate_commands.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,10 @@ def integration_upgrade(
464464
_write_integration_json(project_root, installed_key, installed_keys, settings)
465465
if installed_key == key:
466466
_update_init_options_for_integration(
467-
project_root, integration, script_type=selected_script, parsed_options=parsed_options
467+
project_root,
468+
integration,
469+
script_type=selected_script,
470+
parsed_options=parsed_options,
468471
)
469472
else:
470473
_refresh_init_options_speckit_version(project_root)

src/specify_cli/integrations/base.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,43 @@ def effective_invoke_separator(
171171
"""
172172
return self.invoke_separator
173173

174+
def invoke_separator_for_mode(self, skills_enabled: bool) -> str:
175+
"""Command-ref separator given the project's *resolved* skills state.
176+
177+
Registration paths (extension / preset command rendering) have no CLI
178+
``parsed_options`` — only the persisted ``ai_skills`` flag — so they
179+
resolve the command-reference separator through this hook rather than
180+
the static ``AGENT_CONFIGS[key]["invoke_separator"]`` value, which
181+
cannot represent an agent whose separator differs between its skills
182+
and command layouts.
183+
184+
The default is mode-independent and returns exactly what
185+
``_build_agent_configs`` would place in ``AGENT_CONFIGS`` (the
186+
``registrar_config`` override if present, else the class-level
187+
``invoke_separator``), so single-layout agents are unaffected.
188+
Dual-mode agents whose separator depends on the layout (e.g. Bob:
189+
``-`` for skills, ``.`` for legacy commands) override this.
190+
"""
191+
cfg = self.registrar_config or {}
192+
return cfg.get("invoke_separator", self.invoke_separator)
193+
194+
def is_skills_mode(self, parsed_options: dict[str, Any] | None = None) -> bool:
195+
"""Return whether this integration scaffolds skills for these options.
196+
197+
This is the single, well-defined hook the shared init/install/upgrade
198+
machinery consults to decide whether to persist ``ai_skills=True`` and
199+
render skill invocations. It replaces ad-hoc ``isinstance`` /
200+
``getattr(self, "_skills_mode", ...)`` probing so an integration's
201+
internal representation never has to leak into shared dispatch code.
202+
203+
The default (command-first integrations, e.g. Copilot's default
204+
layout) is skills mode only when ``--skills`` was requested.
205+
``SkillsIntegration`` overrides this to return ``True`` by default;
206+
skills-first integrations that expose a legacy opt-out (e.g. Bob)
207+
override it to honor their own flag.
208+
"""
209+
return bool((parsed_options or {}).get("skills"))
210+
174211
def build_exec_args(
175212
self,
176213
prompt: str,
@@ -1376,6 +1413,10 @@ class SkillsIntegration(IntegrationBase):
13761413

13771414
invoke_separator = "-"
13781415

1416+
def is_skills_mode(self, parsed_options: dict[str, Any] | None = None) -> bool:
1417+
"""Skills-native integrations scaffold skills unconditionally."""
1418+
return True
1419+
13791420
def build_exec_args(
13801421
self,
13811422
prompt: str,

0 commit comments

Comments
 (0)