feat: update Bob integration to skills-based layout for Bob 2.0#3415
Conversation
Bob 2.0 replaces the command-based workflow (.bob/commands/*.md) with a skills-based layout (.bob/skills/speckit-<name>/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)
|
@mnriem please review, we need to make this work with new Bob... Thankyou so much |
There was a problem hiding this comment.
Pull request overview
This PR updates the built-in IBM Bob integration to align with Bob 2.0’s skills-based layout, switching installation output from command files to speckit-<name>/SKILL.md skills directories and bumping the integration’s catalog version accordingly.
Changes:
- Migrate
BobIntegrationfromMarkdownIntegrationtoSkillsIntegrationand update output paths to.bob/skills/.../SKILL.md. - Update Bob integration tests to use the shared
SkillsIntegrationTestsmixin. - Bump the Bob entry in
integrations/catalog.jsonto2.0.0with an updated description.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/integrations/bob/__init__.py |
Switch Bob to SkillsIntegration and update registrar/config output to .bob/skills + /SKILL.md. |
tests/integrations/test_integration_bob.py |
Update base test mixin and expected output directories for the skills layout. |
integrations/catalog.json |
Bump Bob integration version/description to reflect the 2.0.0 skills-based update. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Low
mnriem
left a comment
There was a problem hiding this comment.
As this fundamentally changes the layout for any Bob users what is the migration strategy? This will break them once they adopt a new version of Spec Kit so you need to make sure this goes through a deprecation cycle so they can migrate to the new Bob version. E.g make it an opt-in to the new version of Bob first and then in 2 minor releases (X.Y.Z where Y is minor) you can then remove the non-skill variant
Hi @mnriem |
|
Thanks @davidebibm — but I think we're talking about two different layers, and this still needs to change before it can land. Your point is about Bob the tool — that Bob 1.x can already read the skills layout. My concern is about Spec Kit's generated output: this PR changes what A hard cutover isn't acceptable here — we shouldn't catch existing users off guard. This needs to go through a proper deprecation cycle:
As it stands the Can you rework it along those lines — genuine dual-mode with skills opt-in first — so we phase this in without breaking anyone? |
|
Ok, thankyou @mnriem i'll do that |
|
@mnriem Done, hope this is what you are expecting |
|
Please address Copilot feedback. You will need to update the description to reflect the reality |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
@mnriem Addressed Copilot comments and aligned with main |
|
Please address Copilot feedback and resolve conflicts |
…te legacy commands to opt-in
6b3a18c to
674a8db
Compare
…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-<name> (skills is the default layout; no ai_skills flag required)
|
@mnriem Solved Copilot fb and rebased to avoid get Copilot feedback on other devs code 👍 |
…ator rule (review github#3415) Address review github#3415 (4725516805). The comment above resolve_command_refs still described the removed state-based behavior ("resolve it from the integration using the project's persisted skills state"). Update it to describe the output-layout rule that register_commands now uses: _sep is derived from the layout this registrar writes (a /SKILL.md scaffold uses the skills separator; a command-layout file uses the command separator), not the persisted ai_skills state. Comment-only change; no behavior change. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf
|
Addressed review 4725516805. Pushed as The comment above Posted on behalf of @mnriem by GitHub Copilot (model: Claude Opus 4.8), acting autonomously. |
…ub#3415) When a dual-mode agent (Bob) flips between the legacy commands layout and the skills layout during `integration upgrade` (via `--skills` / `--legacy-commands`), the old layout's extension command/skill files were left orphaned: Phase 2 stale cleanup only removes files tracked by the *integration* manifest, while extension artifacts are tracked in the extension registry. Detect the layout flip by comparing whether the old vs new manifest tracks a `/SKILL.md` scaffold, and when it changed, unregister the agent's extension artifacts before the existing re-registration so they are recreated in the new layout (and the per-agent registry is updated). Preset artifacts are documented as a known, pre-existing cross-cutting gap: no agent-scoped preset re-registration exists in use/switch/upgrade for any agent, so reconciling them is out of scope for this Bob migration. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf
|
Addressed review 4725829110 in Extension artifacts — reconciled. When a dual-mode agent (Bob) flips between the legacy commands layout and the skills layout during Preset artifacts — documented as a known, pre-existing limitation. There is no agent-scoped preset re-registration mechanism anywhere in the CLI: Full suite green after merging the latest Posted on behalf of @mnriem by GitHub Copilot (model: Claude Opus 4.8), acting autonomously. |
… (review github#3415) A command↔skills layout change during `integration upgrade` cannot reconcile preset artifacts: presets track their command/skill files in per-preset `registered_commands`/`registered_skills` metadata, and there is no agent-scoped preset re-registration anywhere in the CLI. Migrating would delete a preset's old-layout files without recreating them in the new layout and leave the preset registry claiming artifacts that no longer exist. Detect the intended layout via `is_skills_mode` (so a plain same-layout upgrade is unaffected) and, when it flips while preset overrides are installed for the agent, reject the upgrade *before any mutation* with an actionable error pointing at the remove → upgrade → reinstall workaround. Extension artifacts are still reconciled for the safe (no-preset) case. Adds a regression test and documents the migration caveat in the Bob integration reference entry. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf
|
Addressed review 4726193915 in Took the reviewer's second option — reject the migration with an actionable error — since full preset reconciliation would require a new cross-cutting A layout-changing Because the guard runs before shared-infra install and Full suite green: 4582 passed, 5 skipped. Posted on behalf of @mnriem by GitHub Copilot (model: Claude Opus 4.8), acting autonomously. |
…eview github#3415) `integration_upgrade` supports upgrading a secondary (non-active) integration, but the layout-change extension reconciliation was unsafe there. `ExtensionManager.unregister_agent_artifacts()` treats the unscoped per-extension `registered_skills` list as belonging to the passed agent and, when that agent's skills directory is absent, falls back to scanning every agent's skills directory — so reconciling a secondary Bob layout flip could delete or untrack the *active* agent's extension skills. The subsequent re-registration cannot repair that because extension skill rendering is intentionally scoped to the active agent (github#2948). Gate the unregister-before-register reconciliation on `installed_key == key` so it only runs for the active integration. Secondary agents only ever have extension command files (skills are active-agent-only), which the existing re-registration rewrites in place, so skipping the unregister orphans nothing new. Adds a regression test asserting a secondary Bob layout change leaves the active agent's extension skill intact on disk and in the registry. 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
|
Addressed review 4726347306 in Good catch — took the first option and restricted the layout-change reconciliation to the active integration. The unregister-before-register block is now gated on Rationale: Added a regression test ( Full suite green: 4583 passed, 5 skipped. Posted on behalf of @mnriem by GitHub Copilot (model: Claude Opus 4.8), acting autonomously. |
…ub#3415) Address review 4744636079: - _migrate_commands: the preset guard previously failed *open* — a registry read/parse error returned an empty "no presets" list, so a --force layout-changing upgrade could delete preset-overridden command files while their registry state was unknown. Read the registry file directly and raise _PresetRegistryUnreadableError on any read/parse failure or malformed structure, rejecting the migration before any mutation. A genuinely absent registry still returns [] (safe). - bob: correct the is_skills_mode docstring — upgrade *does* run setup(); disk detection is needed because legacy Bob 1.x installs never persisted a legacy_commands option, so the stored mode is unavailable. - tests: add fail-closed E2E (corrupted registry rejected, valid-empty allowed) plus a unit test for _installed_presets_affecting_agent covering absent / corrupted / malformed / valid / affecting-agent cases. 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
|
Addressed review 4744636079 in
Posted on behalf of @mnriem by GitHub Copilot (model: Claude Opus 4.8), acting autonomously. |
There was a problem hiding this comment.
Review details
Comments suppressed due to low confidence (1)
src/specify_cli/integrations/_migrate_commands.py:115
- The guard claims to fail closed on a corrupted registry, but malformed per-preset metadata is skipped, and a malformed
registered_commandsvalue is treated as no matching artifacts. A parseable registry such as{"presets":{"p1":[]}}therefore allows the layout migration even thoughp1's ownership is unknown, which can delete preset-managed files. Reject malformed entry/registration shapes with_PresetRegistryUnreadableErrorinstead of interpreting them as empty.
for preset_id, meta in data.get("presets", {}).items():
if not isinstance(meta, dict):
continue
registered_commands = meta.get("registered_commands", {})
has_commands = (
isinstance(registered_commands, dict)
and bool(registered_commands.get(agent_key))
)
has_skills = bool(meta.get("registered_skills"))
- Files reviewed: 19/19 changed files
- Comments generated: 1
- Review effort level: Medium
…3415) Address review 4745191015: the preset guard read a parseable registry but silently skipped malformed per-preset metadata and treated a malformed registered_commands value as "no matching artifacts". A registry such as {"presets":{"p1":[]}} therefore allowed a layout migration even though p1's ownership is unknown, risking deletion of preset-managed files. Now raise _PresetRegistryUnreadableError for a non-dict preset entry, a non-dict registered_commands, or a non-list registered_skills. Extend the unit test to cover these malformed shapes. 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
|
Addressed review 4745191015 in The preset guard read a parseable registry but silently skipped malformed per-preset metadata, and treated a malformed
Posted on behalf of @mnriem by GitHub Copilot (model: Claude Opus 4.8), acting autonomously. |
There was a problem hiding this comment.
Review details
Comments suppressed due to low confidence (2)
src/specify_cli/integrations/_migrate_commands.py:92
- The unreadable-registry guard does not cover this existence probe. On supported Python versions,
Path.exists()may raisePermissionErrorhere before it can be translated into_PresetRegistryUnreadableError; on newer versions that suppress probe errors, it can returnFalseand incorrectly treat an inaccessible registry as absent. Probe withstat()inside the protected path, distinguishing onlyFileNotFoundErrorfrom otherOSErrors, so permission failures consistently fail closed with the actionable CLI error.
if not registry_path.exists():
src/specify_cli/integrations/bob/init.py:59
- This migration guidance is inaccurate for auto-detected Bob 1.x projects: a plain
integration upgrade bobalready omits--legacy-commands, butis_skills_mode()deliberately preserves the on-disk command layout. Such users must explicitly pass--integration-options "--skills"; distinguish new-project behavior from the migration command so the warning provides an action that actually changes layouts.
"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/).",
- Files reviewed: 19/19 changed files
- Comments generated: 0 new
- Review effort level: Medium
|
Thank you! |
Bob 2.0 replaces the command-based workflow with a skills-based layout.
https://bob.ibm.com/blog/bob-v2-release-announcement
Description
Testing
uv run specify --helpuv sync && uv run pytestAI Disclosure
Very simple code generated by Bob but verified by me