Skip to content

Commit 5b8203a

Browse files
committed
fix: address copilot review feedback
1 parent d063842 commit 5b8203a

5 files changed

Lines changed: 43 additions & 9 deletions

File tree

docs/reference/integrations.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,10 @@ specify integration switch <key>
8888
| Option | Description |
8989
| ------------------------ | ------------------------------------------------------------------------ |
9090
| `--script sh\|ps` | Script type: `sh` (bash/zsh) or `ps` (PowerShell) |
91-
| `--force` | Force removal of modified files during uninstall |
91+
| `--force` | Force removal of modified files during uninstall; when the target is already installed, overwrite managed shared templates while changing the default |
9292
| `--integration-options` | Options for the target integration |
9393

94-
If the target integration is not already installed, equivalent to running `uninstall` followed by `install` in a single step. If the target integration is already installed, `switch` only changes the default integration, like `use`.
94+
If the target integration is not already installed, equivalent to running `uninstall` followed by `install` in a single step. In this mode, `--force` controls whether modified files from the removed integration are deleted. If the target integration is already installed, `switch` only changes the default integration, like `use`; in this mode, `--force` controls whether managed shared templates are overwritten while the default changes.
9595

9696
## Use an Installed Integration
9797

src/specify_cli/__init__.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2171,8 +2171,10 @@ def integration_install(
21712171
unsafe_keys.append(installed_key)
21722172
if unsafe_keys or not getattr(integration, "multi_install_safe", False):
21732173
console.print(
2174-
f"[red]Error:[/red] Integration '{', '.join(installed_keys)}' is already installed."
2174+
f"[red]Error:[/red] Installed integrations: {', '.join(installed_keys)}."
21752175
)
2176+
if default_key:
2177+
console.print(f"Default integration: [cyan]{default_key}[/cyan].")
21762178
console.print(
21772179
"Installing multiple integrations is only automatic when all involved "
21782180
"integrations are declared multi-install safe."

src/specify_cli/shared_infra.py

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,29 @@
1010
from .integrations.manifest import IntegrationManifest
1111

1212

13-
def load_speckit_manifest(project_path: Path, *, version: str) -> IntegrationManifest:
13+
def load_speckit_manifest(
14+
project_path: Path,
15+
*,
16+
version: str,
17+
console: Any | None = None,
18+
) -> IntegrationManifest:
1419
"""Load the shared infrastructure manifest, preserving existing entries."""
1520
manifest_path = project_path / ".specify" / "integrations" / "speckit.manifest.json"
1621
if manifest_path.exists():
1722
try:
1823
manifest = IntegrationManifest.load("speckit", project_path)
1924
manifest.version = version
2025
return manifest
21-
except (ValueError, FileNotFoundError):
22-
pass
26+
except (ValueError, FileNotFoundError) as exc:
27+
if console is not None:
28+
console.print(
29+
f"[yellow]Warning:[/yellow] Could not read shared infrastructure "
30+
f"manifest at {manifest_path}: {exc}"
31+
)
32+
console.print(
33+
"A new shared manifest will be created; previously tracked "
34+
"shared files may be treated as untracked."
35+
)
2336
return IntegrationManifest("speckit", project_path, version=version)
2437

2538

@@ -60,7 +73,7 @@ def refresh_shared_templates(
6073
if not templates_src.is_dir():
6174
return
6275

63-
manifest = load_speckit_manifest(project_path, version=version)
76+
manifest = load_speckit_manifest(project_path, version=version, console=console)
6477
tracked_files = manifest.files
6578
modified = set(manifest.check_modified())
6679
skipped_files: list[str] = []
@@ -105,7 +118,7 @@ def install_shared_infra(
105118
invoke_separator: str = ".",
106119
) -> bool:
107120
"""Install shared scripts and templates into *project_path*."""
108-
manifest = load_speckit_manifest(project_path, version=version)
121+
manifest = load_speckit_manifest(project_path, version=version, console=console)
109122
skipped_files: list[str] = []
110123

111124
scripts_src = shared_scripts_source(core_pack=core_pack, repo_root=repo_root)

tests/integrations/test_cli.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,23 @@ def test_shared_infra_skip_warning_displayed(self, tmp_path, capsys):
254254
normalized = " ".join(captured.out.split())
255255
assert "specify integration upgrade --force" in normalized
256256

257+
def test_shared_infra_warns_when_manifest_cannot_be_loaded(self, tmp_path, capsys):
258+
"""Invalid shared manifests warn before falling back to a new manifest."""
259+
from specify_cli import _install_shared_infra
260+
261+
project = tmp_path / "bad-shared-manifest-test"
262+
project.mkdir()
263+
integrations_dir = project / ".specify" / "integrations"
264+
integrations_dir.mkdir(parents=True)
265+
manifest_path = integrations_dir / "speckit.manifest.json"
266+
manifest_path.write_text("{not json", encoding="utf-8")
267+
268+
_install_shared_infra(project, "sh")
269+
270+
captured = capsys.readouterr()
271+
assert "Could not read shared infrastructure manifest" in captured.out
272+
assert "A new shared manifest will be created" in captured.out
273+
257274
def test_shared_infra_no_warning_when_forced(self, tmp_path, capsys):
258275
"""No skip warning when force=True (all files overwritten)."""
259276
from specify_cli import _install_shared_infra

tests/integrations/test_integration_subcommand.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,8 @@ def test_install_different_when_one_exists(self, tmp_path):
131131
finally:
132132
os.chdir(old_cwd)
133133
assert result.exit_code != 0
134-
assert "already installed" in result.output
134+
assert "Installed integrations: copilot" in result.output
135+
assert "Default integration: copilot" in result.output
135136
assert "--force" in result.output
136137

137138
def test_install_multi_safe_integration(self, tmp_path):
@@ -215,6 +216,7 @@ def test_install_multi_unsafe_requires_force(self, tmp_path):
215216
finally:
216217
os.chdir(old_cwd)
217218
assert result.exit_code != 0
219+
assert "Installed integrations: copilot" in result.output
218220
assert "multi-install safe" in result.output
219221
assert "--force" in result.output
220222

0 commit comments

Comments
 (0)