diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index d4e8632215..f410639af3 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -425,6 +425,35 @@ def _get_skills_dir(project_path: Path, selected_ai: str) -> Path: return project_path / ".agents" / "skills" +def _cli_error_detail(exc: BaseException) -> str: + """Return a compact one-line exception detail for CLI output.""" + detail = str(exc).replace("\n", " ").strip() + return detail or exc.__class__.__name__ + + +def _cli_phase_label(phase: str, target_kind: str, target: str | None = None) -> str: + """Format a stable operation label for user-visible diagnostics.""" + label = f"{phase} {target_kind}".strip() + if target: + label = f"{label} '{target}'" + return label + + +def _print_cli_warning( + phase: str, + target_kind: str, + target: str | None, + exc: BaseException, + *, + continuing: str | None = None, +) -> None: + """Print a warning that names the failed CLI phase and target.""" + label = _cli_phase_label(phase, target_kind, target) + console.print(f"[yellow]Warning:[/yellow] Failed to {label}: {_cli_error_detail(exc)}") + if continuing: + console.print(f"[dim]{continuing}[/dim]") + + # Constants kept for backward compatibility with presets and extensions. DEFAULT_SKILLS_DIR = ".agents/skills" SKILL_DESCRIPTIONS = { @@ -857,9 +886,8 @@ def init( git_messages.append("bundled extension not found") except Exception as ext_err: git_has_error = True - sanitized_ext = str(ext_err).replace('\n', ' ').strip() git_messages.append( - f"extension install failed: {sanitized_ext[:120]}" + f"extension install failed during optional git setup: {_cli_error_detail(ext_err)[:120]}" ) summary = "; ".join(git_messages) if git_has_error: @@ -897,8 +925,10 @@ def init( else: tracker.skip("workflow", "bundled workflow not found") except Exception as wf_err: - sanitized_wf = str(wf_err).replace('\n', ' ').strip() - tracker.error("workflow", f"install failed: {sanitized_wf[:120]}") + tracker.error( + "workflow", + f"install bundled workflow 'speckit' failed: {_cli_error_detail(wf_err)[:120]}", + ) # Fix permissions after all installs (scripts + extensions) ensure_executable_scripts(project_path, tracker=tracker) @@ -960,7 +990,13 @@ def init( zip_path = preset_catalog.download_pack(preset) preset_manager.install_from_zip(zip_path, speckit_ver) except PresetError as preset_err: - console.print(f"[yellow]Warning:[/yellow] Failed to install preset '{preset}': {preset_err}") + _print_cli_warning( + "install", + "preset", + preset, + preset_err, + continuing="Continuing without the optional preset.", + ) finally: if zip_path is not None: # Clean up downloaded ZIP to avoid cache accumulation @@ -970,7 +1006,14 @@ def init( # Best-effort cleanup; failure to delete is non-fatal pass except Exception as preset_err: - console.print(f"[yellow]Warning:[/yellow] Failed to install preset: {preset_err}") + # Optional preset install must not abort project initialization. + _print_cli_warning( + "install", + "preset", + preset, + preset_err, + continuing="Continuing without the optional preset.", + ) tracker.complete("final", "project ready") except (typer.Exit, SystemExit): @@ -1782,20 +1825,29 @@ def integration_install( if new_default == integration.key: _update_init_options_for_integration(project_root, integration, script_type=selected_script) - except Exception as e: + except Exception as exc: # Attempt rollback of any files written by setup try: integration.teardown(project_root, manifest, force=True) except Exception as rollback_err: # Suppress so the original setup error remains the primary failure - console.print(f"[yellow]Warning:[/yellow] Failed to roll back integration changes: {rollback_err}") + _print_cli_warning( + "rollback", + "integration", + key, + rollback_err, + continuing="The original install failure is still the primary error.", + ) if installed_keys: _write_integration_json( project_root, default_key, installed_keys, _integration_settings(current) ) else: _remove_integration_json(project_root) - console.print(f"[red]Error:[/red] Failed to install integration: {e}") + console.print( + f"[red]Error:[/red] Failed to {_cli_phase_label('install', 'integration', key)}: " + f"{_cli_error_detail(exc)}" + ) raise typer.Exit(1) name = (integration.config or {}).get("name", key) @@ -2159,9 +2211,12 @@ def integration_switch( ext_mgr = ExtensionManager(project_root) ext_mgr.unregister_agent_artifacts(installed_key) except Exception as ext_err: - console.print( - f"[yellow]Warning:[/yellow] Could not clean up extension artifacts " - f"(commands, skills, registry entries) for '{installed_key}': {ext_err}" + _print_cli_warning( + "clean up extension artifacts for", + "integration", + installed_key, + ext_err, + continuing="Continuing with integration switch; old extension artifacts may need manual cleanup.", ) # Clear metadata so a failed Phase 2 doesn't leave stale references @@ -2256,18 +2311,27 @@ def integration_switch( ext_mgr = ExtensionManager(project_root) ext_mgr.register_enabled_extensions_for_agent(target) except Exception as ext_err: - console.print( - f"[yellow]Warning:[/yellow] Could not register extension commands, skills, " - f"or related artifacts for '{target}': {ext_err}" + _print_cli_warning( + "register extension artifacts for", + "integration", + target, + ext_err, + continuing="The integration switch succeeded, but installed extensions may need re-registration.", ) - except Exception as e: + except Exception as exc: # Attempt rollback of any files written by setup try: target_integration.teardown(project_root, manifest, force=True) except Exception as rollback_err: # Suppress so the original setup error remains the primary failure - console.print(f"[yellow]Warning:[/yellow] Failed to roll back integration '{target}': {rollback_err}") + _print_cli_warning( + "rollback", + "integration", + target, + rollback_err, + continuing="The original switch failure is still the primary error.", + ) if installed_keys: fallback_key = installed_keys[0] fallback_integration = get_integration(fallback_key) @@ -2296,7 +2360,10 @@ def integration_switch( ) else: _remove_integration_json(project_root) - console.print(f"[red]Error:[/red] Failed to install integration '{target}': {e}") + console.print( + f"[red]Error:[/red] Failed to {_cli_phase_label('install', 'integration', target)} " + f"during switch: {_cli_error_detail(exc)}" + ) raise typer.Exit(1) name = (target_integration.config or {}).get("name", target) @@ -2431,7 +2498,8 @@ def integration_upgrade( except Exception as exc: # Don't teardown — setup overwrites in-place, so teardown would # delete files that were working before the upgrade. Just report. - console.print(f"[red]Error:[/red] Failed to upgrade integration: {exc}") + console.print(f"[red]Error:[/red] Failed to {_cli_phase_label('upgrade', 'integration', key)}.") + console.print(f"[dim]Details:[/dim] {_cli_error_detail(exc)}") console.print("[yellow]The previous integration files may still be in place.[/yellow]") raise typer.Exit(1) diff --git a/tests/integrations/test_cli.py b/tests/integrations/test_cli.py index de09205310..6b171c4164 100644 --- a/tests/integrations/test_cli.py +++ b/tests/integrations/test_cli.py @@ -22,6 +22,26 @@ def _normalize_cli_output(output: str) -> str: return output.strip() +class TestCliDiagnosticFormatting: + def test_cli_error_detail_flattens_newlines(self): + import specify_cli + + assert specify_cli._cli_error_detail(RuntimeError("line one\nline two")) == "line one line two" + + def test_cli_error_detail_handles_empty_message(self): + import specify_cli + + assert specify_cli._cli_error_detail(RuntimeError()) == "RuntimeError" + + def test_cli_phase_label_includes_target(self): + import specify_cli + + assert ( + specify_cli._cli_phase_label("rollback", "integration", "codex") + == "rollback integration 'codex'" + ) + + class TestInitIntegrationFlag: def test_integration_and_ai_mutually_exclusive(self, tmp_path): from typer.testing import CliRunner @@ -174,6 +194,42 @@ def test_ai_generic_warning_suggests_integration_options_equivalent(self, tmp_pa assert normalized_output.index("Deprecation Warning") < normalized_output.index("Next Steps") assert (project / ".myagent" / "commands" / "speckit.plan.md").exists() + def test_init_optional_preset_failure_reports_target_and_continues( + self, tmp_path, monkeypatch + ): + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.presets import PresetManager + + def fail_install(self, path, version): + raise OSError("preset install exploded\nwith context") + + monkeypatch.setattr(PresetManager, "install_from_directory", fail_install) + + project = tmp_path / "init-preset-warning" + result = CliRunner().invoke( + app, + [ + "init", + str(project), + "--integration", + "copilot", + "--script", + "sh", + "--no-git", + "--preset", + "lean", + ], + catch_exceptions=False, + ) + normalized = _normalize_cli_output(result.output) + + assert result.exit_code == 0, result.output + assert "Failed to install preset 'lean'" in normalized + assert "preset install exploded with context" in normalized + assert "Continuing without the optional preset" in normalized + assert "Project ready" in normalized + def test_ai_claude_here_preserves_preexisting_commands(self, tmp_path): from typer.testing import CliRunner from specify_cli import app @@ -1055,6 +1111,143 @@ def _invoke(self, argv, cwd): finally: os.chdir(old) + def test_integration_install_failure_reports_phase_target_and_rollback( + self, tmp_path, monkeypatch + ): + from specify_cli.integrations import INTEGRATION_REGISTRY + from specify_cli.integrations.base import IntegrationBase + + class BrokenIntegration(IntegrationBase): + key = "broken-test" + config = { + "name": "Broken Test", + "folder": ".broken/", + "commands_subdir": "commands", + "install_url": None, + "requires_cli": False, + } + registrar_config = { + "dir": ".broken/commands", + "format": "markdown", + "args": "$ARGUMENTS", + "extension": ".md", + } + context_file = "BROKEN.md" + + def setup(self, project_root, manifest, **kwargs): + raise OSError("setup exploded\nwith context") + + def teardown(self, project_root, manifest, force=False): + raise OSError("rollback exploded") + + project = self._make_project(tmp_path) + monkeypatch.setitem(INTEGRATION_REGISTRY, "broken-test", BrokenIntegration()) + + result = self._invoke(["integration", "install", "broken-test"], project) + normalized = _normalize_cli_output(result.output) + + assert result.exit_code == 1, result.output + assert "Failed to rollback integration 'broken-test'" in normalized + assert "rollback exploded" in normalized + assert "Failed to install integration 'broken-test'" in normalized + assert "setup exploded with context" in normalized + + def test_integration_upgrade_failure_reports_phase_and_target( + self, tmp_path, monkeypatch + ): + from specify_cli.integrations import INTEGRATION_REGISTRY + from specify_cli.integrations.copilot import CopilotIntegration + + class UpgradeBrokenIntegration(CopilotIntegration): + key = "upgrade-broken" + config = dict(CopilotIntegration.config) + config["name"] = "Upgrade Broken" + + def setup(self, project_root, manifest, **kwargs): + raise OSError("upgrade exploded\nwith context") + + project = self._make_project(tmp_path) + monkeypatch.setitem( + INTEGRATION_REGISTRY, "upgrade-broken", UpgradeBrokenIntegration() + ) + + (project / ".specify" / "integrations").mkdir(parents=True, exist_ok=True) + (project / ".specify" / "integration.json").write_text( + json.dumps( + { + "version": 1, + "integration": "upgrade-broken", + "integrations": ["upgrade-broken"], + "integration_settings": {"upgrade-broken": {"script": "sh"}}, + } + ), + encoding="utf-8", + ) + ( + project / ".specify" / "integrations" / "upgrade-broken.manifest.json" + ).write_text( + json.dumps( + { + "integration": "upgrade-broken", + "version": "0.0.0", + "installed_at": "2026-05-16T00:00:00+00:00", + "files": {}, + } + ), + encoding="utf-8", + ) + + result = self._invoke(["integration", "upgrade", "upgrade-broken"], project) + normalized = _normalize_cli_output(result.output) + + assert result.exit_code == 1, result.output + assert "Failed to upgrade integration 'upgrade-broken'" in normalized + assert "upgrade exploded with context" in normalized + assert "previous integration files may still be in place" in normalized + + def test_integration_switch_cleanup_warning_reports_phase_and_targets( + self, tmp_path, monkeypatch + ): + from specify_cli.extensions import ExtensionManager + + project = self._make_project(tmp_path) + (project / ".specify" / "integrations").mkdir(parents=True, exist_ok=True) + (project / ".specify" / "integration.json").write_text( + json.dumps( + { + "version": 1, + "integration": "copilot", + "integrations": ["copilot"], + "integration_settings": {"copilot": {"script": "sh"}}, + } + ), + encoding="utf-8", + ) + (project / ".specify" / "integrations" / "copilot.manifest.json").write_text( + json.dumps( + { + "integration": "copilot", + "version": "0.0.0", + "installed_at": "2026-05-16T00:00:00+00:00", + "files": {}, + } + ), + encoding="utf-8", + ) + + def fail_cleanup(self, integration_key): + raise OSError("cleanup exploded") + + monkeypatch.setattr(ExtensionManager, "unregister_agent_artifacts", fail_cleanup) + + result = self._invoke(["integration", "switch", "claude"], project) + normalized = _normalize_cli_output(result.output) + + assert result.exit_code == 0, result.output + assert "Failed to clean up extension artifacts for integration 'copilot'" in normalized + assert "cleanup exploded" in normalized + assert "Switched to integration" in normalized + # -- Project guard ----------------------------------------------------- def test_search_requires_specify_project(self, tmp_path): diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 4c042fc7d5..39d43ea759 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -463,6 +463,7 @@ def test_validate_missing_command(self): assert any("missing 'command'" in e for e in errors) def test_step_override_integration(self): + from unittest.mock import patch from specify_cli.workflows.steps.command import CommandStep from specify_cli.workflows.base import StepContext @@ -474,7 +475,8 @@ def test_step_override_integration(self): "integration": "gemini", "input": {}, } - result = step.execute(config, ctx) + with patch("specify_cli.workflows.steps.command.shutil.which", return_value=None): + result = step.execute(config, ctx) assert result.output["integration"] == "gemini" def test_step_override_model(self): @@ -626,6 +628,7 @@ def test_execute_basic(self): assert result.output["dispatched"] is False def test_execute_with_step_integration(self): + from unittest.mock import patch from specify_cli.workflows.steps.prompt import PromptStep from specify_cli.workflows.base import StepContext @@ -637,10 +640,12 @@ def test_execute_with_step_integration(self): "prompt": "Summarize the codebase", "integration": "gemini", } - result = step.execute(config, ctx) + with patch("specify_cli.workflows.steps.prompt.shutil.which", return_value=None): + result = step.execute(config, ctx) assert result.output["integration"] == "gemini" def test_execute_with_model(self): + from unittest.mock import patch from specify_cli.workflows.steps.prompt import PromptStep from specify_cli.workflows.base import StepContext @@ -652,7 +657,8 @@ def test_execute_with_model(self): "prompt": "hello", "model": "opus-4", } - result = step.execute(config, ctx) + with patch("specify_cli.workflows.steps.prompt.shutil.which", return_value=None): + result = step.execute(config, ctx) assert result.output["model"] == "opus-4" def test_dispatch_with_mock_cli(self, tmp_path):