From b75a8d272a18d1bc027dd6fefe9c5c30df087567 Mon Sep 17 00:00:00 2001 From: AIOSAI Date: Fri, 10 Jul 2026 12:42:39 -0700 Subject: [PATCH] #665 items 1,2,8 (aipass CLI): --version wired to package metadata (was hardcoded 0.1.0), --help lists real COMMAND names not file stems (help not help_chat, init not init_flow), crash-vs-unknown distinguished (handler raise/import fail surfaces real cause, not 'Unknown command'). +5 tests. --- src/aipass/aipass/apps/aipass.py | 40 +++++-- src/aipass/aipass/tests/test_aipass_main.py | 113 +++++++++++++++++--- 2 files changed, 130 insertions(+), 23 deletions(-) diff --git a/src/aipass/aipass/apps/aipass.py b/src/aipass/aipass/apps/aipass.py index 36664c53..4b451595 100644 --- a/src/aipass/aipass/apps/aipass.py +++ b/src/aipass/aipass/apps/aipass.py @@ -18,6 +18,7 @@ Auto-discovery architecture: import os import sys import importlib +import importlib.metadata from pathlib import Path from typing import List, Any @@ -43,9 +44,13 @@ from aipass.prax import logger MODULES_DIR = Path(__file__).parent / "modules" +_import_failures: dict[str, Exception] = {} + + def discover_modules() -> List[Any]: """Auto-discover modules in modules/ directory.""" modules = [] + _import_failures.clear() if not MODULES_DIR.exists(): return modules @@ -62,18 +67,25 @@ def discover_modules() -> List[Any]: modules.append(module) except Exception as e: logger.error(f"[AIPASS] Failed to load module {module_name}: {e}") + _import_failures[file_path.stem] = e return modules def route_command(command: str, args: List[str], modules: List[Any]) -> bool: - """Route command to appropriate module.""" + """Route command to appropriate module. + + Returns True on success. Raises on handler crash so callers can + distinguish 'not found' (False) from 'found but broken'. + """ for module in modules: try: if module.handle_command(command, args): return True except Exception as e: - logger.error(f"[AIPASS] Module {module.__name__} error: {e}") + mod_name = module.__name__.split(".")[-1] + logger.error(f"[AIPASS] Module {mod_name} crashed: {e}") + raise return False @@ -88,14 +100,20 @@ def main(): args = sys.argv[1:] if len(args) > 0 and args[0] in ["--version", "-V"]: - print("aipass 0.1.0") + try: + version = importlib.metadata.version("aipass") + except importlib.metadata.PackageNotFoundError: + logger.info("[AIPASS] Package metadata not found, version unknown") + version = "unknown" + print(f"aipass {version}") return 0 show_root_help = len(args) == 0 or args[0] in ["--help", "-h"] or (args[0] == "help" and len(args) == 1) if show_root_help: print(f"AIPASS - {len(modules)} modules discovered") for module in modules: - name = module.__name__.split(".")[-1] + stem = module.__name__.split(".")[-1] + name = getattr(module, "COMMAND", stem) desc = (module.__doc__ or "").strip().split("\n")[0] if module.__doc__ else "No description" print(f" {name:20} {desc}") return 0 @@ -103,8 +121,13 @@ def main(): command = args[0] remaining = args[1:] if len(args) > 1 else [] - if route_command(command, remaining, modules): - return 0 + try: + if route_command(command, remaining, modules): + return 0 + except Exception as e: + print(f"Error: '{command}' crashed: {e}") + logger.error(f"[AIPASS] '{command}' traceback", exc_info=True) + return 1 if command.startswith("@"): print(f"{command} is a drone routing target, not an aipass command.") @@ -114,6 +137,11 @@ def main(): print(" aipass commands: aipass --help") return 1 + for stem, err in _import_failures.items(): + if command in (stem, stem.replace("_", "")): + print(f"Error: '{command}' failed to load: {err}") + return 1 + print(f"Unknown command: {command}") return 1 diff --git a/src/aipass/aipass/tests/test_aipass_main.py b/src/aipass/aipass/tests/test_aipass_main.py index fa5473bf..a096f311 100644 --- a/src/aipass/aipass/tests/test_aipass_main.py +++ b/src/aipass/aipass/tests/test_aipass_main.py @@ -10,6 +10,7 @@ from __future__ import annotations +import importlib.metadata import types from unittest.mock import MagicMock, patch @@ -129,23 +130,15 @@ class TestRouteCommand: mod2.handle_command.assert_called_once_with("cmd", ["arg1"]) mod3.handle_command.assert_not_called() - def test_handles_module_exception(self) -> None: - """Exception in a module is caught; returns False if no other handles.""" + def test_module_exception_re_raises(self) -> None: + """Exception in a handler is re-raised so callers see the real error.""" mod = MagicMock() mod.handle_command.side_effect = RuntimeError("crash") mod.__name__ = "broken_mod" - assert route_command("cmd", [], [mod]) is False + import pytest - def test_exception_in_first_tries_second(self) -> None: - """Exception in first module does not prevent second from handling.""" - mod1 = MagicMock() - mod1.handle_command.side_effect = RuntimeError("crash") - mod1.__name__ = "mod1" - mod2 = MagicMock() - mod2.handle_command.return_value = True - - assert route_command("cmd", [], [mod1, mod2]) is True - mod2.handle_command.assert_called_once() + with pytest.raises(RuntimeError, match="crash"): + route_command("cmd", [], [mod]) # ============================================================================= @@ -157,22 +150,39 @@ class TestMain: """Tests for the main() entry point.""" def test_version_flag(self) -> None: - """--version prints version and returns 0.""" + """--version prints real package version and returns 0.""" with patch("aipass.aipass.apps.aipass.sys.argv", ["aipass", "--version"]): with patch("aipass.aipass.apps.aipass.discover_modules", return_value=[]): with patch("builtins.print") as mock_print: result = main() assert result == 0 - mock_print.assert_called_once_with("aipass 0.1.0") + printed = mock_print.call_args[0][0] + assert printed.startswith("aipass ") + assert printed != "aipass 0.1.0" def test_version_flag_short(self) -> None: - """-V prints version and returns 0.""" + """-V prints real package version and returns 0.""" with patch("aipass.aipass.apps.aipass.sys.argv", ["aipass", "-V"]): with patch("aipass.aipass.apps.aipass.discover_modules", return_value=[]): with patch("builtins.print") as mock_print: result = main() assert result == 0 - mock_print.assert_called_once_with("aipass 0.1.0") + printed = mock_print.call_args[0][0] + assert printed.startswith("aipass ") + + def test_version_flag_fallback(self) -> None: + """--version prints 'unknown' when package metadata unavailable.""" + _not_found = importlib.metadata.PackageNotFoundError + with patch("aipass.aipass.apps.aipass.sys.argv", ["aipass", "--version"]): + with patch("aipass.aipass.apps.aipass.discover_modules", return_value=[]): + with patch( + "aipass.aipass.apps.aipass.importlib.metadata.version", + side_effect=_not_found, + ): + with patch("builtins.print") as mock_print: + result = main() + assert result == 0 + mock_print.assert_called_once_with("aipass unknown") def test_help_flag_shows_help(self) -> None: """--help shows module list and returns 0.""" @@ -294,3 +304,72 @@ class TestMain: with patch("aipass.aipass.apps.aipass.discover_modules", return_value=[mod]): main() mod.handle_command.assert_called_once_with("doctor", ["--verbose", "--fix"]) + + def test_help_shows_command_constant(self) -> None: + """Help listing uses module COMMAND constant, not file stem.""" + mod = types.ModuleType("aipass.aipass.apps.modules.help_chat") + mod.__doc__ = "Help chatbot" + mod.COMMAND = "help" # type: ignore[attr-defined] + mod.handle_command = lambda c, a: True # type: ignore[attr-defined] + with patch("aipass.aipass.apps.aipass.sys.argv", ["aipass"]): + with patch( + "aipass.aipass.apps.aipass.discover_modules", + return_value=[mod], + ): + with patch("builtins.print") as mock_print: + main() + printed = " ".join(str(a) for call in mock_print.call_args_list for a in call[0]) + assert "help" in printed + assert "help_chat" not in printed + + def test_help_falls_back_to_stem(self) -> None: + """Without COMMAND constant, help listing uses file stem.""" + mod = types.ModuleType("aipass.aipass.apps.modules.doctor") + mod.__doc__ = "Doctor module" + mod.handle_command = lambda c, a: True # type: ignore[attr-defined] + with patch("aipass.aipass.apps.aipass.sys.argv", ["aipass"]): + with patch( + "aipass.aipass.apps.aipass.discover_modules", + return_value=[mod], + ): + with patch("builtins.print") as mock_print: + main() + printed = " ".join(str(a) for call in mock_print.call_args_list for a in call[0]) + assert "doctor" in printed + + def test_handler_crash_surfaces_error(self) -> None: + """Handler crash prints real error, not 'Unknown command'.""" + mod = MagicMock() + mod.handle_command.side_effect = RuntimeError("db connection failed") + mod.__name__ = "aipass.aipass.apps.modules.doctor" + with patch("aipass.aipass.apps.aipass.sys.argv", ["aipass", "doctor"]): + with patch( + "aipass.aipass.apps.aipass.discover_modules", + return_value=[mod], + ): + with patch("builtins.print") as mock_print: + result = main() + assert result == 1 + printed = " ".join(str(a) for call in mock_print.call_args_list for a in call[0]) + assert "db connection failed" in printed + assert "Unknown command" not in printed + + def test_import_failure_surfaces_on_command(self) -> None: + """Failed module import surfaces when user types that command.""" + import aipass.aipass.apps.aipass as aipass_mod + + with patch("aipass.aipass.apps.aipass.sys.argv", ["aipass", "broken"]): + with patch( + "aipass.aipass.apps.aipass.discover_modules", + return_value=[], + ): + aipass_mod._import_failures.clear() + aipass_mod._import_failures["broken"] = ImportError("no module") + with patch("builtins.print") as mock_print: + result = main() + assert result == 1 + printed = " ".join(str(a) for call in mock_print.call_args_list for a in call[0]) + assert "failed to load" in printed + assert "no module" in printed + assert "Unknown command" not in printed + aipass_mod._import_failures.clear()