From c785959443498894836da97bcf072a33f46308cc Mon Sep 17 00:00:00 2001 From: vibeforge1111 Date: Wed, 3 Jun 2026 02:21:54 +0400 Subject: [PATCH] fix: adopt low-risk CLI product diagnostics --- src/spark_cli/cli.py | 63 +++++++++++++++++++++++++++--- tests/test_cli.py | 93 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 151 insertions(+), 5 deletions(-) diff --git a/src/spark_cli/cli.py b/src/spark_cli/cli.py index 91dce4d00..ecd887b76 100644 --- a/src/spark_cli/cli.py +++ b/src/spark_cli/cli.py @@ -110,6 +110,7 @@ def discover_repo_root() -> Path: POWERSHELL_INSTALLER_REF_PATTERN = re.compile(r'\[string\]\$Ref\s*=\s*"([A-Za-z0-9._/-]+)"') TELEGRAM_BOT_TOKEN_PATTERN = re.compile(r"\b\d{5,}:[A-Za-z0-9_-]{20,}\b") TELEGRAM_BOT_TOKEN_TIMEOUT_SECONDS = 10 +PIP_EDITABLE_INSTALL_TIMEOUT_SECONDS = 300 MEMORY_SIDECAR_CHOICES = {"graphiti-kuzu"} MEMORY_SIDECAR_DISABLE_CHOICES = {"none", "off", "disabled"} DEFAULT_GRAPHITI_KUZU_DB_PATH = "{home}/sidecars/graphiti/kuzu/graphiti.kuzu" @@ -3272,6 +3273,17 @@ def module_runtime_env(module: Module, profile: str | None = None) -> dict[str, } LLM_PROVIDER_CHOICES = sorted(provider for provider in LLM_PROVIDER_ENV if provider != "not_configured") +LLM_DOCTOR_DIRECT_PROVIDER_CHOICES = ( + "anthropic", + "codex", + "huggingface", + "kimi", + "minimax", + "ollama", + "openai", + "openrouter", + "zai", +) LLM_ROLES = ("chat", "builder", "memory", "mission") LLM_PROVIDER_WIZARD_ORDER = ("codex", "anthropic", "zai", "kimi", "openrouter", "huggingface", "minimax", "lmstudio", "ollama", "openai") LLM_ROLE_LABELS = { @@ -4620,7 +4632,25 @@ def resolve_install_target(target: str, modules: dict[str, Module]) -> Module: if not manifest_path.exists(): raise SystemExit(f"{candidate} does not contain spark.toml") return load_module(candidate) - raise SystemExit(f"Unknown module target: {target}") + raise SystemExit(unknown_install_target_message(target, modules, registry)) + + +def unknown_install_target_message(target: str, modules: dict[str, Module], registry: dict[str, Any]) -> str: + installed_names = sorted(modules) + registry_modules = registry.get("modules") if isinstance(registry.get("modules"), dict) else {} + registry_names = sorted(name for name in registry_modules if name not in modules) + parts = [f"Unknown module target: {target}."] + if installed_names: + parts.append("Installed modules: " + ", ".join(installed_names) + ".") + else: + parts.append("No modules are installed yet.") + if registry_names: + parts.append("Registry-known modules: " + ", ".join(registry_names) + ".") + parts.append( + "Pass an installed module name, a registry-known module name, a git URL, " + "or a local directory containing spark.toml." + ) + return " ".join(parts) def install_module_record( @@ -5744,12 +5774,22 @@ def install_memory_sidecar_dependencies( install_target = f"{memory.path}[graphiti-kuzu]" print("Installing optional Graphiti/Kuzu memory sidecar extra for domain-chip-memory...") try: - subprocess.run([sys.executable, "-m", "pip", "install", "-e", install_target], check=True) + subprocess.run( + [sys.executable, "-m", "pip", "install", "-e", install_target], + check=True, + timeout=PIP_EDITABLE_INSTALL_TIMEOUT_SECONDS, + ) except subprocess.CalledProcessError as exc: raise SystemExit( f"Optional Graphiti/Kuzu memory sidecar install failed with exit code {exc.returncode}. " "Re-run setup with --skip-install-commands or install the sidecar manually." ) from None + except subprocess.TimeoutExpired as exc: + raise SystemExit( + f"Optional Graphiti/Kuzu memory sidecar install timed out after {exc.timeout}s. " + "The package download may be slow or the network unreachable. " + "Re-run setup with --skip-install-commands or install the sidecar manually." + ) from None except OSError as exc: raise SystemExit( "Optional Graphiti/Kuzu memory sidecar install could not start. " @@ -6628,12 +6668,21 @@ def cmd_browser_use(args: argparse.Namespace) -> int: print("Then run: spark browser-use probe") return 0 try: - subprocess.run([sys.executable, "-m", "pip", "install", "-e", f"{REPO_ROOT}[browser-use]"], check=True) + subprocess.run( + [sys.executable, "-m", "pip", "install", "-e", f"{REPO_ROOT}[browser-use]"], + check=True, + timeout=PIP_EDITABLE_INSTALL_TIMEOUT_SECONDS, + ) except subprocess.CalledProcessError as exc: raise SystemExit( f"browser-use package install failed with exit code {exc.returncode}. " "Fix the Python package install, then rerun `spark browser-use install`." ) from None + except subprocess.TimeoutExpired as exc: + raise SystemExit( + f"browser-use package install timed out after {exc.timeout}s. " + "The package download may be slow or the network unreachable." + ) from None except OSError as exc: raise SystemExit( "browser-use package install could not start. " @@ -8274,7 +8323,7 @@ def module_name_from_generated_env_path(path: Path) -> str | None: def resolve_installed_modules_best_effort() -> dict[str, Module]: try: return resolve_installed_modules() - except Exception: + except (Exception, SystemExit): return {} @@ -10839,7 +10888,11 @@ def call_llm_doctor(target: dict[str, Any], prompt: str) -> str: return claude_cli_completion(target, prompt) if provider == "ollama": return ollama_chat_completion(target, prompt) - raise SystemExit(f"Spark Doctor cannot directly call provider `{provider}` yet.") + raise SystemExit( + f"Spark Doctor cannot directly call provider `{provider}` yet. " + f"Supported providers: {', '.join(LLM_DOCTOR_DIRECT_PROVIDER_CHOICES)}. " + "Run `spark providers list` to see configured paths, or `spark setup` to switch." + ) def write_doctor_report(content: str, *, prefix: str = "spark-doctor") -> Path: diff --git a/tests/test_cli.py b/tests/test_cli.py index 26a354db7..63972c99b 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -266,6 +266,7 @@ windows_service_creationflags, resolve_bundle_names, resolve_setup_bundle_plan, + resolve_installed_modules_best_effort, resolve_install_target, resolve_restart_modules, resolve_start_modules, @@ -2197,6 +2198,10 @@ def test_security_revoke_all_reports_active_mission_pause_os_error(self) -> None self.assertTrue(any(item["path"] == str(active_path) for item in payload["failures"])) self.assertTrue(all("PermissionError" in item["error"] for item in payload["failures"])) + def test_resolve_installed_modules_best_effort_survives_broken_manifest(self) -> None: + with patch("spark_cli.cli.resolve_installed_modules", side_effect=SystemExit("broken manifest")): + self.assertEqual(resolve_installed_modules_best_effort(), {}) + def test_security_audit_includes_secret_surface_and_provider_checks(self) -> None: with patch("spark_cli.cli.collect_secret_surface_payload", return_value={"ok": False, "detail": "secret found"}), \ patch("spark_cli.cli.provider_status_payload", return_value={"ok": False, "summary": "No LLM provider is configured."}), \ @@ -2882,6 +2887,29 @@ def test_provider_test_wraps_windows_claude_powershell_shim(self) -> None: self.assertIn(r"C:\nvm\nodejs\claude.ps1", command) self.assertIn("--model", command) + def test_call_llm_doctor_unsupported_provider_names_supported_set(self) -> None: + target = {"provider": "experimental-xyz", "auth_mode": "api"} + + with self.assertRaises(SystemExit) as captured: + call_llm_doctor(target, "Spark is not working correctly.") + + message = str(captured.exception) + self.assertIn("`experimental-xyz`", message) + for provider in [ + "anthropic", + "codex", + "huggingface", + "kimi", + "minimax", + "ollama", + "openai", + "openrouter", + "zai", + ]: + self.assertIn(provider, message) + self.assertIn("spark providers list", message) + self.assertIn("spark setup", message) + def test_provider_test_explicit_codex_uses_codex_oauth_defaults(self) -> None: setup_state = { "llm": { @@ -4098,6 +4126,40 @@ def test_resolve_install_target_accepts_local_repo_path(self) -> None: resolved = resolve_install_target(str(repo_path), {}) self.assertEqual(resolved.name, "test-module") + def test_resolve_install_target_unknown_message_lists_installed_and_registry(self) -> None: + installed = { + "spark-cli": make_module("spark-cli", ["spark.cli"]), + "spark-telegram-bot": make_module("spark-telegram-bot", ["telegram.ingress"]), + } + registry = { + "modules": { + "spark-cli": {}, + "spark-character": {}, + "spark-researcher": {}, + } + } + + with patch("spark_cli.cli.load_registry_definition", return_value=registry): + with self.assertRaises(SystemExit) as captured: + resolve_install_target("typo", installed) + + message = str(captured.exception) + self.assertIn("Unknown module target: typo.", message) + self.assertIn("Installed modules: spark-cli, spark-telegram-bot.", message) + self.assertIn("Registry-known modules: spark-character, spark-researcher.", message) + self.assertIn("git URL", message) + self.assertIn("spark.toml", message) + + def test_resolve_install_target_unknown_message_handles_empty_registry(self) -> None: + with patch("spark_cli.cli.load_registry_definition", return_value={"modules": {}}): + with self.assertRaises(SystemExit) as captured: + resolve_install_target("typo", {}) + + message = str(captured.exception) + self.assertIn("Unknown module target: typo.", message) + self.assertIn("No modules are installed yet.", message) + self.assertNotIn("Registry-known modules:", message) + def test_resolve_bundle_names_reads_registry_bundle(self) -> None: self.assertEqual( resolve_bundle_names("telegram-starter"), @@ -4311,6 +4373,7 @@ def test_install_memory_sidecar_dependencies_installs_graphiti_kuzu_extra_when_e run.assert_called_once_with( [sys.executable, "-m", "pip", "install", "-e", f"{memory_root}[graphiti-kuzu]"], check=True, + timeout=300, ) def test_install_memory_sidecar_dependencies_honors_skip_install_commands(self) -> None: @@ -4348,6 +4411,22 @@ def test_install_memory_sidecar_dependencies_reports_pip_failure_without_traceba self.assertIn("Optional Graphiti/Kuzu memory sidecar install failed", message) self.assertIn("--skip-install-commands", message) + def test_install_memory_sidecar_dependencies_reports_pip_timeout_without_traceback(self) -> None: + memory = make_module("domain-chip-memory", ["spark.memory.substrate"]) + args = build_parser().parse_args(["setup", "--non-interactive", "--memory-sidecars", "graphiti-kuzu"]) + setup_state = {"memory_sidecars": {"enabled": ["graphiti-kuzu"]}} + + with patch( + "spark_cli.cli.subprocess.run", + side_effect=subprocess.TimeoutExpired([sys.executable, "-m", "pip"], 300), + ): + with self.assertRaises(SystemExit) as error: + install_memory_sidecar_dependencies(args, {"domain-chip-memory": memory}, setup_state) + + message = str(error.exception) + self.assertIn("Optional Graphiti/Kuzu memory sidecar install timed out after 300s", message) + self.assertIn("--skip-install-commands", message) + def test_install_memory_sidecar_dependencies_reports_start_failure_without_traceback(self) -> None: memory = make_module("domain-chip-memory", ["spark.memory.substrate"]) args = build_parser().parse_args(["setup", "--non-interactive", "--memory-sidecars", "graphiti-kuzu"]) @@ -13058,6 +13137,20 @@ def test_browser_use_install_reports_package_install_failure_without_traceback(s self.assertIn("browser-use package install failed", message) self.assertIn("exit code 2", message) + def test_browser_use_install_reports_package_install_timeout_without_traceback(self) -> None: + args = build_parser().parse_args(["browser-use", "install"]) + + with patch( + "spark_cli.cli.subprocess.run", + side_effect=subprocess.TimeoutExpired([sys.executable, "-m", "pip"], 300), + ): + with self.assertRaises(SystemExit) as error: + cmd_browser_use(args) + + message = str(error.exception) + self.assertIn("browser-use package install timed out after 300s", message) + self.assertIn("network unreachable", message) + def test_browser_use_install_reports_browser_setup_failure_without_traceback(self) -> None: args = build_parser().parse_args(["browser-use", "install"]) completed = subprocess.CompletedProcess([sys.executable, "-m", "pip"], 0, "", "")