Skip to content

Commit 8a162b3

Browse files
mnriemCopilot
andcommitted
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
1 parent d929bfb commit 8a162b3

9 files changed

Lines changed: 120 additions & 17 deletions

File tree

src/specify_cli/commands/init.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,7 @@ def init(
458458
script_type=selected_script,
459459
raw_options=integration_options,
460460
parsed_options=integration_parsed_options or None,
461+
project_root=project_path,
461462
)
462463
_write_integration_json(
463464
project_path,
@@ -478,7 +479,7 @@ def init(
478479
tracker=tracker,
479480
force=force,
480481
invoke_separator=resolved_integration.effective_invoke_separator(
481-
integration_parsed_options
482+
integration_parsed_options, project_root=project_path
482483
),
483484
)
484485
tracker.complete(

src/specify_cli/integration_runtime.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ def with_integration_setting(
4646
script_type: str | None = None,
4747
raw_options: str | None = None,
4848
parsed_options: dict[str, Any] | None = None,
49+
project_root: Any = None,
4950
) -> dict[str, dict[str, Any]]:
5051
"""Return integration settings with *key* updated."""
5152
settings = integration_settings(state)
@@ -63,7 +64,9 @@ def with_integration_setting(
6364
elif raw_options is not None:
6465
current.pop("parsed_options", None)
6566

66-
current["invoke_separator"] = integration.effective_invoke_separator(parsed_options)
67+
current["invoke_separator"] = integration.effective_invoke_separator(
68+
parsed_options, project_root
69+
)
6770
settings[key] = current
6871
return settings
6972

@@ -73,10 +76,11 @@ def invoke_separator_for_integration(
7376
state: dict[str, Any],
7477
key: str,
7578
parsed_options: dict[str, Any] | None = None,
79+
project_root: Any = None,
7680
) -> str:
7781
"""Resolve the invocation separator for stored/default integration state."""
7882
if parsed_options is not None:
79-
return integration.effective_invoke_separator(parsed_options)
83+
return integration.effective_invoke_separator(parsed_options, project_root)
8084

8185
setting = integration_setting(state, key)
8286
stored_separator = setting.get("invoke_separator")
@@ -85,6 +89,6 @@ def invoke_separator_for_integration(
8589

8690
stored_parsed = setting.get("parsed_options")
8791
if isinstance(stored_parsed, dict):
88-
return integration.effective_invoke_separator(stored_parsed)
92+
return integration.effective_invoke_separator(stored_parsed, project_root)
8993

90-
return integration.effective_invoke_separator(None)
94+
return integration.effective_invoke_separator(None, project_root)

src/specify_cli/integrations/_helpers.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,7 @@ def _set_default_integration(
320320
script_type=resolved_script,
321321
raw_options=raw_options,
322322
parsed_options=parsed_options,
323+
project_root=project_root,
323324
)
324325

325326
if refresh_templates:
@@ -328,7 +329,8 @@ def _set_default_integration(
328329
project_root,
329330
resolved_script,
330331
invoke_separator=_invoke_separator_for_integration(
331-
integration, {"integration_settings": settings}, key, parsed_options
332+
integration, {"integration_settings": settings}, key, parsed_options,
333+
project_root=project_root,
332334
),
333335
force=refresh_templates_force,
334336
refresh_managed=True,

src/specify_cli/integrations/_install_commands.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,8 @@ def integration_install(
127127
project_root,
128128
selected_script,
129129
invoke_separator=_invoke_separator_for_integration(
130-
infra_integration, current, infra_key, infra_parsed
130+
infra_integration, current, infra_key, infra_parsed,
131+
project_root=project_root,
131132
),
132133
)
133134
if os.name != "nt":
@@ -155,6 +156,7 @@ def integration_install(
155156
script_type=selected_script,
156157
raw_options=raw_options,
157158
parsed_options=parsed_options,
159+
project_root=project_root,
158160
)
159161
_write_integration_json(project_root, new_default, new_installed, settings)
160162
if new_default == integration.key:

src/specify_cli/integrations/_migrate_commands.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,8 @@ def integration_switch(
236236
force=refresh_shared_infra,
237237
refresh_managed=True,
238238
invoke_separator=_invoke_separator_for_integration(
239-
target_integration, current, target, parsed_options
239+
target_integration, current, target, parsed_options,
240+
project_root=project_root,
240241
),
241242
refresh_hint=(
242243
"To overwrite customizations, re-run with "
@@ -415,7 +416,8 @@ def integration_upgrade(
415416
selected_script,
416417
force=force,
417418
invoke_separator=_invoke_separator_for_integration(
418-
infra_integration, current, infra_key, infra_parsed
419+
infra_integration, current, infra_key, infra_parsed,
420+
project_root=project_root,
419421
),
420422
)
421423
if os.name != "nt":
@@ -441,14 +443,16 @@ def integration_upgrade(
441443
script_type=selected_script,
442444
raw_options=raw_options,
443445
parsed_options=parsed_options,
446+
project_root=project_root,
444447
)
445448
if installed_key == key:
446449
try:
447450
_install_shared_infra(
448451
project_root,
449452
selected_script,
450453
invoke_separator=_invoke_separator_for_integration(
451-
integration, {"integration_settings": settings}, key, parsed_options
454+
integration, {"integration_settings": settings}, key, parsed_options,
455+
project_root=project_root,
452456
),
453457
force=force,
454458
refresh_managed=True,

src/specify_cli/integrations/base.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -160,14 +160,16 @@ def options(cls) -> list[IntegrationOption]:
160160
return []
161161

162162
def effective_invoke_separator(
163-
self, parsed_options: dict[str, Any] | None = None
163+
self,
164+
parsed_options: dict[str, Any] | None = None,
165+
project_root: Path | None = None,
164166
) -> str:
165167
"""Return the invoke separator for the given options.
166168
167169
Subclasses whose separator depends on runtime options (e.g.
168170
Copilot in ``--skills`` mode) should override this method.
169-
The default implementation ignores *parsed_options* and returns
170-
the class-level ``invoke_separator``.
171+
The default implementation ignores *parsed_options* and
172+
*project_root* and returns the class-level ``invoke_separator``.
171173
"""
172174
return self.invoke_separator
173175

src/specify_cli/integrations/bob/__init__.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -171,10 +171,19 @@ def is_skills_mode(
171171
return True
172172

173173
def effective_invoke_separator(
174-
self, parsed_options: dict[str, Any] | None = None
174+
self,
175+
parsed_options: dict[str, Any] | None = None,
176+
project_root: Path | None = None,
175177
) -> str:
176-
"""``"."`` for the legacy commands layout, ``"-"`` for skills."""
177-
return "-" if self.is_skills_mode(parsed_options) else "."
178+
"""``"."`` for the legacy commands layout, ``"-"`` for skills.
179+
180+
*project_root* lets the ``use`` / ``switch`` / ``upgrade`` path — which
181+
refreshes shared infrastructure *before* persisting init-options —
182+
detect an already-installed legacy layout, so core command references
183+
are rendered with the correct separator instead of defaulting to the
184+
skills ``-``.
185+
"""
186+
return "-" if self.is_skills_mode(parsed_options, project_root) else "."
178187

179188
def invoke_separator_for_mode(self, skills_enabled: bool) -> str:
180189
"""Resolve the command-ref separator from a project's persisted mode.

src/specify_cli/integrations/copilot/__init__.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,9 @@ class CopilotIntegration(IntegrationBase):
122122
_skills_mode: bool = False
123123

124124
def effective_invoke_separator(
125-
self, parsed_options: dict[str, Any] | None = None
125+
self,
126+
parsed_options: dict[str, Any] | None = None,
127+
project_root: Path | None = None,
126128
) -> str:
127129
"""Return ``"-"`` when skills mode is requested, ``"."`` otherwise."""
128130
if parsed_options and parsed_options.get("skills"):

tests/integrations/test_integration_bob.py

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -545,3 +545,80 @@ def test_update_init_options_keeps_skills_project_as_skills(self, tmp_path):
545545

546546
opts = load_init_options(tmp_path)
547547
assert opts.get("ai_skills") is True
548+
549+
def test_with_integration_setting_stores_dot_separator_for_legacy(self, tmp_path):
550+
"""Regression (review #3415): shared-infra refresh on the use/switch
551+
path resolves the command-ref separator *before* init-options are
552+
rewritten, via ``effective_invoke_separator``. For an existing
553+
``.bob/commands`` project with no stored options this must resolve to
554+
``"."`` (project-aware), not the skills-layout ``"-"``; otherwise core
555+
command references get rewritten to ``/speckit-*``.
556+
"""
557+
from specify_cli.integration_runtime import with_integration_setting
558+
559+
(tmp_path / ".bob" / "commands").mkdir(parents=True)
560+
bob = get_integration("bob")
561+
562+
# Simulate the use/switch path: no parsed options stored.
563+
settings = with_integration_setting(
564+
{}, "bob", bob, parsed_options=None, project_root=tmp_path
565+
)
566+
assert settings["bob"]["invoke_separator"] == ".", (
567+
"legacy .bob/commands project must persist the dot separator so "
568+
"shared templates render Bob 1.x /speckit.<cmd> references"
569+
)
570+
571+
def test_use_force_keeps_legacy_command_refs_in_shared_templates(self, tmp_path):
572+
"""End-to-end (review #3415): ``integration use bob --force`` on an
573+
existing Bob 1.x project (legacy layout on disk, stored options
574+
stripped as a pre-PR install would be) must re-render shared templates
575+
with ``/speckit.<cmd>`` (dot), not ``/speckit-<cmd>``.
576+
"""
577+
import json
578+
from typer.testing import CliRunner
579+
from specify_cli import app
580+
581+
# Create a real legacy Bob project (renders shared templates).
582+
target = tmp_path / "proj"
583+
runner = CliRunner()
584+
result = runner.invoke(app, [
585+
"init", str(target), "--integration", "bob",
586+
"--integration-options", "--legacy-commands",
587+
"--ignore-agent-tools", "--script", "sh",
588+
])
589+
assert result.exit_code == 0, f"init failed: {result.output}"
590+
591+
template = target / ".specify" / "templates" / "plan-template.md"
592+
assert template.is_file(), "expected a rendered shared plan template"
593+
assert "/speckit.plan" in template.read_text(encoding="utf-8")
594+
595+
# Simulate a pre-PR Bob 1.x install: no stored options/separator.
596+
integ_json = target / ".specify" / "integration.json"
597+
data = json.loads(integ_json.read_text(encoding="utf-8"))
598+
bob_settings = data["integration_settings"]["bob"]
599+
for stale in ("raw_options", "parsed_options", "invoke_separator"):
600+
bob_settings.pop(stale, None)
601+
integ_json.write_text(json.dumps(data, indent=2), encoding="utf-8")
602+
603+
# Re-activate with --force so shared templates are re-rendered.
604+
import os
605+
old_cwd = os.getcwd()
606+
try:
607+
os.chdir(target)
608+
result = runner.invoke(
609+
app, ["integration", "use", "bob", "--force"]
610+
)
611+
finally:
612+
os.chdir(old_cwd)
613+
assert result.exit_code == 0, f"use failed: {result.output}"
614+
615+
rendered = template.read_text(encoding="utf-8")
616+
assert "/speckit.plan" in rendered, (
617+
"legacy Bob project must keep /speckit.plan (dot) after refresh"
618+
)
619+
assert "/speckit-plan" not in rendered, (
620+
"shared templates must not be rewritten to the skills /speckit-plan"
621+
)
622+
# And the persisted separator must reflect the legacy layout.
623+
data = json.loads(integ_json.read_text(encoding="utf-8"))
624+
assert data["integration_settings"]["bob"].get("invoke_separator") == "."

0 commit comments

Comments
 (0)