fix(integrations): dispatch bob commands via bob run - #4492
Open
temp-droid wants to merge 3 commits into
Open
Conversation
`BobIntegration` never overrode `build_exec_args()`, so it inherited the `IntegrationBase` no-op returning `None`. Callers read `None` as "this CLI is unavailable", so every workflow command/prompt step targeting Bob reported `CLI not found or not installed` even with `bob` on PATH. `build_command_invocation()` was inherited too, rendering `/speckit.specify` where skills-mode projects install `.bob/skills/speckit-specify/`.
mnriem
added
the
triage-nice-to-have
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Legacy-mode and dotted extension commands currently produce incorrect invocations.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Enables Bob workflow dispatch through its headless bob run interface.
Changes:
- Adds Bob CLI argument and command-invocation construction.
- Adds regression tests for dispatch formats and IDE-first configuration.
File summaries
| File | Description |
|---|---|
src/specify_cli/integrations/bob/__init__.py |
Implements Bob headless dispatch. |
tests/integrations/test_integration_bob.py |
Tests Bob dispatch behavior. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Collaborator
|
Thanks — the |
…dispatched into
`build_command_invocation()` called `effective_invoke_separator()` with no
arguments, so `is_skills_mode()` never reached its disk-detection branch
(bob/__init__.py:223-228) and always returned the fresh-project default.
`dispatch_command()` does receive the workflow project root, but the shared
two-argument `build_command_invocation(command_name, args)` contract -- which
eight integrations implement -- had nowhere to put it.
Two consequences, measured against a real `.bob/commands/speckit.specify.md`
project:
legacy project, speckit.specify -> /speckit-specify (want /speckit.specify)
skills project, speckit.git.commit -> /speckit-git.commit (want /speckit-git-commit)
(a) Every existing Bob 1.x install is dispatched a skills-mode invocation
naming a command it does not have, so the run fails exactly as it did before
dispatch was implemented.
(b) Only the `speckit.` prefix was converted, leaving inner dots. The
installed skill directory is `speckit-git-commit` -- `SkillsIntegration`
derives it with `stem.replace(".", "-")` (base.py:1759) and renders the
invocation the same way (base.py:1649) -- so a dotted extension command names
a skill that does not exist. The two layouts differ in both the separator and
the dot handling: skills flatten every dot, legacy commands keep them.
`_build_dispatch_prompt()` carries the project root from `dispatch_command()`
into invocation building. The base implementation is the call it replaces, so
the other integrations are unaffected; Bob overrides it to resolve the layout
from disk, falling back to the working directory, which is where
`dispatch_command` runs `bob` when no root is given.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TqDT4qTj3sFBeg9tMRiXqZ
Upstream github#4425 (per-step integration configuration) landed after this branch was pushed and collided with it in two ways. The textual conflict is one region of dispatch_command(): github#4425 inserted a validate_runtime_config() call on the line above the one this branch changed. The two edits are independent, so both survive -- validation first, then _build_dispatch_prompt() builds the prompt. The second collision is semantic and git does not flag it. github#4425 widened build_exec_args() with integration_args/integration_options and updated all fifteen integrations that override it. Bob's override arrives with this branch, so github#4425 had nothing to update, and merging alone would raise TypeError on every Bob dispatch -- the path this branch exists to make work. tests/integrations/test_base.py enumerates the registry and catches it: AssertionError: bob TypeError: BobIntegration.build_exec_args() got an unexpected keyword argument 'integration_args' Bob therefore takes the same four lines as every other integration: the two keyword-only parameters and a validate_runtime_config() call first, copied from CursorAgentIntegration and DroidIntegration. Bob accepts no per-step runtime configuration, so the inherited default -- which rejects non-empty values rather than ignoring them -- is the correct behaviour. Verified: 2915 integration tests pass; the registry test is red without the Bob change and green with it; ruff reports nothing on the three changed files; and dispatch renders /speckit.specify for a legacy project, /speckit-git-commit for a skills project. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqDT4qTj3sFBeg9tMRiXqZ
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The reviewed changes have regression coverage and no unresolved blocking issues.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #4491.
BobIntegrationnever overrodebuild_exec_args(), so it inherited theIntegrationBaseno-op returningNone. Every workflowcommand:/prompt:step targeting Bob failed with "CLI not found or not installed" even with
bobon PATH. Bob Shell has a headless entry point (
bob run), so this dispatchesthrough it.
Also overrides
build_command_invocation(), which was inherited fromIntegrationBasewith a hardcoded.and rendered/speckit.specifywhereskills-mode projects install
.bob/skills/speckit-specify/. The class alreadycomputes the right separator in
effective_invoke_separator().requires_clistaysFalse— dispatch is opt-in by overriding the method,mirroring
CursorAgentIntegration. Same fix shape as #2631.Testing
uv run specify --helpuv sync && uv run pytestAdded 6 tests to
tests/integrations/test_integration_bob.py(62 pass in thatfile);
tests/test_agent_config_consistency.pypasses.Verified end-to-end against Bob Shell 2.0.1: a workspace skill at
.bob/skills/speckit-marker/was reached throughdispatch_command → build_exec_args → bob run, returningstatus: successwith one
UseSkillToolcall.Note: the full suite shows 11 failures in the bash/Python parity tests. They are
identical on
mainwithout this change — 11 failed / 7702 passed onmain,11 failed / 7708 passed here, same test set — and pass when run in isolation.
AI Disclosure
Investigation, patch and tests written with Claude; reviewed and verified by me.