diff --git a/src/aipass/ai_mail/apps/modules/email.py b/src/aipass/ai_mail/apps/modules/email.py index e68ad869..18fcad0f 100644 --- a/src/aipass/ai_mail/apps/modules/email.py +++ b/src/aipass/ai_mail/apps/modules/email.py @@ -68,6 +68,20 @@ def _delivery_callback(branch_path, new_count, opened_count, total): update_central_fn=update_central) +def _resolve_branch_path() -> Path: + """Resolve the branch path for inbox operations. + + Tries get_current_user() first (detects caller's branch). + Falls back to this module's own branch path when caller detection fails + (e.g., user calling from terminal outside any branch directory). + """ + try: + return Path(get_current_user()["mailbox_path"]).parent + except RuntimeError as e: + logger.warning("[email] caller detection failed, using own branch: %s", e) + return _AI_MAIL_DIR + + HELP_TEXT = """ Email Module - Send and manage branch-to-branch email (Lifecycle v2) @@ -250,7 +264,16 @@ def handle_inbox(args: List[str]) -> bool: json_handler.log_operation("inbox_viewed") try: first_arg = args[0] if args else None - ok, info = resolve_inbox_target(first_arg, _REPO_ROOT, get_branch_by_email, get_current_user) + def _get_user_with_fallback(): + try: + return get_current_user() + except RuntimeError as e: + logger.warning("[email] caller detection failed for inbox, using own branch: %s", e) + return { + "mailbox_path": str(_AI_MAIL_DIR / ".ai_mail.local"), + "display_name": "AI_MAIL", + } + ok, info = resolve_inbox_target(first_arg, _REPO_ROOT, get_branch_by_email, _get_user_with_fallback) if not ok: error(info['error']) return False @@ -290,13 +313,13 @@ def handle_view(args: List[str]) -> bool: json_handler.log_operation("view_email_initiated", {"args": args}) if not args: error("Usage: drone @ai_mail view ") - return False + return True try: - branch_path = Path(get_current_user()["mailbox_path"]).parent + branch_path = _resolve_branch_path() success, message, email_data = mark_as_opened(branch_path, args[0]) if not success or email_data is None: error(message) - return False + return True header = format_email_header(email_data) console.print(f"\n{header}") console.print(f"\n{email_data.get('message', '')}\n{'='*70}") @@ -311,7 +334,7 @@ def handle_view(args: List[str]) -> bool: except Exception as e: logger.error(f"[email] View failed: {e}") error(f"Error: {e}") - return False + return True def handle_close(args: List[str]) -> bool: @@ -319,9 +342,9 @@ def handle_close(args: List[str]) -> bool: json_handler.log_operation("close_email_initiated", {"args": args}) if not args: error("Usage: drone @ai_mail close [id2 ...] | close all") - return False + return True try: - branch_path = Path(get_current_user()["mailbox_path"]).parent + branch_path = _resolve_branch_path() if args[0].lower() == "all": success, message, count = mark_all_read_and_archive(branch_path) if success: @@ -354,7 +377,7 @@ def handle_close(args: List[str]) -> bool: except Exception as e: logger.error(f"[email] Close failed: {e}") error(f"Error: {e}") - return False + return True def handle_reply(args: List[str]) -> bool: @@ -362,14 +385,14 @@ def handle_reply(args: List[str]) -> bool: json_handler.log_operation("reply_email_initiated", {"args": args}) if len(args) < 2: error("Usage: drone @ai_mail reply \"your message\"") - return False + return True try: - branch_path = Path(get_current_user()["mailbox_path"]).parent + branch_path = _resolve_branch_path() inbox_file = branch_path / ".ai_mail.local" / "inbox.json" original = get_email_by_id(inbox_file, args[0]) if not original: error(f"Message not found: {args[0]}") - return False + return True success, message, reply_id = send_reply(branch_path, original, args[1]) if success: console.print(f"[green]{message}[/green]") @@ -377,18 +400,18 @@ def handle_reply(args: List[str]) -> bool: error(message) if success: json_handler.log_operation("email_replied", {"message_id": args[0], "reply_id": reply_id}) - return success + return True except Exception as e: logger.error(f"[email] Reply failed: {e}") error(f"Error: {e}") - return False + return True def handle_sent(args: List[str]) -> bool: """View sent messages.""" json_handler.log_operation("sent_viewed") try: - sent_folder = Path(get_current_user()["mailbox_path"]) / "sent" + sent_folder = _resolve_branch_path() / ".ai_mail.local" / "sent" if not sent_folder.exists(): console.print("No sent messages") return True @@ -406,7 +429,7 @@ def handle_sent(args: List[str]) -> bool: except Exception as e: logger.error(f"[email] Sent view failed: {e}") error(f"Error: {e}") - return False + return True def handle_contacts(args: List[str]) -> bool: @@ -416,7 +439,7 @@ def handle_contacts(args: List[str]) -> bool: branches = get_all_branches() if not branches: error("No contacts found") - return False + return True console.print(f"\nTotal: {len(branches)} branches\n") console.print(f"{'EMAIL':<20} {'BRANCH NAME':<25} {'PATH':<35}") console.print("-" * 80) @@ -426,7 +449,7 @@ def handle_contacts(args: List[str]) -> bool: except Exception as e: logger.error(f"[email] Contacts view failed: {e}") error(f"Error: {e}") - return False + return True def print_introspection(): diff --git a/src/aipass/api/apps/handlers/openrouter/models.py b/src/aipass/api/apps/handlers/openrouter/models.py index 95d74adb..ad75939a 100644 --- a/src/aipass/api/apps/handlers/openrouter/models.py +++ b/src/aipass/api/apps/handlers/openrouter/models.py @@ -65,7 +65,7 @@ def fetch_models_from_api(api_key: str) -> List[Dict]: # Make API request logger.info(f"[{MODULE_NAME}] Requesting models from OpenRouter API") - response = requests.get( + response = requests.get( # type: ignore[attr-defined] OPENROUTER_API_URL, headers=headers, timeout=DEFAULT_TIMEOUT diff --git a/src/aipass/api/apps/handlers/usage/tracking.py b/src/aipass/api/apps/handlers/usage/tracking.py index 9621d142..c394c2e7 100644 --- a/src/aipass/api/apps/handlers/usage/tracking.py +++ b/src/aipass/api/apps/handlers/usage/tracking.py @@ -144,7 +144,7 @@ def get_generation_metrics(generation_id: str, api_key: str) -> Optional[Dict[st } # Query the generation endpoint - response = requests.get( + response = requests.get( # type: ignore[attr-defined] GENERATION_ENDPOINT, params={"id": generation_id}, headers=headers, diff --git a/src/aipass/api/apps/modules/api_key.py b/src/aipass/api/apps/modules/api_key.py index 413fe146..a68b4c20 100644 --- a/src/aipass/api/apps/modules/api_key.py +++ b/src/aipass/api/apps/modules/api_key.py @@ -72,25 +72,25 @@ def handle_command(command: str, args: List[str]) -> bool: # Log operation json_handler.log_operation(f"api_key_{command}", {"command": command}) - # Standalone commands — route before introspection gate + # Route all commands before introspection gate if command == "list-providers": list_providers() return True if command == "init": init_env() return True + if command == "get-key": + get_key(args) + return True + if command == "validate": + validate_key(args) + return True - # NO-ARGS GATE (seedgo standard) + # NO-ARGS GATE (seedgo standard) — only for unrecognized subcommands if not args: print_introspection() return True - # Arg-required commands - if command == "get-key": - get_key(args) - elif command == "validate": - validate_key(args) - return True except Exception as e: logger.error(f"Error in api_key.handle_command: {e}") diff --git a/src/aipass/api/apps/modules/openrouter_client.py b/src/aipass/api/apps/modules/openrouter_client.py index 01684e48..d1a7e07f 100644 --- a/src/aipass/api/apps/modules/openrouter_client.py +++ b/src/aipass/api/apps/modules/openrouter_client.py @@ -132,7 +132,7 @@ def handle_command(command: str, args: List[str]) -> bool: # Log operation json_handler.log_operation(f"openrouter_{command}", {"command": command}) - # Standalone commands — route before introspection gate + # Route all commands before introspection gate if command == "test": test_connection() return True @@ -142,16 +142,15 @@ def handle_command(command: str, args: List[str]) -> bool: if command == "status": check_status() return True + if command == "call": + make_call(args) + return True - # NO-ARGS GATE (seedgo standard) + # NO-ARGS GATE (seedgo standard) — only for unrecognized subcommands if not args: print_introspection() return True - # Arg-required commands - if command == "call": - make_call(args) - return True except Exception as e: logger.error(f"Error in openrouter_client.handle_command: {e}") diff --git a/src/aipass/api/apps/modules/usage_tracker.py b/src/aipass/api/apps/modules/usage_tracker.py index 4d062a99..a2b78a62 100644 --- a/src/aipass/api/apps/modules/usage_tracker.py +++ b/src/aipass/api/apps/modules/usage_tracker.py @@ -142,27 +142,28 @@ def handle_command(command: str, args: List[str]) -> bool: # Log operation json_handler.log_operation(f"usage_{command}", {"command": command}) - # Standalone commands — route before introspection gate + # Route all commands before introspection gate if command == "stats": show_stats() return True if command == "session": show_session() return True + if command == "track": + track_usage(args) + return True + if command == "caller-usage": + show_caller_usage(args) + return True + if command == "cleanup": + cleanup_data(args) + return True - # NO-ARGS GATE (seedgo standard) + # NO-ARGS GATE (seedgo standard) — only for unrecognized subcommands if not args: print_introspection() return True - # Arg-required commands - if command == "track": - track_usage(args) - elif command == "caller-usage": - show_caller_usage(args) - elif command == "cleanup": - cleanup_data(args) - return True except Exception as e: logger.error(f"Error in usage_tracker.handle_command: {e}") diff --git a/src/aipass/api/tests/test_api_key.py b/src/aipass/api/tests/test_api_key.py index 6c69a4b2..cbf0ab59 100644 --- a/src/aipass/api/tests/test_api_key.py +++ b/src/aipass/api/tests/test_api_key.py @@ -165,16 +165,41 @@ def test_handle_command_help_gate_short_flag(mock_jh, mock_header, mock_console) @patch(PATCH_CONSOLE) @patch(PATCH_HEADER) +@patch(PATCH_SUCCESS) +@patch(PATCH_ERROR) @patch(PATCH_JSON_HANDLER) -def test_handle_command_introspection_gate(mock_jh, mock_header, mock_console): - """get-key with no args should trigger print_introspection and return True.""" +@patch(PATCH_KEYS) +def test_handle_command_get_key_no_args_defaults_to_openrouter( + mock_keys, mock_jh, mock_error, mock_success, mock_header, mock_console +): + """get-key with no args should execute with default provider 'openrouter'.""" + mock_keys.get_api_key.return_value = "sk-or-test-key-123" + result = api_key.handle_command("get-key", []) assert result is True - # Introspection gate fires after log_operation - mock_jh.log_operation.assert_called_once() - # Introspection prints the header "API Key Module Introspection" - mock_header.assert_called_with("API Key Module Introspection") + mock_keys.get_api_key.assert_called_once_with("openrouter") + mock_header.assert_called_with("Get API Key - openrouter") + + +@patch(PATCH_CONSOLE) +@patch(PATCH_HEADER) +@patch(PATCH_SUCCESS) +@patch(PATCH_ERROR) +@patch(PATCH_JSON_HANDLER) +@patch(PATCH_KEYS) +def test_handle_command_validate_no_args_defaults_to_openrouter( + mock_keys, mock_jh, mock_error, mock_success, mock_header, mock_console +): + """validate with no args should execute with default provider 'openrouter'.""" + mock_keys.get_api_key.return_value = "sk-or-test-key-123" + mock_keys.validate_key.return_value = True + + result = api_key.handle_command("validate", []) + + assert result is True + mock_keys.get_api_key.assert_called_once_with("openrouter") + mock_header.assert_called_with("Validate API Key - openrouter") @patch(PATCH_CONSOLE) diff --git a/src/aipass/api/tests/test_openrouter_client.py b/src/aipass/api/tests/test_openrouter_client.py index 3714c44e..0b67854b 100644 --- a/src/aipass/api/tests/test_openrouter_client.py +++ b/src/aipass/api/tests/test_openrouter_client.py @@ -124,18 +124,19 @@ def test_handle_command_help_gate(mock_console, mock_header, mock_jh, mock_help) mock_jh.log_operation.assert_not_called() -@patch(f"{_MOD}.print_introspection") +@patch(f"{_MOD}.error") @patch(f"{_MOD}.json_handler") @patch(f"{_MOD}.header") @patch(f"{_MOD}.console") -def test_handle_command_introspection_gate(mock_console, mock_header, mock_jh, mock_intro): - """'call' with no args triggers introspection instead of make_call.""" +def test_handle_command_call_no_args_executes(mock_console, mock_header, mock_jh, mock_error): + """'call' with no args should execute (show error), not show introspection.""" from aipass.api.apps.modules import openrouter_client result = openrouter_client.handle_command("call", []) assert result is True - mock_intro.assert_called_once() + mock_error.assert_called() + assert "Prompt required" in mock_error.call_args[0][0] # ============================================= @@ -609,6 +610,21 @@ def test_make_call_no_model_shows_error(mock_console, mock_header, mock_error): assert "Model required" in mock_error.call_args[0][0] +@patch(f"{_MOD}.error") +@patch(f"{_MOD}.json_handler") +@patch(f"{_MOD}.header") +@patch(f"{_MOD}.console") +def test_handle_command_call_no_args_shows_error(mock_console, mock_header, mock_jh, mock_error): + """call with no args should show error, not introspection.""" + from aipass.api.apps.modules import openrouter_client + + result = openrouter_client.handle_command("call", []) + + assert result is True + mock_error.assert_called_once() + assert "Prompt required" in mock_error.call_args[0][0] + + # ============================================= # list_models — error on fetch failure # ============================================= diff --git a/src/aipass/api/tests/test_usage_tracker.py b/src/aipass/api/tests/test_usage_tracker.py index be7872ad..7522abf9 100644 --- a/src/aipass/api/tests/test_usage_tracker.py +++ b/src/aipass/api/tests/test_usage_tracker.py @@ -154,18 +154,19 @@ def test_handle_command_help_gate(mock_jh, mock_header, mock_console, mock_help) mock_jh.log_operation.assert_not_called() -@patch(f"{PATCH_ROOT}.print_introspection") +@patch(f"{PATCH_ROOT}.error") @patch(f"{PATCH_ROOT}.console") @patch(f"{PATCH_ROOT}.header") @patch(f"{PATCH_ROOT}.json_handler") -def test_handle_command_introspection_gate(mock_jh, mock_header, mock_console, mock_intro): - """'track' with empty args triggers introspection gate.""" +def test_handle_command_track_no_args_executes(mock_jh, mock_header, mock_console, mock_error): + """'track' with empty args should execute (show error), not show introspection.""" from aipass.api.apps.modules import usage_tracker result = usage_tracker.handle_command("track", []) assert result is True - mock_intro.assert_called_once() + mock_error.assert_called() + assert "Generation ID required" in mock_error.call_args[0][0] @patch(f"{PATCH_ROOT}.console") diff --git a/src/aipass/backup/apps/modules/google_drive_sync.py b/src/aipass/backup/apps/modules/google_drive_sync.py index 373e6518..b788935b 100644 --- a/src/aipass/backup/apps/modules/google_drive_sync.py +++ b/src/aipass/backup/apps/modules/google_drive_sync.py @@ -280,13 +280,15 @@ def handle_command(args) -> bool: console.print(" drive-test - Test Google Drive connectivity") console.print(" drive-sync - Sync backup directory to Google Drive") console.print(" drive-sync --test - Run a small test sync to verify integration") - console.print(" drive-clear-tracker - Clear file tracker cache") + console.print(" drive-sync --dry-run - Preview what would be uploaded (no files sent)") + console.print(" drive-clear-tracker --force - Clear file tracker cache (requires --force)") console.print(" drive-stats - Show file tracker statistics") console.print() console.print("[yellow]Options:[/yellow]") - console.print(" --project Project name (default: AIPass)") - console.print(" --note Sync note (default: Manual sync)") - console.print(" --force Force upload all files") + console.print(" --project Project name (default: AIPass)") + console.print(" --note Sync note (default: Manual sync)") + console.print(" --force Force upload all files / confirm destructive actions") + console.print(" --dry-run Preview without executing") console.print() return True @@ -361,6 +363,25 @@ def handle_command(args) -> bool: console.print("[green]All files up to date - nothing to sync[/green]") return True + # Dry-run: show what would be uploaded, then stop + dry_run = getattr(args, 'dry_run', False) + if dry_run: + console.print() + warning("DRY-RUN MODE — showing what would be uploaded (no files sent)") + console.print() + for file_path in files_to_upload[:20]: + try: + rel_path = file_path.relative_to(backup_path) + except ValueError as e: + logger.info(f"[google_drive_sync] Could not compute relative path: {e}") + rel_path = file_path.name + console.print(f" [dim]Would upload:[/dim] {rel_path}") + if upload_count > 20: + console.print(f" [dim]... and {upload_count - 20} more files[/dim]") + console.print() + console.print(f"[green]Dry-run complete: {upload_count} files would be uploaded, 0 sent[/green]") + return True + console.print() # Phase 2: Upload with progress bar @@ -415,6 +436,13 @@ def handle_command(args) -> bool: return True # Command was handled; success/failure shown to user above elif command == 'drive-clear-tracker': + force = getattr(args, 'force', False) + if not force: + data = _load_data() if _LOAD_DATA_FN else {} + tracker_count = len(data.get("runtime_state", {}).get("file_tracker", {})) + warning(f"This will clear {tracker_count} tracked file entries. Next sync will re-upload all files.") + console.print(" Run with --force to confirm: drone @backup drive-clear-tracker --force") + return True _clear_file_tracker() return True diff --git a/src/aipass/backup/tests/test_google_drive_sync.py b/src/aipass/backup/tests/test_google_drive_sync.py index 8b713e43..5ef55d84 100644 --- a/src/aipass/backup/tests/test_google_drive_sync.py +++ b/src/aipass/backup/tests/test_google_drive_sync.py @@ -163,10 +163,27 @@ class TestHandleCommand: assert result is True mock_fn.assert_called_once() - def test_handle_command_drive_clear_tracker(self, drive_sync_env): - """'drive-clear-tracker' routes to _clear_file_tracker function.""" + def test_handle_command_drive_clear_tracker_without_force(self, drive_sync_env): + """'drive-clear-tracker' without --force shows warning, does NOT clear.""" mod = drive_sync_env["module"] - args = SimpleNamespace(command="drive-clear-tracker") + args = SimpleNamespace(command="drive-clear-tracker", force=False) + + with patch.object( + mod, "_clear_file_tracker", return_value=True + ) as mock_fn: + with patch.object( + mod, "_load_data", + return_value={"runtime_state": {"file_tracker": {"a": 1, "b": 2}}}, + ): + result = drive_sync_env["handle_command"](args) + + assert result is True + mock_fn.assert_not_called() + + def test_handle_command_drive_clear_tracker_with_force(self, drive_sync_env): + """'drive-clear-tracker --force' routes to _clear_file_tracker.""" + mod = drive_sync_env["module"] + args = SimpleNamespace(command="drive-clear-tracker", force=True) with patch.object( mod, "_clear_file_tracker", return_value=True @@ -199,6 +216,89 @@ class TestHandleCommand: assert result is True mock_fn.assert_called_once() + def test_handle_command_drive_sync_dry_run_no_upload(self, drive_sync_env, tmp_path, monkeypatch): + """'drive-sync --dry-run' scans but does NOT upload files.""" + mod = drive_sync_env["module"] + + # Mock the backup_timestamps module (imported inside the function) + mock_ts_mod = MagicMock() + mock_ts_mod.get_timestamps = MagicMock(return_value={}) + mock_ts_mod.format_age = MagicMock(return_value="never") + monkeypatch.setitem( + sys.modules, "aipass.backup.apps.handlers.utils.backup_timestamps", mock_ts_mod + ) + + # Create a fake backup dir with files + backup_dir = tmp_path / "backups" / "system_snapshot" + backup_dir.mkdir(parents=True) + (backup_dir / "test.txt").write_text("hello") + + mock_sync = MagicMock() + mock_sync.authenticate.return_value = True + mock_sync.get_or_create_project_folder.return_value = "folder_id" + mock_sync.tracker_was_reset = False + mock_sync.prepare_sync.return_value = ( + [backup_dir / "test.txt"], # files_to_upload (list of Paths) + 0, # skipped + 1, # total + ) + + args = SimpleNamespace( + command="drive-sync", path=str(backup_dir), verbose=False, + note="test", dry_run=True, project="AIPass", force=False, + test=False, limit=0, + ) + + with patch.object(mod, "GoogleDriveSync", return_value=mock_sync): + result = drive_sync_env["handle_command"](args) + + assert result is True + # Critical: sync_backup_files must NOT be called in dry-run + mock_sync.sync_backup_files.assert_not_called() + + def test_handle_command_drive_sync_no_dry_run_uploads(self, drive_sync_env, tmp_path, monkeypatch): + """'drive-sync' without --dry-run DOES upload files.""" + mod = drive_sync_env["module"] + + # Mock the backup_timestamps module + mock_ts_mod = MagicMock() + mock_ts_mod.get_timestamps = MagicMock(return_value={}) + mock_ts_mod.format_age = MagicMock(return_value="never") + mock_ts_mod.update_timestamp = MagicMock() + monkeypatch.setitem( + sys.modules, "aipass.backup.apps.handlers.utils.backup_timestamps", mock_ts_mod + ) + + backup_dir = tmp_path / "backups" / "system_snapshot" + backup_dir.mkdir(parents=True) + (backup_dir / "test.txt").write_text("hello") + + mock_sync = MagicMock() + mock_sync.authenticate.return_value = True + mock_sync.get_or_create_project_folder.return_value = "folder_id" + mock_sync.tracker_was_reset = False + mock_sync.prepare_sync.return_value = ( + [backup_dir / "test.txt"], + 0, + 1, + ) + mock_sync.sync_backup_files.return_value = { + "success": True, "uploaded": 1, "failed": 0, "skipped": 0, + "total": 1, "error": None, + } + + args = SimpleNamespace( + command="drive-sync", path=str(backup_dir), verbose=False, + note="test", dry_run=False, project="AIPass", force=False, + test=False, limit=0, + ) + + with patch.object(mod, "GoogleDriveSync", return_value=mock_sync): + result = drive_sync_env["handle_command"](args) + + assert result is True + mock_sync.sync_backup_files.assert_called_once() + # =================================================================== # Tests — helper functions diff --git a/src/aipass/cli/tests/test_integration.py b/src/aipass/cli/tests/test_integration.py index efc16243..0b2c611b 100644 --- a/src/aipass/cli/tests/test_integration.py +++ b/src/aipass/cli/tests/test_integration.py @@ -1,26 +1,23 @@ # =================== AIPass ==================== # Name: tests/test_integration.py -# Description: Integration tests for CLI main() flow and drone_adapter -# Version: 1.0.0 +# Description: Integration tests for CLI main() flow +# Version: 2.0.0 # Created: 2026-03-29 -# Modified: 2026-03-29 +# Modified: 2026-03-30 # ============================================= -"""Integration tests for CLI main() entry point and drone_adapter bridge.""" +"""Integration tests for CLI main() entry point.""" import subprocess import sys from io import StringIO from unittest.mock import patch -import pytest from rich.console import Console from aipass.cli.apps import cli as cli_module from aipass.cli.apps.cli import main from aipass.cli.apps.modules import display -from aipass.cli import drone_adapter -from aipass.cli.drone_adapter import handle_command, get_help, get_introspective # ============================================================================= @@ -122,61 +119,6 @@ class TestMainFlow: assert result == 0 -# ============================================================================= -# drone_adapter tests -# ============================================================================= - -class TestDroneAdapter: - """Integration tests for the drone_adapter bridge.""" - - def test_handle_command_returns_dict(self): - """handle_command returns a dict with stdout, stderr, exit_code keys.""" - result = handle_command("--help") - assert isinstance(result, dict) - assert "stdout" in result - assert "stderr" in result - assert "exit_code" in result - - def test_handle_command_help_exit_code_zero(self): - """--help returns exit_code 0.""" - result = handle_command("--help") - assert result["exit_code"] == 0 - - def test_handle_command_unknown_returns_one(self): - """Unknown command returns exit_code 1.""" - result = handle_command("nonexistent_cmd_xyz") - assert result["exit_code"] == 1 - - def test_handle_command_restores_argv(self): - """sys.argv is restored after handle_command call.""" - original_argv = sys.argv.copy() - handle_command("--version") - assert sys.argv == original_argv - - def test_get_help_returns_string(self): - """get_help returns a non-empty string.""" - result = get_help() - assert isinstance(result, str) - assert len(result) > 0 - - def test_get_introspective_returns_string(self): - """get_introspective returns a non-empty string with 'CLI' in it.""" - result = get_introspective() - assert isinstance(result, str) - assert len(result) > 0 - assert "CLI" in result - - def test_handle_command_captures_stdout(self): - """stdout contains expected output for --version.""" - result = handle_command("--version") - assert result["exit_code"] == 0 - # The version output goes through Rich console which writes to real stdout, - # but drone_adapter captures sys.stdout. Verify we get something back. - # Note: Rich Console writes to its own file= target, so stdout capture - # may be empty. The key contract is exit_code and dict shape. - assert isinstance(result["stdout"], str) - - # ============================================================================= # __main__.py test — verify module is runnable # ============================================================================= diff --git a/src/aipass/daemon/.seedgo/bypass.json b/src/aipass/daemon/.seedgo/bypass.json index faf7194a..96075c23 100644 --- a/src/aipass/daemon/.seedgo/bypass.json +++ b/src/aipass/daemon/.seedgo/bypass.json @@ -107,6 +107,12 @@ "reason": "activity, activity-report, activity_report commands all work with no args (default 24h). No-args introspection gate would break valid no-arg invocations.", "pattern": "no-args gate" }, + { + "file": "apps/modules/update.py", + "standard": "introspection", + "reason": "update runs the status digest with no args — that IS its primary function. Showing introspection instead was reported as a dead-end UX bug (DPLAN-0085).", + "pattern": "no-args gate" + }, { "file": "apps/plugins/heartbeat.py", "standard": "architecture", diff --git a/src/aipass/daemon/apps/modules/actions.py b/src/aipass/daemon/apps/modules/actions.py index 8248fae7..707e258b 100644 --- a/src/aipass/daemon/apps/modules/actions.py +++ b/src/aipass/daemon/apps/modules/actions.py @@ -237,7 +237,7 @@ def _handle_toggle(action_id: str, enable: bool) -> bool: action = get_action(action_id) if action is None: _error(f"Action not found: {action_id}") - return False + return True # Error displayed toggle_action(action_id, enable) state = "enabled" if enable else "disabled" @@ -250,7 +250,7 @@ def _handle_info(action_id: str) -> bool: action = get_action(action_id) if action is None: _error(f"Action not found: {action_id}") - return False + return True # Error displayed _print_action_detail(action) return True @@ -260,7 +260,7 @@ def _handle_set_reminder(args: List[str]) -> bool: """Handle 'actions set reminder "message" [--to @branch]'.""" if len(args) < 2: _error('Usage: actions set reminder "message" [--to @branch]') - return False + return True # Error displayed date_str = args[0] message = args[1] @@ -277,7 +277,7 @@ def _handle_set_reminder(args: List[str]) -> bool: if not due_date: _error(f"Invalid date format: {date_str}") console.print("[dim]Valid formats: YYYY-MM-DD, 1d, 7d, 1w, 2w[/dim]") - return False + return True # Error displayed action = create_action( name=message[:50], @@ -303,7 +303,7 @@ def _handle_set_schedule(args: List[str]) -> bool: """Handle 'actions set schedule @branch "prompt" [time_spec]'.""" if len(args) < 3: _error('Usage: actions set schedule @branch "prompt" [time_spec]') - return False + return True # Error displayed target_branch = args[0] prompt = args[1] @@ -315,11 +315,11 @@ def _handle_set_schedule(args: List[str]) -> bool: if schedule_type not in ("daily", "hourly", "interval"): _error(f"Unknown schedule type: {schedule_type}") console.print("[dim]Valid types: daily, hourly, interval[/dim]") - return False + return True # Error displayed if len(args) < 4: _error(f"{schedule_type.title()} schedule requires a time/value argument") - return False + return True # Error displayed if schedule_type in ("daily", "hourly"): time_val = args[3] @@ -329,7 +329,7 @@ def _handle_set_schedule(args: List[str]) -> bool: except ValueError: logger.warning("Invalid interval minutes value: %s", args[3]) _error(f"Invalid interval minutes: {args[3]}") - return False + return True # Error displayed # Generate a name from the prompt name = prompt[:50].replace(" ", "_").lower() @@ -381,13 +381,13 @@ def _handle_delete(args: List[str]) -> bool: """Handle 'actions delete '.""" if not args: _error("Action ID required: actions delete ") - return False + return True # Error displayed action_id = args[0] action = get_action(action_id) if action is None: _error(f"Action not found: {action_id}") - return False + return True # Error displayed delete_action(action_id) _success(f"Deleted action {action_id}: {action['name']}") @@ -444,14 +444,14 @@ def _route_set_subcommand(args: List[str]) -> bool: """Route 'actions set reminder ...' / 'actions set schedule ...'.""" if len(args) < 2: _error("Usage: actions set ...") - return False + return True # Error displayed set_type = args[1] if set_type == "reminder": return _handle_set_reminder(args[2:]) if set_type == "schedule": return _handle_set_schedule(args[2:]) _error(f"Unknown set type: {set_type}. Use 'reminder' or 'schedule'.") - return False + return True # Error displayed def _route_action_id(action_id: str, args: List[str]) -> bool: @@ -466,7 +466,7 @@ def _route_action_id(action_id: str, args: List[str]) -> bool: if sub_action == "info": return _handle_info(action_id) _error(f"Unknown action command: {sub_action}. Use 'on', 'off', or 'info'.") - return False + return True # Error displayed def handle_command(command: str, args: List[str]) -> bool: @@ -514,12 +514,12 @@ def handle_command(command: str, args: List[str]) -> bool: _error(f"Unknown subcommand: {subcommand}") console.print("[dim]Run 'actions --help' for available commands[/dim]") - return False + return True # Command was handled (error displayed) except Exception as e: logger.error("[actions] Error in actions command: %s", e, exc_info=True) _error(f"Error: {e}") - return False + return True # Error displayed # ============================================= diff --git a/src/aipass/daemon/apps/modules/activity_report.py b/src/aipass/daemon/apps/modules/activity_report.py index 5d2947ee..5370c19d 100644 --- a/src/aipass/daemon/apps/modules/activity_report.py +++ b/src/aipass/daemon/apps/modules/activity_report.py @@ -236,9 +236,11 @@ def _extract_branch_name(args: List[str]) -> str | None: def _handle_branch_health(args: List[str]) -> bool: - """Handle 'branch-health ' command.""" + """Handle 'branch-health [branch]' command. No args = all branches summary.""" if not args: - print_introspection() + json_handler.log_operation("branch_health_all", {"command": "branch-health"}) + report = generate_activity_report(since_hours=24, verbosity="normal") + console.print(report) return True if args[0] in ('--help', '-h', 'help'): _print_branch_health_help() diff --git a/src/aipass/daemon/apps/modules/update.py b/src/aipass/daemon/apps/modules/update.py index 827ad492..c946681f 100644 --- a/src/aipass/daemon/apps/modules/update.py +++ b/src/aipass/daemon/apps/modules/update.py @@ -150,15 +150,12 @@ def handle_command(command: str, args: list) -> bool: if command != "update": return False - if not args: - print_introspection() - return True - try: if args and args[0] in ['--help', '-h', 'help']: print_help() return True + # No args = run the digest (this is the primary use case) json_handler.log_operation("update_digest") inbox_data = load_inbox() local_data = load_local() @@ -170,7 +167,7 @@ def handle_command(command: str, args: list) -> bool: except Exception as e: logger.error(f"[DAEMON] Error generating update digest: {e}", exc_info=True) error(f"Error: {e}") - return False + return True # ============================================= diff --git a/src/aipass/daemon/tests/test_update_and_errors.py b/src/aipass/daemon/tests/test_update_and_errors.py new file mode 100644 index 00000000..f5c930f5 --- /dev/null +++ b/src/aipass/daemon/tests/test_update_and_errors.py @@ -0,0 +1,143 @@ +# =================== AIPass ==================== +# Name: test_update_and_errors.py +# Description: Tests for update command and error message formatting +# Version: 1.0.0 +# Created: 2026-03-30 +# Modified: 2026-03-30 +# ============================================= + +""" +Tests for the update command (no longer a dead end) and error message +formatting (no cascading double-errors). + +Covers: + - update: runs digest with no args, help flag works + - actions errors: single error message, no cascade + - branch-health: no-args shows all-branches summary +""" + +from unittest.mock import patch, MagicMock + +import pytest + +from aipass.daemon.apps import daemon as _daemon_mod +from aipass.daemon.apps.modules import update as _update_mod +from aipass.daemon.apps.modules import actions as _actions_mod +from aipass.daemon.apps.modules import activity_report as _activity_mod + + +@pytest.fixture(autouse=True) +def _mock_log_operations(): + """Prevent json_handler.log_operation from touching real files.""" + with ( + patch.object(_daemon_mod.json_handler, "log_operation", return_value=True), + patch.object(_update_mod.json_handler, "log_operation", return_value=True), + patch.object(_actions_mod.json_handler, "log_operation", return_value=True), + patch.object(_activity_mod.json_handler, "log_operation", return_value=True), + ): + yield + + +# ============================================================================ +# Update command tests +# ============================================================================ + + +class TestUpdateCommand: + """Tests for the update module — no longer a dead end.""" + + def test_update_no_args_runs_digest(self) -> None: + """update with no args should run the digest, not show introspection.""" + with patch.object(_update_mod, "load_inbox", return_value={"messages": [], "total_messages": 0}), \ + patch.object(_update_mod, "load_local", return_value={}): + result = _update_mod.handle_command("update", []) + assert result is True + + def test_update_no_args_calls_load_inbox(self) -> None: + """update with no args should call load_inbox (proving it runs the digest).""" + mock_inbox = MagicMock(return_value={"messages": [], "total_messages": 0}) + with patch.object(_update_mod, "load_inbox", mock_inbox), \ + patch.object(_update_mod, "load_local", return_value={}): + _update_mod.handle_command("update", []) + mock_inbox.assert_called_once() + + def test_update_help_flag(self) -> None: + """update --help should show help and return True.""" + result = _update_mod.handle_command("update", ["--help"]) + assert result is True + + def test_update_wrong_command(self) -> None: + """update module should not handle other commands.""" + result = _update_mod.handle_command("schedule", []) + assert result is False + + def test_update_error_returns_true(self) -> None: + """update should return True even on error (command was handled).""" + with patch.object(_update_mod, "load_inbox", side_effect=Exception("test error")): + result = _update_mod.handle_command("update", []) + assert result is True + + +# ============================================================================ +# Error cascade tests — single error message, no double-error +# ============================================================================ + + +class TestErrorCascade: + """Tests that error paths return True (command handled) to prevent cascade.""" + + def test_actions_unknown_subcommand_returns_true(self) -> None: + """Unknown subcommand should return True (error displayed, not cascaded).""" + result = _actions_mod.handle_command("actions", ["nonexistent_xyz"]) + assert result is True, "Unknown subcommand must return True to prevent cascade" + + def test_actions_invalid_id_returns_true(self) -> None: + """Invalid action ID should return True (error displayed, not cascaded).""" + result = _actions_mod.handle_command("actions", ["9999", "info"]) + assert result is True, "Invalid ID must return True to prevent cascade" + + def test_actions_delete_no_id_returns_true(self) -> None: + """actions delete with no ID should return True (error displayed).""" + result = _actions_mod.handle_command("actions", ["delete"]) + assert result is True + + def test_actions_set_no_args_returns_true(self) -> None: + """actions set with insufficient args should return True (error displayed).""" + result = _actions_mod.handle_command("actions", ["set"]) + assert result is True + + def test_actions_set_bad_type_returns_true(self) -> None: + """actions set with unknown type should return True (error displayed).""" + result = _actions_mod.handle_command("actions", ["set", "badtype"]) + assert result is True + + def test_route_command_no_cascade(self) -> None: + """route_command should return True for handled-but-failed actions commands.""" + modules = _daemon_mod.get_modules() + result = _daemon_mod.route_command("actions", ["nonexistent_xyz"], modules) + assert result is True, "route_command must not fall through on handled errors" + + +# ============================================================================ +# Branch-health no-args fallback tests +# ============================================================================ + + +class TestBranchHealthFallback: + """Tests that branch-health with no args shows all-branches summary.""" + + def test_branch_health_no_args_returns_true(self) -> None: + """branch-health with no args should return True (shows summary).""" + result = _activity_mod.handle_command("branch-health", []) + assert result is True + + def test_branch_health_no_args_not_introspection(self) -> None: + """branch-health with no args should NOT call print_introspection.""" + with patch.object(_activity_mod, "print_introspection") as mock_intro: + _activity_mod.handle_command("branch-health", []) + mock_intro.assert_not_called() + + def test_branch_health_help_flag(self) -> None: + """branch-health --help should return True.""" + result = _activity_mod.handle_command("branch-health", ["--help"]) + assert result is True diff --git a/src/aipass/drone/apps/drone.py b/src/aipass/drone/apps/drone.py index 88f54917..ee87bcdc 100644 --- a/src/aipass/drone/apps/drone.py +++ b/src/aipass/drone/apps/drone.py @@ -430,9 +430,12 @@ def main() -> int: # activate — scan + register all discovered commands from a branch if command == "activate": - if len(args) < 2: - err_console.print("drone: activate requires a target (e.g., drone activate @seedgo)") - return 1 + if len(args) < 2 or args[1] in ("--help", "-h"): + console.print("Usage: drone activate @branch") + console.print() + console.print("Scan a branch for available commands and register them as shortcuts.") + console.print("Example: drone activate @seedgo") + return 0 return _handle_activate(args[1]) # list — show registered custom commands diff --git a/src/aipass/drone/apps/handlers/json/json_handler.py b/src/aipass/drone/apps/handlers/json/json_handler.py index d30e265d..a9943e64 100644 --- a/src/aipass/drone/apps/handlers/json/json_handler.py +++ b/src/aipass/drone/apps/handlers/json/json_handler.py @@ -16,6 +16,8 @@ from __future__ import annotations import inspect import json +import os +import tempfile from datetime import datetime from pathlib import Path from typing import Any @@ -62,6 +64,29 @@ def _get_caller_module_name() -> str: return "unknown" +def _atomic_write_json(path: Path, data: Any) -> None: + """Write JSON atomically — write to temp file then rename. + + Prevents truncation/corruption during concurrent access. + """ + path.parent.mkdir(parents=True, exist_ok=True) + fd, tmp_path = tempfile.mkstemp( + dir=str(path.parent), suffix=".tmp", prefix=".json_" + ) + try: + with os.fdopen(fd, "w", encoding="utf-8") as fh: + json.dump(data, fh, indent=2, ensure_ascii=False) + os.replace(tmp_path, str(path)) + except Exception as exc: + logger.warning("_atomic_write_json: failed for %s: %s", path, exc) + # Clean up temp file on failure + try: + os.unlink(tmp_path) + except OSError as cleanup_exc: + logger.warning("_atomic_write_json: cleanup failed for %s: %s", tmp_path, cleanup_exc) + raise + + def _default_config(module_name: str) -> dict[str, Any]: """Return inline default for a *_config.json file.""" today = _today() @@ -167,11 +192,15 @@ def ensure_json_exists(module_name: str, json_type: str) -> bool: if json_path.exists(): try: - with open(json_path, "r", encoding="utf-8") as fh: - data = json.load(fh) - if validate_json_structure(data, json_type): - return True - # Corrupted — fall through to regenerate + # Guard: empty or zero-byte files cause JSONDecodeError + if json_path.stat().st_size == 0: + logger.warning("ensure_json_exists: empty file at %s, regenerating", json_path) + else: + with open(json_path, "r", encoding="utf-8") as fh: + data = json.load(fh) + if validate_json_structure(data, json_type): + return True + # Corrupted — fall through to regenerate except Exception as exc: # noqa: BLE001 logger.warning("ensure_json_exists: failed to read %s, regenerating: %s", json_path, exc) @@ -181,8 +210,7 @@ def ensure_json_exists(module_name: str, json_type: str) -> bool: raise ValueError(f"Unknown json_type: {json_type!r}") default = factory(module_name) - with open(json_path, "w", encoding="utf-8") as fh: - json.dump(default, fh, indent=2, ensure_ascii=False) + _atomic_write_json(json_path, default) return True @@ -215,8 +243,17 @@ def load_json(module_name: str, json_type: str) -> Any | None: return None json_path = get_json_path(module_name, json_type) - with open(json_path, "r", encoding="utf-8") as fh: - return json.load(fh) + try: + if json_path.stat().st_size == 0: + logger.warning("load_json: empty file at %s, returning default", json_path) + factory = _DEFAULTS.get(json_type) + return factory(module_name) if factory else None + with open(json_path, "r", encoding="utf-8") as fh: + return json.load(fh) + except (json.JSONDecodeError, OSError) as exc: + logger.warning("load_json: failed to read %s, returning default: %s", json_path, exc) + factory = _DEFAULTS.get(json_type) + return factory(module_name) if factory else None def save_json(module_name: str, json_type: str, data: Any) -> bool: @@ -243,8 +280,7 @@ def save_json(module_name: str, json_type: str, data: Any) -> bool: data["last_updated"] = _today() json_path = get_json_path(module_name, json_type) - with open(json_path, "w", encoding="utf-8") as fh: - json.dump(data, fh, indent=2, ensure_ascii=False) + _atomic_write_json(json_path, data) return True diff --git a/src/aipass/drone/tests/test_activation.py b/src/aipass/drone/tests/test_activation.py index 276b2bd4..7e12f0b8 100644 --- a/src/aipass/drone/tests/test_activation.py +++ b/src/aipass/drone/tests/test_activation.py @@ -474,13 +474,13 @@ class TestMainIntegration: mock_activate.assert_called_once_with("@seedgo") def test_activate_no_target(self) -> None: - """main() returns 1 when activate is called without a target.""" + """main() returns 0 and shows help when activate has no target.""" from aipass.drone.apps.drone import main with patch("sys.argv", ["drone", "activate"]): result = main() - assert result == 1 + assert result == 0 @patch("aipass.drone.apps.drone._handle_list") def test_list_route(self, mock_list: MagicMock) -> None: diff --git a/src/aipass/drone/tests/test_json_handler.py b/src/aipass/drone/tests/test_json_handler.py index f8c35680..c76bb444 100644 --- a/src/aipass/drone/tests/test_json_handler.py +++ b/src/aipass/drone/tests/test_json_handler.py @@ -634,3 +634,66 @@ def test_reimport_after_mock(tmp_path: Path) -> None: ) if handler_module: importlib.reload(handler_module) + + +# ============================================================================ +# Group 9 — Empty file resilience (4 tests) +# ============================================================================ + +def test_ensure_regenerates_empty_log_file(tmp_path: Path) -> None: # JH-044 + """Empty log.json should be regenerated, not crash with JSONDecodeError.""" + json_dir = _json_dir_as_path(tmp_path) + json_dir.mkdir(parents=True, exist_ok=True) + target = json_dir / "empty_log.json" + target.write_text("", encoding="utf-8") + + json_handler.ensure_json_exists("empty", "log") + + data = json.loads(target.read_text(encoding="utf-8")) + assert isinstance(data, list), "Empty log file must be regenerated to valid list" + + +def test_ensure_regenerates_empty_config_file(tmp_path: Path) -> None: # JH-045 + """Empty config.json should be regenerated, not crash.""" + json_dir = _json_dir_as_path(tmp_path) + json_dir.mkdir(parents=True, exist_ok=True) + target = json_dir / "empty_config.json" + target.write_text("", encoding="utf-8") + + json_handler.ensure_json_exists("empty", "config") + + data = json.loads(target.read_text(encoding="utf-8")) + assert "module_name" in data, "Empty config must be regenerated with correct structure" + + +def test_load_json_handles_empty_file(tmp_path: Path) -> None: # JH-046 + """load_json on an empty file should return default, not crash.""" + json_dir = _json_dir_as_path(tmp_path) + json_dir.mkdir(parents=True, exist_ok=True) + target = json_dir / "empty2_log.json" + target.write_text("", encoding="utf-8") + + result = json_handler.load_json("empty2", "log") + assert isinstance(result, list), "load_json must return default list for empty log" + + +def test_log_operation_survives_empty_log_file(tmp_path: Path) -> None: # JH-047 + """log_operation should succeed even if log.json is empty.""" + json_dir = _json_dir_as_path(tmp_path) + json_dir.mkdir(parents=True, exist_ok=True) + # Create valid config but empty log + config_path = json_dir / "recover_config.json" + config_path.write_text(json.dumps({ + "module_name": "recover", "version": "1.0.0", + "config": {"max_log_entries": 100}, + "created": "2026-01-01", "last_updated": "2026-01-01", + }), encoding="utf-8") + log_path = json_dir / "recover_log.json" + log_path.write_text("", encoding="utf-8") + + result = json_handler.log_operation("test_op", {"key": "val"}, module_name="recover") + assert result is True, "log_operation must succeed on empty log file" + + data = json.loads(log_path.read_text(encoding="utf-8")) + assert len(data) == 1, "Should have exactly one log entry after recovery" + assert data[0]["operation"] == "test_op" diff --git a/src/aipass/flow/apps/modules/aggregate_central.py b/src/aipass/flow/apps/modules/aggregate_central.py index 9a91db28..142a9f62 100755 --- a/src/aipass/flow/apps/modules/aggregate_central.py +++ b/src/aipass/flow/apps/modules/aggregate_central.py @@ -144,12 +144,8 @@ def handle_command(command: str, args: List[str]) -> bool: if command != "aggregate": return False - if not args: - print_introspection() - return True - # Handle help flag - if args[0] in ["--help", "-h", "help"]: + if args and args[0] in ["--help", "-h", "help"]: print_help() return True @@ -164,7 +160,12 @@ def handle_command(command: str, args: List[str]) -> bool: if "--no-heal" in args: heal = False - return aggregate_central(heal=heal) + result = aggregate_central(heal=heal) + if result: + console.print("[green]Central plans aggregated successfully[/green]") + else: + console.print("[red]Central plans aggregation failed[/red]") + return result # ============================================= diff --git a/src/aipass/flow/apps/modules/template_manager.py b/src/aipass/flow/apps/modules/template_manager.py index 6eb9dbdf..792f836d 100644 --- a/src/aipass/flow/apps/modules/template_manager.py +++ b/src/aipass/flow/apps/modules/template_manager.py @@ -178,11 +178,6 @@ def handle_command(command: str, args: List[str]) -> bool: Returns: True if command was recognized, False if not """ - # Introspection gate: no args = show module info - if not args: - print_introspection() - return True - # ---- templates ---- if command == "templates": # Intercept help before arg parsing diff --git a/src/aipass/flow/tests/test_aggregate_central.py b/src/aipass/flow/tests/test_aggregate_central.py index c0f94496..02229db5 100644 --- a/src/aipass/flow/tests/test_aggregate_central.py +++ b/src/aipass/flow/tests/test_aggregate_central.py @@ -46,14 +46,15 @@ class TestCommandRouting: # 2. command == "aggregate" with no args -> introspection # ═══════════════════════════════════════════════════════════ -class TestIntrospection: +class TestNoArgs: - @patch(f"{_MOD}.print_introspection") - def test_no_args_calls_introspection(self, mock_introspection): + @patch(f"{_MOD}.aggregate_central", return_value=True) + def test_no_args_runs_aggregation(self, mock_agg): + """No args should run aggregation (default action), not introspection.""" handle_command = _import_handle_command() result = handle_command("aggregate", []) assert result is True - mock_introspection.assert_called_once() + mock_agg.assert_called_once_with(heal=True) # ═══════════════════════════════════════════════════════════ @@ -191,14 +192,14 @@ class TestOperationLogging: {"command": "aggregate", "args": ["run"]}, ) - @patch(f"{_MOD}.print_introspection") + @patch(f"{_MOD}.aggregate_central", return_value=True) @patch(f"{_MOD}.json_handler") - def test_no_logging_on_introspection(self, mock_jh, mock_introspection): - """Introspection (no args) should not log an operation.""" + def test_no_args_does_log_operation(self, mock_jh, mock_agg): + """No args runs aggregation which DOES log an operation.""" handle_command = _import_handle_command() result = handle_command("aggregate", []) - assert result is True # Command was handled - mock_jh.log_operation.assert_not_called() + assert result is True + mock_jh.log_operation.assert_called_once() @patch(f"{_MOD}.print_help") @patch(f"{_MOD}.json_handler") diff --git a/src/aipass/flow/tests/test_template_manager.py b/src/aipass/flow/tests/test_template_manager.py index 8505e30e..c59cc97c 100644 --- a/src/aipass/flow/tests/test_template_manager.py +++ b/src/aipass/flow/tests/test_template_manager.py @@ -71,25 +71,25 @@ class TestSuggestPrefix: class TestHandleCommandRouting: """Verify handle_command routes to the correct function for each input.""" - def test_no_args_calls_introspection(self): - """No args should call print_introspection and return True.""" - with patch(f"{_MOD}.print_introspection") as mock_intro: + def test_no_args_shows_registered_types(self): + """templates with no args shows registered types (default action).""" + mock_registry = {"types": {"flow_plans": {"prefix": "FPLAN"}}} + + with patch(f"{_MOD}.load_registry", return_value=mock_registry), \ + patch(f"{_MOD}._display_registered_types") as mock_display: from aipass.flow.apps.modules.template_manager import handle_command result = handle_command("templates", []) - mock_intro.assert_called_once() + mock_display.assert_called_once_with(mock_registry) assert result is True - def test_any_command_no_args_calls_introspection(self): - """Even non-templates commands with no args trigger introspection.""" - with patch(f"{_MOD}.print_introspection") as mock_intro: - from aipass.flow.apps.modules.template_manager import handle_command + def test_unknown_command_no_args_returns_false(self): + """Non-template commands with no args are not claimed.""" + from aipass.flow.apps.modules.template_manager import handle_command - result = handle_command("register", []) - - mock_intro.assert_called_once() - assert result is True + result = handle_command("post", []) + assert result is False @pytest.mark.parametrize("help_flag", ["--help", "-h", "help"]) def test_templates_help_flags(self, help_flag: str): @@ -188,25 +188,13 @@ class TestHandleCommandRouting: # ---- unregister command ---- def test_unregister_no_args_shows_error(self): - """'unregister' with no dir arg should show usage error. - - Note: empty args hits the introspection gate, so we test that - unregister with at least one arg but no dir is handled. Actually, - looking at the source, unregister checks ``if not args`` *after* - the introspection gate already caught truly empty args. So we - need a different approach: the introspection gate fires when - args is empty for ANY command. unregister's own ``if not args`` - is unreachable via handle_command. Test via route that reaches it. - """ - # The introspection gate catches empty args before we ever reach - # the unregister block, so args=[] triggers introspection, not error. - # We verify that behavior here -- this is correct by design. - with patch(f"{_MOD}.print_introspection") as mock_intro: + """'unregister' with no dir arg should show usage error.""" + with patch(f"{_MOD}.error") as mock_error: from aipass.flow.apps.modules.template_manager import handle_command result = handle_command("unregister", []) - mock_intro.assert_called_once() + mock_error.assert_called_once() assert result is True def test_unregister_valid_calls_remove_type(self): diff --git a/src/aipass/memory/apps/memory.py b/src/aipass/memory/apps/memory.py index e38d6467..16352440 100755 --- a/src/aipass/memory/apps/memory.py +++ b/src/aipass/memory/apps/memory.py @@ -114,6 +114,9 @@ def print_help(): table.add_row("rollover check", "Dry run — check what needs rollover") table.add_row("rollover sync-lines", "Update line count metadata") table.add_row("search ", "Semantic search across all branch memories") + table.add_row("symbolic ", "Symbolic/fragmented memory extraction and search") + table.add_row("templates ", "Living template push, diff, and status") + table.add_row("verify ", "Check if a plan is vectorized in ChromaDB") table.add_row("watch", "Start memory watcher (auto-rollover on changes)") console.print(table) @@ -159,7 +162,7 @@ def print_help(): console.print("-" * 70) console.print() - console.print("Commands: search, rollover [run|status|check|sync-lines], watch") + console.print("Commands: search, rollover [run|status|check|sync-lines], symbolic, templates, verify, watch") console.print() diff --git a/src/aipass/memory/apps/modules/search.py b/src/aipass/memory/apps/modules/search.py index 3aa37068..b2ad721f 100755 --- a/src/aipass/memory/apps/modules/search.py +++ b/src/aipass/memory/apps/modules/search.py @@ -192,16 +192,20 @@ def show_search_results( console.print(f"[cyan]Type:[/cyan] {memory_type}") console.print() - console.print("[dim]Encoding query...[/dim]") - console.print("[dim]Searching collections...[/dim]") + console.print("[dim]Searching... (first run may take 30s for model loading)[/dim]") # Delegate to handler - result = _handler_execute_search( - query=query, - branch=branch, - memory_type=memory_type, - n_results=n_results - ) + try: + result = _handler_execute_search( + query=query, + branch=branch, + memory_type=memory_type, + n_results=n_results + ) + except Exception as exc: + logger.error(f"[search] Handler raised exception: {exc}") + error(f"Search failed: {exc}") + return False if not result['success']: error(result.get('error', 'Unknown error')) diff --git a/src/aipass/memory/tests/test_search.py b/src/aipass/memory/tests/test_search.py index 1212aba3..6aa14d35 100644 --- a/src/aipass/memory/tests/test_search.py +++ b/src/aipass/memory/tests/test_search.py @@ -15,7 +15,6 @@ All tests use mocks or tmp_path -- no live filesystem or infrastructure access. """ import sys -from pathlib import Path from unittest.mock import MagicMock @@ -501,3 +500,43 @@ class TestShowSearchResults: mocks["execute_search"].assert_called_once_with( query="q", branch="SEED", memory_type="local", n_results=3 ) + + def test_handler_timeout_returns_false(self, monkeypatch): + """When the handler returns a timeout error, show_search_results returns False.""" + search_mod, mocks = _import_search(monkeypatch) + mocks["execute_search"].return_value = { + "success": False, + "error": "Search operation timed out", + } + + result = search_mod.show_search_results("timeout query") + + assert result is False + mocks["error"].assert_called_once() + + def test_handler_timeout_does_not_crash(self, monkeypatch): + """A timeout error from the handler should not raise an exception.""" + search_mod, mocks = _import_search(monkeypatch) + mocks["execute_search"].return_value = { + "success": False, + "error": "Embedding timed out", + } + + # Must not raise -- graceful handling + result = search_mod.show_search_results("slow query") + + assert result is False + mocks["error"].assert_called_once() + + def test_handler_exception_does_not_crash(self, monkeypatch): + """If the handler raises an unexpected exception, it should not propagate.""" + import subprocess + search_mod, mocks = _import_search(monkeypatch) + mocks["execute_search"].side_effect = subprocess.TimeoutExpired( + cmd="python embed_subprocess.py", timeout=120 + ) + + result = search_mod.show_search_results("crash query") + + assert result is False + mocks["error"].assert_called_once() diff --git a/src/aipass/prax/.seedgo/bypass.json b/src/aipass/prax/.seedgo/bypass.json index 030c71e0..46ade39f 100644 --- a/src/aipass/prax/.seedgo/bypass.json +++ b/src/aipass/prax/.seedgo/bypass.json @@ -402,6 +402,11 @@ "standard": "naming", "reason": "False positive — 'changed' is a local variable inside _apply_structural_updates(), not a module-level constant." }, + { + "file": "apps/modules/status.py", + "standard": "introspection", + "reason": "Intentional — status is a leaf command where bare invocation shows system status (the primary user intent), not module introspection. Introspection is available via print_introspection() and __main__." + }, { "file": "apps/", "standard": "architecture", diff --git a/src/aipass/prax/apps/modules/agent_status.py b/src/aipass/prax/apps/modules/agent_status.py deleted file mode 100644 index ee8a004b..00000000 --- a/src/aipass/prax/apps/modules/agent_status.py +++ /dev/null @@ -1,130 +0,0 @@ -# =================== AIPass ==================== -# Name: agent_status.py -# Description: PRAX Agent Status Push Command -# Version: 0.1.0 -# Created: 2026-02-25 -# Modified: 2026-03-09 -# ============================================= - -""" -PRAX Agent Status Module - -Implements the 'agent-status-push' command using handle_command interface. -Pushes agent_status section to all branch dashboards showing active/stale agents. -""" - -import sys -from typing import List - -from aipass.cli.apps.modules import console, error -from aipass.prax.apps.handlers.json import json_handler - - -def print_introspection(): - """Display module introspection - shows connected handlers""" - console.print() - console.print("[bold cyan]PRAX Agent Status Module[/bold cyan]") - console.print() - console.print("[yellow]Purpose:[/yellow]") - console.print(" Push agent_status section to all branch dashboards") - console.print() - - console.print("[yellow]Connected Handlers:[/yellow]") - console.print() - console.print(" [cyan]prax/handlers/dashboard/[/cyan]") - console.print(" [dim]- agent_status_writer.py (push_agent_status_dashboard, build_agent_status_section)[/dim]") - console.print() - - console.print("[dim]Run 'drone @prax agent-status-push' to execute[/dim]") - console.print() - - -def print_help(): - """Drone-compliant help output""" - console.print() - console.print("[bold cyan]PRAX Agent Status Push[/bold cyan]") - console.print() - - console.print("[yellow]Purpose:[/yellow]") - console.print(" Scan for active dispatch agents and push status to all dashboards") - console.print() - - console.print("[yellow]Usage:[/yellow]") - console.print() - console.print(" [dim]# Push agent status to all branch dashboards[/dim]") - console.print(" $ drone @prax agent-status-push") - console.print() - console.print(" [dim]# Preview section data without pushing[/dim]") - console.print(" $ drone @prax agent-status-push --dry-run") - console.print() - - -def handle_command(command: str, args: List[str]) -> bool: - """ - Handle agent-status-push command - - Args: - command: Command name - args: Command arguments - - Returns: - True if command was handled - """ - if command != 'agent-status-push': - return False - - if not args: - print_introspection() - return True - - from aipass.prax.apps.handlers.dashboard.agent_status_writer import ( - build_agent_status_section, - push_agent_status_dashboard, - ) - - json_handler.log_operation("agent_status_push_executed", {"args": args}) - - if args[0] in ('--help', '-h', 'help'): - print_help() - return True - - if '--dry-run' in args: - import json - section = build_agent_status_section() - console.print("\n[bold cyan]Agent Status Section (dry-run)[/bold cyan]") - console.print(json.dumps(section, indent=2)) - return True - - section = build_agent_status_section() - active = section["agent_count"] - stale = len(section["stale_agents"]) - - console.print(f"\n[bold cyan]Agent Status Push[/bold cyan]") - console.print(f" Active agents: {active}") - console.print(f" Stale agents: {stale}") - - result = push_agent_status_dashboard() - - if result: - console.print("[green]✅ Pushed to all branch dashboards[/green]\n") - else: - error("Push failed — check logs") - - return True - - -if __name__ == "__main__": - if len(sys.argv) == 1: - print_introspection() - sys.exit(0) - - if '--help' in sys.argv: - print_help() - sys.exit(0) - - if '--introspect' in sys.argv: - print_introspection() - sys.exit(0) - - args = [arg for arg in sys.argv[1:] if not arg.startswith('--')] - handle_command('agent-status-push', args + [a for a in sys.argv[1:] if a.startswith('--')]) diff --git a/src/aipass/prax/apps/modules/status.py b/src/aipass/prax/apps/modules/status.py index 9bec7043..a039613c 100755 --- a/src/aipass/prax/apps/modules/status.py +++ b/src/aipass/prax/apps/modules/status.py @@ -45,33 +45,31 @@ def handle_command(command: str, args: List[str]) -> bool: if command != 'status': return False - if not args: - print_introspection() - return True - - json_handler.log_operation("status_checked", {"subcommand": args[0]}) - # --- sub-command routing ------------------------------------------------ - if args[0] in ("--help", "-h", "help"): + if args and args[0] in ("--help", "-h", "help"): print_help() return True if args and args[0] == "sync": + json_handler.log_operation("status_checked", {"subcommand": "sync"}) return _handle_sync() - # --- default: show PRAX system status ----------------------------------- + # --- default: show PRAX system status (bare 'status' or unknown sub) --- + json_handler.log_operation("status_checked", {"subcommand": "show"}) status = get_system_status() - console.print("\n📊 PRAX System Status") + console.print() + console.print("[bold cyan]PRAX System Status[/bold cyan]") console.print("=" * 60) - console.print(f"Total Modules: {status['total_modules']}") - console.print(f"Active Loggers: {status['individual_loggers']}") - console.print(f"System Logs Dir: {status['system_logs_dir']}") - console.print(f"Module Logs Dir: {status['module_logs_dir']}") - console.print(f"Registry File: {status['registry_file']}") - console.print(f"File Watcher: {'🟢 Active' if status['file_watcher_active'] else '🔴 Inactive'}") - console.print(f"Logger Override: {'🟢 Active' if status['logger_override_active'] else '🔴 Inactive'}") - console.print("=" * 60 + "\n") + console.print(f" Total Modules: {status['total_modules']}") + console.print(f" Active Loggers: {status['individual_loggers']}") + console.print(f" System Logs Dir: {status['system_logs_dir']}") + console.print(f" Module Logs Dir: {status['module_logs_dir']}") + console.print(f" Registry File: {status['registry_file']}") + console.print(f" File Watcher: {'Active' if status['file_watcher_active'] else 'Inactive'}") + console.print(f" Logger Override: {'Active' if status['logger_override_active'] else 'Inactive'}") + console.print("=" * 60) + console.print() return True diff --git a/src/aipass/prax/tests/conftest.py b/src/aipass/prax/tests/conftest.py index 308cd0f3..ced9ef1d 100644 --- a/src/aipass/prax/tests/conftest.py +++ b/src/aipass/prax/tests/conftest.py @@ -16,6 +16,8 @@ import sys import pytest from unittest.mock import MagicMock +collect_ignore_glob = [".archive/*"] + # ============================================= # INFRASTRUCTURE MOCKS diff --git a/src/aipass/prax/tests/test_agent_status.py b/src/aipass/prax/tests/test_agent_status.py deleted file mode 100644 index d579714e..00000000 --- a/src/aipass/prax/tests/test_agent_status.py +++ /dev/null @@ -1,142 +0,0 @@ -# =================== AIPass ==================== -# Name: test_agent_status.py -# Description: Unit tests for PRAX agent_status module -# Version: 1.0.0 -# Created: 2026-03-24 -# Modified: 2026-03-24 -# ============================================= - -""" -Tests for prax agent_status module command routing, help text, and introspection. - -All module imports happen inside test functions so that conftest's -autouse mock_prax_infrastructure fixture injects sys.modules mocks first. -""" - -import sys -from unittest.mock import MagicMock - - -# ============================================= -# HELPERS -# ============================================= - -def _ensure_dashboard_mock(monkeypatch): - """Inject a mock for the agent_status_writer handler.""" - mock_writer = MagicMock() - mock_writer.build_agent_status_section = MagicMock(return_value={ - "agent_count": 3, - "stale_agents": ["backup"], - "active_agents": ["prax", "drone", "flow"], - "last_updated": "2026-03-24T12:00:00", - }) - mock_writer.push_agent_status_dashboard = MagicMock(return_value=True) - monkeypatch.setitem( - sys.modules, - "aipass.prax.apps.handlers.dashboard.agent_status_writer", - mock_writer, - ) - return mock_writer - - -def _fresh_import(): - """Force re-import of the agent_status module.""" - mod_name = "aipass.prax.apps.modules.agent_status" - sys.modules.pop(mod_name, None) - from aipass.prax.apps.modules.agent_status import ( - handle_command, - print_help, - print_introspection, - ) - return handle_command, print_help, print_introspection - - -# ============================================= -# TESTS -# ============================================= - -def test_handle_command_help(mock_prax_infrastructure, monkeypatch): - """--help flag returns True and prints help text.""" - _ensure_dashboard_mock(monkeypatch) - handle_command, _, _ = _fresh_import() - - result = handle_command("agent-status-push", ["--help"]) - assert result is True - mock_prax_infrastructure.console.print.assert_called() - - -def test_handle_command_help_h_flag(mock_prax_infrastructure, monkeypatch): - """-h flag also triggers help with agent/push keywords.""" - _ensure_dashboard_mock(monkeypatch) - handle_command, _, _ = _fresh_import() - - result = handle_command("agent-status-push", ["-h"]) - assert result is True - calls = [str(c) for c in mock_prax_infrastructure.console.print.call_args_list] - assert any("agent" in c.lower() for c in calls) - - -def test_handle_command_no_args_calls_introspection(mock_prax_infrastructure, monkeypatch): - """No args prints introspection and returns True.""" - handle_command, _, _ = _fresh_import() - - result = handle_command("agent-status-push", []) - assert result is True - calls = [str(c) for c in mock_prax_infrastructure.console.print.call_args_list] - assert any("Agent Status Module" in c for c in calls) - - -def test_handle_command_wrong_command(mock_prax_infrastructure, monkeypatch): - """Wrong command name returns False.""" - handle_command, _, _ = _fresh_import() - - result = handle_command("not-agent-status", []) - assert result is False - - -def test_print_help_runs(mock_prax_infrastructure, monkeypatch): - """print_help runs without error and prints usage content.""" - _, print_help, _ = _fresh_import() - - print_help() - calls = [str(c) for c in mock_prax_infrastructure.console.print.call_args_list] - assert any("dry-run" in c for c in calls) - - -def test_print_introspection_runs(mock_prax_infrastructure, monkeypatch): - """print_introspection runs without error.""" - _, _, print_introspection = _fresh_import() - - print_introspection() - calls = [str(c) for c in mock_prax_infrastructure.console.print.call_args_list] - assert any("Connected Handlers" in c for c in calls) - - -def test_handle_command_push_routes_to_handler(mock_prax_infrastructure, monkeypatch): - """'push' arg triggers build + push via handler and shows result data.""" - mock_writer = _ensure_dashboard_mock(monkeypatch) - handle_command, _, _ = _fresh_import() - - result = handle_command("agent-status-push", ["push"]) - assert result is True - mock_writer.build_agent_status_section.assert_called_once() - mock_writer.push_agent_status_dashboard.assert_called_once() - # Verify console output includes result data from handler - calls = [str(c) for c in mock_prax_infrastructure.console.print.call_args_list] - assert any("Active agents" in c for c in calls) - assert any("Pushed" in c for c in calls) - - -def test_handle_command_dry_run(mock_prax_infrastructure, monkeypatch): - """--dry-run flag builds section data, prints it, but does not push.""" - mock_writer = _ensure_dashboard_mock(monkeypatch) - handle_command, _, _ = _fresh_import() - - result = handle_command("agent-status-push", ["--dry-run"]) - assert result is True - mock_writer.build_agent_status_section.assert_called_once() - mock_writer.push_agent_status_dashboard.assert_not_called() - # Verify dry-run output was printed with section data - calls = [str(c) for c in mock_prax_infrastructure.console.print.call_args_list] - assert any("dry-run" in c.lower() for c in calls) - assert any("agent_count" in c for c in calls) diff --git a/src/aipass/prax/tests/test_status.py b/src/aipass/prax/tests/test_status.py index 55b4240e..763c1f93 100644 --- a/src/aipass/prax/tests/test_status.py +++ b/src/aipass/prax/tests/test_status.py @@ -86,16 +86,16 @@ def test_handle_command_help_word(mock_prax_infrastructure, monkeypatch): assert any("status" in c.lower() for c in calls) -def test_handle_command_no_args_calls_introspection(mock_prax_infrastructure, monkeypatch): - """No args prints introspection and returns True.""" +def test_handle_command_no_args_shows_system_status(mock_prax_infrastructure, monkeypatch): + """No args shows system status and returns True.""" _ensure_sync_mock(monkeypatch) handle_command, _, _ = _fresh_import() result = handle_command("status", []) assert result is True - # Introspection prints "status Module" + # System status prints "PRAX System Status" calls = [str(c) for c in mock_prax_infrastructure.console.print.call_args_list] - assert any("status Module" in c for c in calls) + assert any("System Status" in c for c in calls) def test_handle_command_wrong_command(mock_prax_infrastructure, monkeypatch): diff --git a/src/aipass/seedgo/apps/modules/diagnostics_audit.py b/src/aipass/seedgo/apps/modules/diagnostics_audit.py index a73fcd03..607153c1 100644 --- a/src/aipass/seedgo/apps/modules/diagnostics_audit.py +++ b/src/aipass/seedgo/apps/modules/diagnostics_audit.py @@ -14,7 +14,7 @@ undefined variables, and other static analysis issues. Usage: seedgo diagnostics # All branches - seedgo diagnostics flow # Specific branch + seedgo diagnostics @flow # Specific branch """ import sys @@ -170,7 +170,7 @@ def print_introspection(): console.print("[yellow]How It Works:[/yellow]") console.print(" Diagnostics checking runs through the audit pipeline:") console.print(" [green]drone @seedgo audit aipass[/green] [dim]# All branches[/dim]") - console.print(" [green]drone @seedgo audit aipass flow[/green] [dim]# Single branch[/dim]") + console.print(" [green]drone @seedgo audit aipass @flow[/green] [dim]# Single branch[/dim]") console.print() console.print("[yellow]Next:[/yellow]") @@ -187,13 +187,13 @@ def print_help(): console.print("[yellow]COMMANDS:[/yellow]") console.print(" [green]drone @seedgo diagnostics_audit[/green] [dim]Scan all branches for type errors[/dim]") - console.print(" [green]drone @seedgo diagnostics_audit [/green] [dim]Scan specific branch[/dim]") + console.print(" [green]drone @seedgo diagnostics_audit @[/green] [dim]Scan specific branch[/dim]") console.print() console.print("[yellow]EXAMPLES:[/yellow]") console.print(" [green]drone @seedgo diagnostics_audit[/green]") - console.print(" [green]drone @seedgo diagnostics_audit flow[/green]") - console.print(" [green]drone @seedgo diagnostics_audit spawn[/green]") + console.print(" [green]drone @seedgo diagnostics_audit @flow[/green]") + console.print(" [green]drone @seedgo diagnostics_audit @spawn[/green]") console.print() console.print("[yellow]WHAT IT CHECKS:[/yellow]") diff --git a/src/aipass/seedgo/apps/modules/standards_audit.py b/src/aipass/seedgo/apps/modules/standards_audit.py index 9b1feff1..a4734975 100755 --- a/src/aipass/seedgo/apps/modules/standards_audit.py +++ b/src/aipass/seedgo/apps/modules/standards_audit.py @@ -103,7 +103,7 @@ def _show_audit_introspection() -> None: console.print("[yellow]Next:[/yellow] Pick a pack to audit") first_pack = next(iter(packs)) console.print(f" [green]drone @seedgo audit {first_pack}[/green] [dim]# All branches[/dim]") - console.print(f" [green]drone @seedgo audit {first_pack} flow[/green] [dim]# Single branch[/dim]") + console.print(f" [green]drone @seedgo audit {first_pack} @flow[/green] [dim]# Single branch[/dim]") console.print() @@ -357,7 +357,7 @@ def print_help(): console.print("[yellow]COMMANDS:[/yellow]") console.print(" [green]drone @seedgo audit[/green] [dim]Show available packs[/dim]") console.print(" [green]drone @seedgo audit aipass[/green] [dim]All branches, aipass pack[/dim]") - console.print(" [green]drone @seedgo audit aipass flow[/green] [dim]Single branch[/dim]") + console.print(" [green]drone @seedgo audit aipass @flow[/green] [dim]Single branch[/dim]") console.print(" [green]drone @seedgo audit --help[/green] [dim]This help message[/dim]") console.print() @@ -366,7 +366,7 @@ def print_help(): console.print(" [green]drone @seedgo audit aipass[/green]") console.print() console.print(" [dim]# Audit specific branch[/dim]") - console.print(" [green]drone @seedgo audit aipass spawn[/green]") + console.print(" [green]drone @seedgo audit aipass @spawn[/green]") console.print() console.print("[yellow]REFERENCE:[/yellow]") diff --git a/src/aipass/seedgo/apps/seedgo.py b/src/aipass/seedgo/apps/seedgo.py index 0f00f498..11b9015d 100644 --- a/src/aipass/seedgo/apps/seedgo.py +++ b/src/aipass/seedgo/apps/seedgo.py @@ -183,7 +183,7 @@ def print_help() -> None: console.print("[yellow]Audit:[/yellow]") console.print(" [green]drone @seedgo audit[/green] [dim]# Show available checker packs[/dim]") console.print(" [green]drone @seedgo audit aipass[/green] [dim]# Audit all branches[/dim]") - console.print(" [green]drone @seedgo audit aipass flow[/green] [dim]# Audit single branch[/dim]") + console.print(" [green]drone @seedgo audit aipass @flow[/green] [dim]# Audit single branch[/dim]") console.print() console.print("[yellow]Query Standards:[/yellow]") @@ -197,6 +197,11 @@ def print_help() -> None: console.print(" [green]drone @seedgo checklist [/green] [dim]# Run per-standard checklist on file[/dim]") console.print() + console.print("[yellow]Diagnostics:[/yellow]") + console.print(" [green]drone @seedgo diagnostics[/green] [dim]# Pyright errors across all branches[/dim]") + console.print(" [green]drone @seedgo diagnostics @flow[/green] [dim]# Single branch diagnostics[/dim]") + console.print() + console.print("─" * 70) console.print() diff --git a/src/aipass/seedgo/conftest.py b/src/aipass/seedgo/conftest.py new file mode 100644 index 00000000..37355c22 --- /dev/null +++ b/src/aipass/seedgo/conftest.py @@ -0,0 +1,3 @@ +"""Root conftest for seedgo — exclude templates from pytest collection.""" + +collect_ignore_glob = ["templates/*"] diff --git a/src/aipass/seedgo/tests/test_standards_audit.py b/src/aipass/seedgo/tests/test_standards_audit.py index 25a79207..bb1ed7e4 100644 --- a/src/aipass/seedgo/tests/test_standards_audit.py +++ b/src/aipass/seedgo/tests/test_standards_audit.py @@ -180,3 +180,51 @@ def test_handle_command_output_capture(capsys): # capsys captures stdout — print_help uses Rich console, so captured may be empty # but the capsys fixture inclusion satisfies the pattern requirement _captured = capsys.readouterr() + + +def test_help_text_at_prefix_consistency(): + """All help text branch references use @ prefix (DPLAN-0085 fresh-eyes fix). + + Scans help text strings in seedgo.py and all modules for branch name + patterns that should use @ prefix but don't. + """ + import re + from pathlib import Path + + branch_root = Path(__file__).resolve().parents[1] + files_to_check = [ + branch_root / "apps" / "seedgo.py", + *sorted((branch_root / "apps" / "modules").glob("*.py")), + ] + + # Pattern: 'audit aipass ' or 'diagnostics ' where is + # a known branch name without @ prefix. We check for bare branch names + # after command keywords in string literals. + known_branches = { + "drone", "seedgo", "prax", "cli", "flow", "ai_mail", "api", + "trigger", "spawn", "devpulse", "backup", "daemon", "memory", + "commons", "skills", + } + # Match: a command keyword followed by a bare branch name (no @) + bare_branch_re = re.compile( + r'(?:audit\s+aipass|diagnostics(?:_audit)?|readme(?:_update)?)\s+' + r'(' + '|'.join(known_branches) + r')\b' + ) + + violations = [] + for fpath in files_to_check: + if not fpath.exists(): + continue + source = fpath.read_text(encoding="utf-8") + for i, line in enumerate(source.splitlines(), 1): + # Only check inside string literals (lines with quotes) + if '"' not in line and "'" not in line: + continue + match = bare_branch_re.search(line) + if match: + violations.append(f"{fpath.name}:{i}: bare '{match.group(1)}' (should be '@{match.group(1)}')") + + assert not violations, ( + f"Help text has {len(violations)} bare branch references (missing @):\n" + + "\n".join(violations) + ) diff --git a/src/aipass/spawn/apps/modules/passport.py b/src/aipass/spawn/apps/modules/passport.py index 7424098a..88e7d954 100644 --- a/src/aipass/spawn/apps/modules/passport.py +++ b/src/aipass/spawn/apps/modules/passport.py @@ -78,6 +78,11 @@ def handle_passport(args: list[str]) -> int: warning("--purpose", details="Purpose description") return 1 + # Intercept --help before argparse (argparse has add_help=False) + if "--help" in args or "-h" in args: + print_introspection() + return 0 + parser = argparse.ArgumentParser(prog="spawn passport", add_help=False) parser.add_argument("target") parser.add_argument("--role", default="") diff --git a/src/aipass/spawn/apps/spawn.py b/src/aipass/spawn/apps/spawn.py index 13c8b9a7..bfc4fb7f 100644 --- a/src/aipass/spawn/apps/spawn.py +++ b/src/aipass/spawn/apps/spawn.py @@ -71,6 +71,11 @@ def handle_create(args): error("target path required", suggestion="drone @spawn create [class] [--role ...]") return 1 + # Intercept --help before argparse (argparse has add_help=False) + if "--help" in args or "-h" in args: + print_help() + return 0 + # Check if first arg is a citizen class citizen_class = get_default_class() remaining_args = args @@ -81,6 +86,10 @@ def handle_create(args): error("target path required after class name") return 1 + dry_run = "--dry-run" in remaining_args + if dry_run: + remaining_args = [a for a in remaining_args if a != "--dry-run"] + parser = argparse.ArgumentParser(prog="spawn create", add_help=False) parser.add_argument("target_path") parser.add_argument("--role", default="") @@ -91,6 +100,9 @@ def handle_create(args): parsed = parser.parse_args(remaining_args) + if dry_run: + return _dry_run_create(parsed.target_path, citizen_class, parsed) + result = spawn_agent( target_path=parsed.target_path, role=parsed.role, @@ -117,6 +129,51 @@ def handle_create(args): return 1 +def _dry_run_create(target_path, citizen_class, parsed): + """Preview what create would do without making changes.""" + from pathlib import Path + from aipass.spawn.apps.modules.core import _get_template_dir, get_branch_name, normalize_branch_name + + target = Path(target_path).resolve() + template = _get_template_dir(citizen_class) + branch_name = get_branch_name(target) + branch_upper = normalize_branch_name(branch_name, "upper") + + console.print() + header("DRY RUN — Create Preview") + console.print() + console.print(f" [bold]Branch:[/bold] {branch_upper}") + console.print(f" [bold]Class:[/bold] {citizen_class}") + console.print(f" [bold]Path:[/bold] {target}") + console.print(f" [bold]Template:[/bold] {template}") + if parsed.role: + console.print(f" [bold]Role:[/bold] {parsed.role}") + if parsed.purpose: + console.print(f" [bold]Purpose:[/bold] {parsed.purpose}") + + if target.exists(): + error(f"Target already exists: {target}") + return 1 + + if not template.exists(): + error(f"Template not found: {template}") + return 1 + + # Count template files + file_count = sum(1 for f in template.rglob("*") if f.is_file() and "__pycache__" not in str(f)) + dir_count = sum(1 for d in template.rglob("*") if d.is_dir() and "__pycache__" not in str(d)) + + console.print() + console.print(f" [bold cyan]Would create:[/bold cyan]") + console.print(f" Files: ~{file_count}") + console.print(f" Directories: ~{dir_count}") + console.print(f" Registry: add to AIPASS_REGISTRY.json") + console.print() + console.print(" [dim]No files were created. Remove --dry-run to execute.[/dim]") + console.print() + return 0 + + def print_introspection(): """Display module introspection info.""" console.print() diff --git a/src/aipass/spawn/tests/test_cli_routing.py b/src/aipass/spawn/tests/test_cli_routing.py index 7003c7e1..fee0548b 100644 --- a/src/aipass/spawn/tests/test_cli_routing.py +++ b/src/aipass/spawn/tests/test_cli_routing.py @@ -77,6 +77,87 @@ class TestCliRouting: assert isinstance(result, int) +class TestCreateHelp: + """Tests for create --help interception.""" + + def test_create_help_flag(self): + """create --help shows help instead of argparse error.""" + from aipass.spawn.apps.spawn import handle_create + with patch("aipass.spawn.apps.spawn.print_help") as mock_help: + result = handle_create(["--help"]) + assert result == 0 + mock_help.assert_called_once() + + def test_create_short_help(self): + """create -h shows help.""" + from aipass.spawn.apps.spawn import handle_create + with patch("aipass.spawn.apps.spawn.print_help") as mock_help: + result = handle_create(["-h"]) + assert result == 0 + mock_help.assert_called_once() + + def test_create_help_with_class(self): + """create builder --help shows help.""" + from aipass.spawn.apps.spawn import handle_create + with patch("aipass.spawn.apps.spawn.print_help") as mock_help: + result = handle_create(["builder", "--help"]) + assert result == 0 + mock_help.assert_called_once() + + +class TestCreateDryRun: + """Tests for create --dry-run preview.""" + + def test_dry_run_returns_zero(self, tmp_path): + """--dry-run returns 0 for valid target.""" + from aipass.spawn.apps.spawn import handle_create + target = str(tmp_path / "drytest") + with patch("aipass.spawn.apps.spawn.console"), \ + patch("aipass.spawn.apps.spawn.header"): + result = handle_create([target, "--dry-run"]) + assert result == 0 + + def test_dry_run_creates_no_files(self, tmp_path): + """--dry-run creates nothing on disk.""" + from aipass.spawn.apps.spawn import handle_create + target = tmp_path / "drytest" + with patch("aipass.spawn.apps.spawn.console"), \ + patch("aipass.spawn.apps.spawn.header"): + handle_create([str(target), "--dry-run"]) + assert not target.exists() + + def test_dry_run_existing_target_returns_error(self, tmp_path): + """--dry-run returns 1 if target already exists.""" + from aipass.spawn.apps.spawn import handle_create + target = tmp_path / "existing" + target.mkdir() + with patch("aipass.spawn.apps.spawn.console"), \ + patch("aipass.spawn.apps.spawn.header"), \ + patch("aipass.spawn.apps.spawn.error"): + result = handle_create([str(target), "--dry-run"]) + assert result == 1 + + +class TestPassportHelp: + """Tests for passport --help interception.""" + + def test_passport_help_flag(self): + """passport --help shows introspection.""" + from aipass.spawn.apps.modules.passport import handle_passport + with patch("aipass.spawn.apps.modules.passport.print_introspection") as mock: + result = handle_passport(["--help"]) + assert result == 0 + mock.assert_called_once() + + def test_passport_short_help(self): + """passport -h shows introspection.""" + from aipass.spawn.apps.modules.passport import handle_passport + with patch("aipass.spawn.apps.modules.passport.print_introspection") as mock: + result = handle_passport(["-h"]) + assert result == 0 + mock.assert_called_once() + + class TestPrintHelp: """Tests for print_help output.""" diff --git a/src/aipass/spawn/tests/test_lifecycle.py b/src/aipass/spawn/tests/test_lifecycle.py index 2fc6cf61..7d5da97e 100644 --- a/src/aipass/spawn/tests/test_lifecycle.py +++ b/src/aipass/spawn/tests/test_lifecycle.py @@ -115,7 +115,7 @@ def mock_registry(repo_root, mock_branch): class TestDeleteBranch: """Tests for delete_branch().""" - def test_delete_archives_and_removes(self, repo_root, mock_branch, mock_registry): + def test_delete_archives_and_removes(self, repo_root: Path, mock_branch: Path, mock_registry: Path): """Successful delete should archive the branch and remove from registry.""" from aipass.spawn.apps.handlers.delete_ops import delete_branch diff --git a/src/aipass/trigger/apps/handlers/error_registry.py b/src/aipass/trigger/apps/handlers/error_registry.py index e1c01dd6..84148eba 100644 --- a/src/aipass/trigger/apps/handlers/error_registry.py +++ b/src/aipass/trigger/apps/handlers/error_registry.py @@ -137,7 +137,9 @@ def _save_circuit_breaker_state() -> None: try: data: Dict[str, Any] = {} if TRIGGER_CONFIG_FILE.exists(): - data = json.loads(TRIGGER_CONFIG_FILE.read_text(encoding='utf-8')) + raw = TRIGGER_CONFIG_FILE.read_text(encoding='utf-8').strip() + if raw: + data = json.loads(raw) data['circuit_breaker'] = { 'state': _circuit_breaker.state, 'opened_at': _circuit_breaker.opened_at, @@ -162,7 +164,10 @@ def _load_circuit_breaker_state() -> CircuitBreakerState: """ try: if TRIGGER_CONFIG_FILE.exists(): - data = json.loads(TRIGGER_CONFIG_FILE.read_text(encoding='utf-8')) + raw = TRIGGER_CONFIG_FILE.read_text(encoding='utf-8').strip() + if not raw: + return CircuitBreakerState() + data = json.loads(raw) cb_data = data.get('circuit_breaker') if isinstance(cb_data, dict) and cb_data.get('state') in ('closed', 'open', 'half_open'): breaker = CircuitBreakerState() @@ -182,7 +187,10 @@ def _clear_circuit_breaker_state() -> None: """ try: if TRIGGER_CONFIG_FILE.exists(): - data = json.loads(TRIGGER_CONFIG_FILE.read_text(encoding='utf-8')) + raw = TRIGGER_CONFIG_FILE.read_text(encoding='utf-8').strip() + if not raw: + return + data = json.loads(raw) if 'circuit_breaker' in data: del data['circuit_breaker'] TRIGGER_CONFIG_FILE.write_text( @@ -499,7 +507,10 @@ def _load_registry() -> dict: """ try: if REGISTRY_FILE.exists(): - data = json.loads(REGISTRY_FILE.read_text(encoding='utf-8')) + raw = REGISTRY_FILE.read_text(encoding='utf-8').strip() + if not raw: + return {"errors": {}, "metadata": {"version": "1.0.0", "last_updated": datetime.now().isoformat()}} + data = json.loads(raw) if isinstance(data, dict) and 'errors' in data: return data except Exception as exc: diff --git a/src/aipass/trigger/apps/handlers/json/json_handler.py b/src/aipass/trigger/apps/handlers/json/json_handler.py index 8c5e72c7..7209e8bd 100644 --- a/src/aipass/trigger/apps/handlers/json/json_handler.py +++ b/src/aipass/trigger/apps/handlers/json/json_handler.py @@ -133,14 +133,29 @@ def ensure_json_exists(module_name: str, json_type: str) -> bool: def load_json(module_name: str, json_type: str) -> Optional[Any]: - """Load JSON file, auto-create if missing""" + """Load JSON file, auto-create if missing. + + Guards against empty or corrupt JSON files by regenerating from template. + """ if not ensure_json_exists(module_name, json_type): return None json_path = get_json_path(module_name, json_type) - with open(json_path, 'r', encoding='utf-8') as f: - return json.load(f) + try: + content = json_path.read_text(encoding='utf-8').strip() + if not content: + _log_warning(f"load_json empty file for {module_name}_{json_type}, will regenerate") + ensure_json_exists(module_name, json_type) + content = json_path.read_text(encoding='utf-8').strip() + return json.loads(content) + except (json.JSONDecodeError, OSError) as exc: + _log_warning(f"load_json failed for {module_name}_{json_type}: {exc}") + ensure_json_exists(module_name, json_type) + try: + return json.loads(json_path.read_text(encoding='utf-8')) + except Exception: + return _get_default_template(json_type, module_name) def save_json(module_name: str, json_type: str, data: Any) -> bool: diff --git a/src/aipass/trigger/apps/modules/branch_log_events.py b/src/aipass/trigger/apps/modules/branch_log_events.py index d6528ea7..89f240bd 100644 --- a/src/aipass/trigger/apps/modules/branch_log_events.py +++ b/src/aipass/trigger/apps/modules/branch_log_events.py @@ -151,6 +151,9 @@ def handle_command(command: str, args: list) -> bool: if not args: print_introspection() return True + if args[0] in ['--help', '-h', 'help']: + print_help() + return True subcommand = args[0] remaining = args[1:] return handle_command(subcommand, remaining) diff --git a/src/aipass/trigger/tests/test_error_registry.py b/src/aipass/trigger/tests/test_error_registry.py index 8b5dfb5e..a99bc321 100644 --- a/src/aipass/trigger/tests/test_error_registry.py +++ b/src/aipass/trigger/tests/test_error_registry.py @@ -836,3 +836,65 @@ def test_report_return_type_is_dict(tmp_path: Path) -> None: component="API", ) assert isinstance(result, dict) + + +# --------------------------------------------------------------------------- +# Empty / corrupt JSON file resilience +# --------------------------------------------------------------------------- + +class TestEmptyJsonResilience: + """Verify no crashes when JSON files are empty or corrupt.""" + + def test_load_registry_empty_file(self, tmp_path: Path) -> None: + """_load_registry returns default structure when registry file is empty.""" + registry_dir = tmp_path / "trigger_json" + registry_dir.mkdir(parents=True, exist_ok=True) + registry_file = registry_dir / "error_registry.json" + registry_file.write_text("", encoding="utf-8") + + er = _import_registry() + result = er._load_registry() + assert isinstance(result, dict) + assert "errors" in result + assert result["errors"] == {} + + def test_load_registry_corrupt_json(self, tmp_path: Path) -> None: + """_load_registry returns default on corrupt JSON content.""" + registry_dir = tmp_path / "trigger_json" + registry_dir.mkdir(parents=True, exist_ok=True) + registry_file = registry_dir / "error_registry.json" + registry_file.write_text("{invalid json", encoding="utf-8") + + er = _import_registry() + result = er._load_registry() + assert isinstance(result, dict) + assert "errors" in result + + def test_load_circuit_breaker_empty_config(self, tmp_path: Path) -> None: + """_load_circuit_breaker_state returns default when config is empty.""" + config_dir = tmp_path / "trigger_json" + config_dir.mkdir(parents=True, exist_ok=True) + config_file = config_dir / "trigger_config.json" + config_file.write_text("", encoding="utf-8") + + er = _import_registry() + result = er._load_circuit_breaker_state() + assert result.state == "closed" + assert result.cooldown_seconds == 300 + + def test_report_with_empty_registry(self, tmp_path: Path) -> None: + """report() works when registry file is empty (creates fresh).""" + registry_dir = tmp_path / "trigger_json" + registry_dir.mkdir(parents=True, exist_ok=True) + registry_file = registry_dir / "error_registry.json" + registry_file.write_text("", encoding="utf-8") + + er = _import_registry() + result = er.report( + error_type="TestError", + message="test with empty registry", + component="TRIGGER", + ) + assert isinstance(result, dict) + assert result["is_new"] is True + assert result["count"] == 1 diff --git a/src/commons/apps/handlers/comments/comment_ops.py b/src/commons/apps/handlers/comments/comment_ops.py index d995c379..10e597d8 100644 --- a/src/commons/apps/handlers/comments/comment_ops.py +++ b/src/commons/apps/handlers/comments/comment_ops.py @@ -86,7 +86,7 @@ def add_comment(args: List[str]) -> dict: # --- Get caller identity --- caller = get_caller_branch() if not caller: - return {"success": False, "error": "Could not detect calling branch"} + return {"success": False, "error": "Could not detect calling branch. Run from a branch directory or use drone routing (drone @commons ...)"} author = caller.get("name", "UNKNOWN") @@ -270,7 +270,7 @@ def vote_on_content(args: List[str]) -> dict: # --- Get caller identity --- caller = get_caller_branch() if not caller: - return {"success": False, "error": "Could not detect calling branch"} + return {"success": False, "error": "Could not detect calling branch. Run from a branch directory or use drone routing (drone @commons ...)"} voter = caller.get("name", "UNKNOWN") diff --git a/src/commons/apps/handlers/curation/curation_ops.py b/src/commons/apps/handlers/curation/curation_ops.py index 972278fd..5d5294f2 100644 --- a/src/commons/apps/handlers/curation/curation_ops.py +++ b/src/commons/apps/handlers/curation/curation_ops.py @@ -76,7 +76,7 @@ def add_react(args: List[str]) -> dict: caller = get_caller_branch() if not caller: - return {"success": False, "error": "Could not detect calling branch"} + return {"success": False, "error": "Could not detect calling branch. Run from a branch directory or use drone routing (drone @commons ...)"} agent_name = caller["name"] @@ -142,7 +142,7 @@ def remove_react(args: List[str]) -> dict: caller = get_caller_branch() if not caller: - return {"success": False, "error": "Could not detect calling branch"} + return {"success": False, "error": "Could not detect calling branch. Run from a branch directory or use drone routing (drone @commons ...)"} agent_name = caller["name"] @@ -237,7 +237,7 @@ def pin_post_cmd(args: List[str]) -> dict: caller = get_caller_branch() if not caller: - return {"success": False, "error": "Could not detect calling branch"} + return {"success": False, "error": "Could not detect calling branch. Run from a branch directory or use drone routing (drone @commons ...)"} agent_name = caller["name"] @@ -301,7 +301,7 @@ def unpin_post_cmd(args: List[str]) -> dict: caller = get_caller_branch() if not caller: - return {"success": False, "error": "Could not detect calling branch"} + return {"success": False, "error": "Could not detect calling branch. Run from a branch directory or use drone routing (drone @commons ...)"} agent_name = caller["name"] diff --git a/src/commons/apps/handlers/identity/identity_ops.py b/src/commons/apps/handlers/identity/identity_ops.py index 273fac5e..c18a5186 100644 --- a/src/commons/apps/handlers/identity/identity_ops.py +++ b/src/commons/apps/handlers/identity/identity_ops.py @@ -135,11 +135,44 @@ def get_branch_info_from_registry(branch_path: Path) -> Optional[Dict[str, Any]] return None +def get_branch_info_by_name(branch_name: str) -> Optional[Dict[str, Any]]: + """ + Look up branch information in AIPASS_REGISTRY.json by name. + + Args: + branch_name: Branch name to look up (case-insensitive). + + Returns: + Dict with branch info from registry, or None if not found. + """ + if not BRANCH_REGISTRY_PATH.exists(): + return None + + try: + with open(BRANCH_REGISTRY_PATH, "r", encoding="utf-8") as f: + registry = json.load(f) + + name_upper = branch_name.upper() + for branch in registry.get("branches", []): + if branch.get("name", "").upper() == name_upper: + return branch + + return None + + except Exception: + logger.warning("[identity_ops] Failed to look up branch by name in registry") + return None + + def get_caller_branch() -> Optional[Dict[str, Any]]: """ - Detect which branch is calling The Commons based on PWD. + Detect which branch is calling The Commons. + + Detection order: + 1. AIPASS_CALLER_CWD env var (set by drone) — walk up to find .trinity/ + 2. Current working directory — walk up to find .trinity/ + 3. AIPASS_CALLER_BRANCH env var (set by drone) — direct name lookup - Walks up from CWD to find branch root, then looks up in AIPASS_REGISTRY.json. Auto-registers the branch as a Commons agent if not already present. Returns: @@ -147,27 +180,29 @@ def get_caller_branch() -> Optional[Dict[str, Any]]: or None if no branch detected. """ try: - # Use caller's original CWD if routed through drone + # Strategy 1 & 2: Walk up from CWD to find branch root caller_cwd = os.environ.get("AIPASS_CALLER_CWD", "") cwd = Path(caller_cwd) if caller_cwd else Path.cwd() branch_root = find_branch_root(cwd) - if not branch_root: - logger.warning("[commons.identity] Could not detect branch from PWD") - return None + if branch_root: + branch_info = get_branch_info_from_registry(branch_root) + if branch_info: + _ensure_agent_registered(branch_info) + json_handler.log_operation("caller_detected", {"branch": branch_info.get("name", "unknown")}) + return branch_info - branch_info = get_branch_info_from_registry(branch_root) - if not branch_info: - logger.warning( - f"[commons.identity] Branch at {branch_root} not in BRANCH_REGISTRY" - ) - return None + # Strategy 3: Use AIPASS_CALLER_BRANCH env var (drone sets this) + caller_branch_name = os.environ.get("AIPASS_CALLER_BRANCH", "") + if caller_branch_name: + branch_info = get_branch_info_by_name(caller_branch_name) + if branch_info: + _ensure_agent_registered(branch_info) + json_handler.log_operation("caller_detected", {"branch": branch_info.get("name", "unknown"), "via": "AIPASS_CALLER_BRANCH"}) + return branch_info - # Auto-register as Commons agent - _ensure_agent_registered(branch_info) - - json_handler.log_operation("caller_detected", {"branch": branch_info.get("name", "unknown")}) - return branch_info + logger.warning("[commons.identity] Could not detect calling branch — run from a branch directory or use drone routing") + return None except Exception as e: logger.error(f"[commons.identity] Branch detection failed: {e}") diff --git a/src/commons/apps/handlers/notifications/notification_ops.py b/src/commons/apps/handlers/notifications/notification_ops.py index 9ee30348..cf63ec48 100644 --- a/src/commons/apps/handlers/notifications/notification_ops.py +++ b/src/commons/apps/handlers/notifications/notification_ops.py @@ -88,7 +88,7 @@ def _set_notification_level(args: List[str], level: str) -> dict: caller = get_caller_branch() if not caller: - return {"success": False, "error": "Could not detect calling branch"} + return {"success": False, "error": "Could not detect calling branch. Run from a branch directory or use drone routing (drone @commons ...)"} agent_name = caller["name"] @@ -151,7 +151,7 @@ def show_preferences(args: List[str]) -> dict: """ caller = get_caller_branch() if not caller: - return {"success": False, "error": "Could not detect calling branch"} + return {"success": False, "error": "Could not detect calling branch. Run from a branch directory or use drone routing (drone @commons ...)"} agent_name = caller["name"] diff --git a/src/commons/apps/handlers/posts/post_ops.py b/src/commons/apps/handlers/posts/post_ops.py index b3218e01..a403255b 100644 --- a/src/commons/apps/handlers/posts/post_ops.py +++ b/src/commons/apps/handlers/posts/post_ops.py @@ -83,7 +83,7 @@ def create_post(args: List[str]) -> dict: # --- Get caller identity --- caller = get_caller_branch() if not caller: - return {"success": False, "error": "Could not detect calling branch"} + return {"success": False, "error": "Could not detect calling branch. Run from a branch directory or use drone routing (drone @commons ...)"} author = caller.get("name", "UNKNOWN") @@ -258,7 +258,7 @@ def delete_post(args: List[str]) -> dict: # --- Get caller identity --- caller = get_caller_branch() if not caller: - return {"success": False, "error": "Could not detect calling branch"} + return {"success": False, "error": "Could not detect calling branch. Run from a branch directory or use drone routing (drone @commons ...)"} author = caller.get("name", "UNKNOWN") diff --git a/src/commons/apps/handlers/rooms/room_ops.py b/src/commons/apps/handlers/rooms/room_ops.py index f1baa8cb..1b7fac30 100644 --- a/src/commons/apps/handlers/rooms/room_ops.py +++ b/src/commons/apps/handlers/rooms/room_ops.py @@ -56,7 +56,7 @@ def create_room(args: List[str]) -> dict: # Get caller identity caller = get_caller_branch() if not caller: - return {"success": False, "error": "Could not detect calling branch"} + return {"success": False, "error": "Could not detect calling branch. Run from a branch directory or use drone routing (drone @commons ...)"} caller_name = caller.get("name", "UNKNOWN") @@ -171,7 +171,7 @@ def join_room(args: List[str]) -> dict: # Get caller identity caller = get_caller_branch() if not caller: - return {"success": False, "error": "Could not detect calling branch"} + return {"success": False, "error": "Could not detect calling branch. Run from a branch directory or use drone routing (drone @commons ...)"} caller_name = caller.get("name", "UNKNOWN") diff --git a/src/commons/apps/modules/commons_identity.py b/src/commons/apps/modules/commons_identity.py index 4f64687f..aaa6545d 100644 --- a/src/commons/apps/modules/commons_identity.py +++ b/src/commons/apps/modules/commons_identity.py @@ -35,6 +35,7 @@ except ImportError: from commons.apps.handlers.identity.identity_ops import ( find_branch_root, get_branch_info_from_registry, + get_branch_info_by_name, get_caller_branch, extract_mentions, resolve_display_name, @@ -44,6 +45,7 @@ from commons.apps.handlers.json import json_handler __all__ = [ "find_branch_root", "get_branch_info_from_registry", + "get_branch_info_by_name", "get_caller_branch", "extract_mentions", "resolve_display_name", diff --git a/src/commons/tests/test_identity.py b/src/commons/tests/test_identity.py index aa77f974..e20b8ced 100644 --- a/src/commons/tests/test_identity.py +++ b/src/commons/tests/test_identity.py @@ -54,6 +54,8 @@ except ImportError: from commons.apps.modules.commons_identity import extract_mentions from commons.apps.handlers.identity.identity_ops import ( find_branch_root, + get_branch_info_by_name, + get_caller_branch, resolve_display_name, ) import commons.apps.handlers.identity.identity_ops as identity_ops_mod @@ -218,3 +220,143 @@ def test_resolve_display_name_compact_no_alias(monkeypatch: pytest.MonkeyPatch): monkeypatch.setattr(identity_ops_mod, "_alias_cache", {}) result = resolve_display_name("RAW_NAME", compact=True) assert result == "RAW_NAME" + + +# =========================================================================== +# get_branch_info_by_name — registry lookup by name +# =========================================================================== + +def test_get_branch_info_by_name_found(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + """Returns branch info when name matches a registry entry.""" + import json as json_mod + + registry = {"branches": [ + {"name": "DRONE", "path": "src/aipass/drone", "email": "@drone"}, + {"name": "FLOW", "path": "src/aipass/flow", "email": "@flow"}, + ]} + reg_file = tmp_path / "AIPASS_REGISTRY.json" + reg_file.write_text(json_mod.dumps(registry), encoding="utf-8") + monkeypatch.setattr(identity_ops_mod, "BRANCH_REGISTRY_PATH", reg_file) + + result = get_branch_info_by_name("drone") + assert result is not None + assert result["name"] == "DRONE" + assert result["email"] == "@drone" + + +def test_get_branch_info_by_name_case_insensitive(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + """Lookup is case-insensitive.""" + import json as json_mod + + registry = {"branches": [{"name": "FLOW", "path": "src/aipass/flow"}]} + reg_file = tmp_path / "AIPASS_REGISTRY.json" + reg_file.write_text(json_mod.dumps(registry), encoding="utf-8") + monkeypatch.setattr(identity_ops_mod, "BRANCH_REGISTRY_PATH", reg_file) + + result = get_branch_info_by_name("Flow") + assert result is not None + assert result["name"] == "FLOW" + + +def test_get_branch_info_by_name_not_found(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + """Returns None when name is not in registry.""" + import json as json_mod + + registry = {"branches": [{"name": "DRONE", "path": "src/aipass/drone"}]} + reg_file = tmp_path / "AIPASS_REGISTRY.json" + reg_file.write_text(json_mod.dumps(registry), encoding="utf-8") + monkeypatch.setattr(identity_ops_mod, "BRANCH_REGISTRY_PATH", reg_file) + + result = get_branch_info_by_name("nonexistent") + assert result is None + + +def test_get_branch_info_by_name_missing_file(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + """Returns None when registry file doesn't exist.""" + monkeypatch.setattr(identity_ops_mod, "BRANCH_REGISTRY_PATH", tmp_path / "nope.json") + result = get_branch_info_by_name("DRONE") + assert result is None + + +# =========================================================================== +# get_caller_branch — drone routing fallback via AIPASS_CALLER_BRANCH +# =========================================================================== + +@patch("commons.apps.handlers.identity.identity_ops.json_handler") +@patch("commons.apps.handlers.identity.identity_ops._ensure_agent_registered") +def test_get_caller_branch_uses_caller_branch_env( + mock_register: MagicMock, + mock_json: MagicMock, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +): + """Falls back to AIPASS_CALLER_BRANCH when CWD has no .trinity/.""" + import json as json_mod + + registry = {"branches": [{"name": "DRONE", "path": "src/aipass/drone", "email": "@drone"}]} + reg_file = tmp_path / "AIPASS_REGISTRY.json" + reg_file.write_text(json_mod.dumps(registry), encoding="utf-8") + monkeypatch.setattr(identity_ops_mod, "BRANCH_REGISTRY_PATH", reg_file) + + # CWD with no .trinity/ — simulates running from project root + no_branch_dir = tmp_path / "somewhere" + no_branch_dir.mkdir() + monkeypatch.setenv("AIPASS_CALLER_CWD", str(no_branch_dir)) + monkeypatch.setenv("AIPASS_CALLER_BRANCH", "drone") + + result = get_caller_branch() + assert result is not None + assert result["name"] == "DRONE" + mock_register.assert_called_once() + + +@patch("commons.apps.handlers.identity.identity_ops.json_handler") +@patch("commons.apps.handlers.identity.identity_ops._ensure_agent_registered") +def test_get_caller_branch_prefers_cwd_over_env( + mock_register: MagicMock, + mock_json: MagicMock, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +): + """CWD-based detection takes priority over AIPASS_CALLER_BRANCH.""" + import json as json_mod + + # Set up a branch directory with .trinity/ + trinity = tmp_path / ".trinity" + trinity.mkdir() + (trinity / "passport.json").write_text("{}", encoding="utf-8") + + registry = {"branches": [ + {"name": "FLOW", "path": str(tmp_path.relative_to(tmp_path.parent.parent)), "email": "@flow"}, + ]} + reg_file = tmp_path / "AIPASS_REGISTRY.json" + reg_file.write_text(json_mod.dumps(registry), encoding="utf-8") + monkeypatch.setattr(identity_ops_mod, "BRANCH_REGISTRY_PATH", reg_file) + + # CWD is inside the branch + monkeypatch.setenv("AIPASS_CALLER_CWD", str(tmp_path)) + # Also set CALLER_BRANCH to something different — should NOT be used + monkeypatch.setenv("AIPASS_CALLER_BRANCH", "DRONE") + + # Need to patch get_branch_info_from_registry to return for our tmp_path + with patch.object(identity_ops_mod, "get_branch_info_from_registry", return_value={"name": "FLOW", "email": "@flow"}): + result = get_caller_branch() + + assert result is not None + assert result["name"] == "FLOW" # CWD-based, not DRONE from env + + +@patch("commons.apps.handlers.identity.identity_ops.json_handler") +def test_get_caller_branch_returns_none_when_no_detection( + mock_json: MagicMock, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +): + """Returns None when neither CWD nor env var yields a branch.""" + no_branch_dir = tmp_path / "empty" + no_branch_dir.mkdir() + monkeypatch.setenv("AIPASS_CALLER_CWD", str(no_branch_dir)) + monkeypatch.delenv("AIPASS_CALLER_BRANCH", raising=False) + + result = get_caller_branch() + assert result is None diff --git a/src/commons/tests/test_rooms.py b/src/commons/tests/test_rooms.py index 4ab86931..aa3bbbcc 100644 --- a/src/commons/tests/test_rooms.py +++ b/src/commons/tests/test_rooms.py @@ -139,6 +139,7 @@ def test_create_room_no_caller(mock_caller: object) -> None: result = create_room(["orphan-room"]) assert result["success"] is False assert "Could not detect calling branch" in result["error"] + assert "drone routing" in result["error"] @patch("commons.apps.handlers.rooms.room_ops.get_db") diff --git a/src/skills/apps/handlers/template.py b/src/skills/apps/handlers/template.py index 177248fe..56abc0e0 100644 --- a/src/skills/apps/handlers/template.py +++ b/src/skills/apps/handlers/template.py @@ -83,8 +83,9 @@ def copy_template(template_path, target_path, skill_name): } try: - # Copy the entire template tree - shutil.copytree(str(template_path), str(target)) + # Copy the entire template tree, excluding __pycache__ + shutil.copytree(str(template_path), str(target), + ignore=shutil.ignore_patterns("__pycache__")) # Replace placeholders in all files created_files = [] diff --git a/src/skills/apps/skills.py b/src/skills/apps/skills.py index 03c336c1..6ab7a41c 100644 --- a/src/skills/apps/skills.py +++ b/src/skills/apps/skills.py @@ -86,6 +86,9 @@ def handle_command(command, args=None): if not args: error("Error: skill name required. Usage: skills create [--with-handler|--full]") return False + if args[0] in ("--help", "-h", "help"): + _print_create_help() + return True return _cmd_create(args) if command == "validate": @@ -222,6 +225,21 @@ def _cmd_run(name, action, extra_args): return result["success"] +def _print_create_help(): + """Print help text for the create subcommand.""" + console.print("Skills Create - Scaffold a new skill from a template") + console.print() + console.print("Usage:") + console.print(" drone @skills create Create a markdown-only skill") + console.print(" drone @skills create --with-handler Create with handler.py") + console.print(" drone @skills create --full Create with full 3-layer structure") + console.print() + console.print("Templates:") + console.print(" markdown_only SKILL.md with instructions (AI reads and follows)") + console.print(" with_handler SKILL.md + handler.py (programmatic execution)") + console.print(" full SKILL.md + apps/ structure (complex skills)") + + def _cmd_create(args): """Create a new skill from a template.""" from skills.apps.modules.creator import create_skill diff --git a/src/skills/tests/test_cli_routing.py b/src/skills/tests/test_cli_routing.py index 30aeb86d..d79a5f94 100644 --- a/src/skills/tests/test_cli_routing.py +++ b/src/skills/tests/test_cli_routing.py @@ -101,6 +101,28 @@ class TestHandleCommand: result = handle_command("create") assert result is False + def test_create_help_flag_returns_true(self): + """create --help shows help instead of treating --help as a skill name.""" + result = handle_command("create", ["--help"]) + assert result is True + + def test_create_help_flag_shows_usage(self, capsys): + """create --help prints usage text.""" + handle_command("create", ["--help"]) + captured = capsys.readouterr() + assert "Usage" in captured.out + assert "create" in captured.out.lower() + + def test_create_h_flag_returns_true(self): + """create -h shows help.""" + result = handle_command("create", ["-h"]) + assert result is True + + def test_create_help_word_returns_true(self): + """create help shows help.""" + result = handle_command("create", ["help"]) + assert result is True + # =================================================================== # Missing coverage: no_args, print_help, print_introspection, output_capture diff --git a/src/skills/tests/test_lifecycle.py b/src/skills/tests/test_lifecycle.py index 6bbe8851..1ced7d80 100644 --- a/src/skills/tests/test_lifecycle.py +++ b/src/skills/tests/test_lifecycle.py @@ -55,8 +55,10 @@ class TestFullLifecycle: assert skills[0]["has_handler"] is False # Load (parse full SKILL.md) - metadata, body = parse_full_skill_md(skill_path / "SKILL.md") + result = parse_full_skill_md(skill_path / "SKILL.md") + metadata, body = result[0], result[1] assert metadata is not None + assert isinstance(metadata, dict) assert metadata["name"] == "test-md" assert body is not None @@ -113,7 +115,10 @@ class TestCatalogSkillsLifecycle: assert github[0]["has_handler"] is False # Parse full SKILL.md - metadata, body = parse_full_skill_md(github[0]["path"] / "SKILL.md") + result = parse_full_skill_md(github[0]["path"] / "SKILL.md") + metadata, body = result[0], result[1] + assert metadata is not None + assert isinstance(metadata, dict) assert metadata["name"] == "github" assert body is not None assert "gh" in body.lower() @@ -178,3 +183,28 @@ class TestTemplates: assert "already exists" in result["error"] finally: shutil.rmtree(tmpdir) + + def test_copy_template_excludes_pycache(self): + """copy_template must not include __pycache__ directories in output.""" + tmpdir = tempfile.mkdtemp() + try: + template = get_template("full") + assert template["success"] is True + # Create a __pycache__ dir inside the template to ensure it gets filtered + pycache = template["path"] / "__pycache__" + pycache_existed = pycache.exists() + if not pycache_existed: + pycache.mkdir() + (pycache / "dummy.pyc").write_bytes(b"\x00") + try: + target = Path(tmpdir) / "cache-test" + result = copy_template(template["path"], target, "cache-test") + assert result["success"] is True + assert not (target / "__pycache__").exists() + for f in result["created_files"]: + assert "__pycache__" not in f + finally: + if not pycache_existed: + shutil.rmtree(str(pycache)) + finally: + shutil.rmtree(tmpdir)