From 47b5b2d871badd959790766c1b1174ea86a85b04 Mon Sep 17 00:00:00 2001 From: AIOSAI Date: Mon, 6 Jul 2026 10:16:11 -0700 Subject: [PATCH 01/73] =?UTF-8?q?prax:=20log=20watchdog=20covers=20branch?= =?UTF-8?q?=20logs/=20dirs=20=E2=80=94=20.jsonl=20runaway=20growth=20caugh?= =?UTF-8?q?t=20(built=20by=20@prax)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Root cause: rotation .log-hardcoded; .jsonl files are raw open('a') appenders bypassing prax; watchdog only scanned system_logs - New: scan_branch_log_files (WARN 1MB / CRITICAL 10MB unrotated), enforce_branch_log_limits (tail-keep 5000 lines), branch health summary, log-audit shows both scopes - 11 new tests, prax suite 947 green (61/61 verified in touched files by devpulse) - Offender writers routed to owners: @hooks engine.jsonl+telegram_delivery.jsonl, @backup operations.jsonl, @trigger medic_suppressed.log --- CHANGELOG.md | 19 +++ .../apps/handlers/logging/log_watchdog.py | 151 ++++++++++++++++- src/aipass/prax/apps/modules/log_audit.py | 68 +++++++- src/aipass/prax/tests/test_log_audit.py | 137 ++++++++++++++-- .../prax/tests/test_logging_handlers.py | 153 +++++++++++++++++- 5 files changed, 506 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2b3b76a..ca50f028 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,25 @@ PyPI version — not the changelog header. --- +## [2026-07-06] + +### Fixed + +- **prax log watchdog now covers branch `logs/` dirs — `.jsonl` runaway growth + caught.** Rotation was hardcoded to `.log` files, and several branches write + `.jsonl` logs via raw `open(path, "a")` appenders that bypass prax entirely — + `hooks/logs/engine.jsonl` had grown to 63 MB, `backup/logs/operations.jsonl` + to 31 MB, `trigger/logs/medic_suppressed.log` to 7 MB, all unrotated. The + log-watchdog safety net also only scanned `system_logs/*.log`. @prax extended + it: `scan_branch_log_files()` sweeps every `src/aipass/*/logs/` for `.log` + + `.jsonl` (WARN at 1 MB unrotated, CRITICAL at 10 MB), + `enforce_branch_log_limits()` truncates flagged files to the last 5000 lines, + and `drone @prax log-audit` now reports both system and branch scopes. 11 new + tests, full prax suite 947 green. The raw-appender writers themselves still + need per-owner caps — routed to @hooks, @backup, @trigger. (built by @prax) + +--- + ## [2026-07-05] ### Fixed diff --git a/src/aipass/prax/apps/handlers/logging/log_watchdog.py b/src/aipass/prax/apps/handlers/logging/log_watchdog.py index 20290203..d612b031 100644 --- a/src/aipass/prax/apps/handlers/logging/log_watchdog.py +++ b/src/aipass/prax/apps/handlers/logging/log_watchdog.py @@ -9,21 +9,24 @@ """ System Log Size Watchdog -Scans the system_logs/ directory for oversized log files and enforces size limits. -Catches ALL log files regardless of how they were created — even those bypassing -PRAX's RotatingFileHandler (e.g., telegram bots using plain FileHandler). +Scans system_logs/ and all branch logs/ directories for oversized files and +enforces size limits. Catches ALL log files regardless of how they were created +— even those bypassing PRAX's RotatingFileHandler (e.g., raw open("a") appenders +writing .jsonl files, telegram bots using plain FileHandler). This is the safety net: even if a branch misconfigures logging, the watchdog prevents unbounded growth that caused the 2026-02-26 system crash (DPLAN-037). +Two scopes: + - system_logs/ (central) — scanned by scan_log_files() + - src/aipass/*/logs/ (branch-local) — scanned by scan_branch_log_files() + Two modes: - audit: Report oversized files without changing anything - enforce: Truncate oversized files to keep last max_lines """ import logging - -logger = logging.getLogger(__name__) import sys from datetime import datetime from pathlib import Path @@ -31,6 +34,8 @@ from typing import Any, Dict, List, Tuple from aipass.prax.apps.handlers.json import json_handler +logger = logging.getLogger(__name__) + # ============================================================================= # CONSTANTS @@ -57,11 +62,16 @@ def _get_system_logs_dir() -> Path: return _system_logs_dir_cache -# Thresholds +# Thresholds — system_logs (line-based) WARN_THRESHOLD_LINES = 5000 # Fire warning at this line count DEFAULT_MAX_LINES = 1000 # Truncate to this many lines (matches prax config) CRITICAL_THRESHOLD_LINES = 10000 # Immediate action recommended +# Thresholds — branch logs (size-based, catches .jsonl and unrotated .log) +BRANCH_WARN_SIZE_MB = 1.0 +BRANCH_CRITICAL_SIZE_MB = 10.0 +BRANCH_DEFAULT_MAX_LINES = 5000 + # ============================================================================= # SCANNING @@ -277,6 +287,135 @@ def log_health_summary() -> Dict[str, Any]: } +# ============================================================================= +# BRANCH LOG SCANNING — covers .log and .jsonl in src/aipass/*/logs/ +# ============================================================================= + + +def _get_ecosystem_root() -> Path: + """Find src/aipass/ directory.""" + return _find_repo_root() / "src" / "aipass" + + +def _has_rotation_sibling(filepath: Path) -> bool: + """Check if a file has a .1 rotation sibling.""" + return (filepath.parent / f"{filepath.name}.1").exists() + + +def _classify_branch_log(log_file: Path, branch_name: str) -> Dict[str, Any]: + """Build an audit record for a single branch log file.""" + size_kb = _get_file_size_kb(log_file) + size_mb = size_kb / 1024.0 + lines = _count_lines(log_file) + has_rotation = _has_rotation_sibling(log_file) + + if size_mb >= BRANCH_CRITICAL_SIZE_MB: + status = "critical" + elif size_mb >= BRANCH_WARN_SIZE_MB and not has_rotation: + status = "warning" + else: + status = "ok" + + return { + "path": str(log_file), + "name": log_file.name, + "branch": branch_name, + "lines": lines, + "size_kb": round(size_kb, 1), + "size_mb": round(size_mb, 1), + "has_rotation": has_rotation, + "status": status, + } + + +def scan_branch_log_files() -> List[Dict[str, Any]]: + """ + Scan all branch logs/ directories for files with unbounded growth. + + Checks .log and .jsonl files. Flags unrotated files exceeding size + thresholds — the safety net for writers that bypass RotatingFileHandler. + """ + results: List[Dict[str, Any]] = [] + eco_root = _get_ecosystem_root() + + if not eco_root.exists(): + return results + + for branch_dir in sorted(eco_root.iterdir()): + logs_dir = branch_dir / "logs" + if not branch_dir.is_dir() or not logs_dir.is_dir(): + continue + for log_file in sorted(logs_dir.glob("*.log")) + sorted(logs_dir.glob("*.jsonl")): + results.append(_classify_branch_log(log_file, branch_dir.name)) + + results.sort(key=lambda x: x["size_kb"], reverse=True) + json_handler.log_operation("branch_log_watchdog_check", {"files_scanned": len(results)}) + return results + + +def get_oversized_branch_files() -> List[Dict[str, Any]]: + """Get branch log files exceeding size thresholds.""" + return [f for f in scan_branch_log_files() if f["status"] != "ok"] + + +def enforce_branch_log_limits( + max_lines: int = BRANCH_DEFAULT_MAX_LINES, +) -> List[Dict[str, Any]]: + """ + Truncate oversized branch log files (including .jsonl). + + Only truncates files flagged as warning or critical by scan_branch_log_files(). + """ + actions: List[Dict[str, Any]] = [] + + for file_info in get_oversized_branch_files(): + filepath = Path(file_info["path"]) + original, new = truncate_log_file(filepath, max_lines) + + actions.append( + { + "name": file_info["name"], + "branch": file_info["branch"], + "original_lines": original, + "new_lines": new, + "size_mb": file_info["size_mb"], + "truncated": original != new, + } + ) + + return actions + + +def branch_log_health_summary() -> Dict[str, Any]: + """Generate a health summary of branch logs.""" + files = scan_branch_log_files() + + if not files: + return { + "total_files": 0, + "oversized_count": 0, + "critical_count": 0, + "total_size_mb": 0.0, + "largest_file": None, + "healthy": True, + } + + oversized = [f for f in files if f["status"] in ("warning", "critical")] + critical = [f for f in files if f["status"] == "critical"] + largest = files[0] if files else None + total_size = sum(f["size_mb"] for f in files) + + return { + "total_files": len(files), + "oversized_count": len(oversized), + "critical_count": len(critical), + "total_size_mb": round(total_size, 1), + "largest_file": f"{largest['branch']}/{largest['name']}" if largest else None, + "largest_size_mb": largest["size_mb"] if largest else 0.0, + "healthy": len(oversized) == 0, + } + + # ============================================================================= # CLI ENTRY POINT (for testing) # ============================================================================= diff --git a/src/aipass/prax/apps/modules/log_audit.py b/src/aipass/prax/apps/modules/log_audit.py index 185c1da2..4046c8f1 100644 --- a/src/aipass/prax/apps/modules/log_audit.py +++ b/src/aipass/prax/apps/modules/log_audit.py @@ -75,9 +75,9 @@ def print_help(): def _display_audit(files: list, summary: dict) -> None: - """Display audit results.""" + """Display system_logs/ audit results.""" console.print() - console.print("[bold cyan]System Log Audit[/bold cyan]") + console.print("[bold cyan]System Log Audit[/bold cyan] [dim](system_logs/)[/dim]") console.print(f" Total files: {summary['total_files']}") console.print(f" Total lines: {summary['total_lines']:,}") if summary.get("largest_file"): @@ -90,7 +90,6 @@ def _display_audit(files: list, summary: dict) -> None: else: error(f"Status: {summary['oversized_count']} oversized, {summary['critical_count']} critical") - # Show oversized files oversized = [f for f in files if f["status"] != "ok"] if oversized: console.print() @@ -101,8 +100,36 @@ def _display_audit(files: list, summary: dict) -> None: f" [{status_color}]{f['status'].upper()}[/{status_color}] " f"{f['name']}: {f['lines']:,} lines ({f['size_kb']} KB)" ) + console.print() + + +def _display_branch_audit(files: list, summary: dict) -> None: + """Display branch logs/ audit results.""" + console.print("[bold cyan]Branch Log Audit[/bold cyan] [dim](src/aipass/*/logs/)[/dim]") + console.print(f" Total files: {summary['total_files']}") + console.print(f" Total size: {summary['total_size_mb']} MB") + if summary.get("largest_file"): + console.print(f" Largest: {summary['largest_file']} ({summary.get('largest_size_mb', 0)} MB)") + + if summary["healthy"]: + console.print("[green] Status: HEALTHY — no unbounded files[/green]") + else: + error(f"Status: {summary['oversized_count']} unbounded, {summary['critical_count']} critical") + + oversized = [f for f in files if f["status"] != "ok"] + if oversized: console.print() - console.print("[dim]Run 'drone @prax log-audit enforce' to truncate oversized files[/dim]") + console.print("[bold]Unbounded files (no rotation, exceeds size threshold):[/bold]") + for f in oversized: + status_color = "red" if f["status"] == "critical" else "yellow" + rotation = "[green]rotated[/green]" if f["has_rotation"] else "[red]unrotated[/red]" + console.print( + f" [{status_color}]{f['status'].upper()}[/{status_color}] " + f"{f['branch']}/{f['name']}: {f['size_mb']} MB, " + f"{f['lines']:,} lines, {rotation}" + ) + console.print() + console.print("[dim]Run 'drone @prax log-audit enforce' to truncate[/dim]") console.print() @@ -140,10 +167,20 @@ def handle_command(command: str, args: List[str]) -> bool: files = scan_log_files() summary = log_health_summary() _display_audit(files, summary) + + from aipass.prax.apps.handlers.logging.log_watchdog import ( + scan_branch_log_files, + branch_log_health_summary, + ) + + branch_files = scan_branch_log_files() + branch_summary = branch_log_health_summary() + _display_branch_audit(branch_files, branch_summary) return True if subcmd == "enforce": _run_enforce() + _run_branch_enforce() return True error(f"Unknown log-audit subcommand: {subcmd}") @@ -174,6 +211,29 @@ def _run_enforce(): logger.info("[log-audit] Enforced limits on %d files", len(actions)) +def _run_branch_enforce(): + """Execute branch log enforcement and display results.""" + from aipass.prax.apps.handlers.logging.log_watchdog import enforce_branch_log_limits + + console.print("[bold cyan]Enforcing branch log limits...[/bold cyan]") + actions = enforce_branch_log_limits() + + if not actions: + console.print("[green]All branch logs within limits — nothing to truncate[/green]\n") + return + + for action in actions: + if action["truncated"]: + console.print( + f" [red]TRUNCATED[/red] {action['branch']}/{action['name']}: " + f"{action['size_mb']} MB, {action['original_lines']:,} → {action['new_lines']:,} lines" + ) + else: + console.print(f" [green]OK[/green] {action['branch']}/{action['name']}: within limits") + console.print() + logger.info("[log-audit] Enforced branch log limits on %d files", len(actions)) + + if __name__ == "__main__": if len(sys.argv) == 1: print_introspection() diff --git a/src/aipass/prax/tests/test_log_audit.py b/src/aipass/prax/tests/test_log_audit.py index c1c6bd03..3c1cc5a4 100644 --- a/src/aipass/prax/tests/test_log_audit.py +++ b/src/aipass/prax/tests/test_log_audit.py @@ -47,6 +47,43 @@ def _ensure_watchdog_mock(monkeypatch): {"name": "error.log", "truncated": True, "original_lines": 2500, "new_lines": 1000}, ] ) + mock_watchdog.scan_branch_log_files = MagicMock( + return_value=[ + { + "name": "engine.jsonl", + "branch": "hooks", + "lines": 200000, + "size_kb": 64512.0, + "size_mb": 63.0, + "has_rotation": False, + "status": "critical", + "path": "/fake/hooks/logs/engine.jsonl", + }, + ] + ) + mock_watchdog.branch_log_health_summary = MagicMock( + return_value={ + "total_files": 5, + "oversized_count": 1, + "critical_count": 1, + "total_size_mb": 95.1, + "largest_file": "hooks/engine.jsonl", + "largest_size_mb": 63.0, + "healthy": False, + } + ) + mock_watchdog.enforce_branch_log_limits = MagicMock( + return_value=[ + { + "name": "engine.jsonl", + "branch": "hooks", + "original_lines": 200000, + "new_lines": 5001, + "size_mb": 63.0, + "truncated": True, + }, + ] + ) monkeypatch.setitem( sys.modules, "aipass.prax.apps.handlers.logging.log_watchdog", @@ -64,9 +101,10 @@ def _fresh_import(): print_help, print_introspection, _display_audit, + _display_branch_audit, ) - return handle_command, print_help, print_introspection, _display_audit + return handle_command, print_help, print_introspection, _display_audit, _display_branch_audit # ============================================= @@ -76,7 +114,7 @@ def _fresh_import(): def test_handle_command_help(mock_prax_infrastructure, monkeypatch): """--help flag returns True and displays help text.""" - handle_command, _, _, _ = _fresh_import() + handle_command, _, _, _, _ = _fresh_import() result = handle_command("log-audit", ["--help"]) assert result is True @@ -85,7 +123,7 @@ def test_handle_command_help(mock_prax_infrastructure, monkeypatch): def test_handle_command_help_h_flag(mock_prax_infrastructure, monkeypatch): """-h flag also triggers help with audit-related content.""" - handle_command, _, _, _ = _fresh_import() + handle_command, _, _, _, _ = _fresh_import() result = handle_command("log-audit", ["-h"]) assert result is True @@ -95,7 +133,7 @@ def test_handle_command_help_h_flag(mock_prax_infrastructure, monkeypatch): def test_handle_command_no_args_calls_introspection(mock_prax_infrastructure, monkeypatch): """No args prints introspection and returns True.""" - handle_command, _, _, _ = _fresh_import() + handle_command, _, _, _, _ = _fresh_import() result = handle_command("log-audit", []) assert result is True @@ -105,7 +143,7 @@ def test_handle_command_no_args_calls_introspection(mock_prax_infrastructure, mo def test_handle_command_wrong_command(mock_prax_infrastructure, monkeypatch): """Wrong command name returns False.""" - handle_command, _, _, _ = _fresh_import() + handle_command, _, _, _, _ = _fresh_import() result = handle_command("not-log-audit", []) assert result is False @@ -113,7 +151,7 @@ def test_handle_command_wrong_command(mock_prax_infrastructure, monkeypatch): def test_print_help_runs(mock_prax_infrastructure, monkeypatch): """print_help runs without error and includes audit/enforce subcommands.""" - _, print_help, _, _ = _fresh_import() + _, print_help, _, _, _ = _fresh_import() print_help() mock_prax_infrastructure.console.print.assert_called() @@ -124,7 +162,7 @@ def test_print_help_runs(mock_prax_infrastructure, monkeypatch): def test_print_introspection_runs(mock_prax_infrastructure, monkeypatch): """print_introspection runs without error.""" - _, _, print_introspection, _ = _fresh_import() + _, _, print_introspection, _, _ = _fresh_import() print_introspection() calls = [str(c) for c in mock_prax_infrastructure.console.print.call_args_list] @@ -133,7 +171,7 @@ def test_print_introspection_runs(mock_prax_infrastructure, monkeypatch): def test_display_audit_healthy(mock_prax_infrastructure, monkeypatch): """_display_audit formats healthy summary correctly.""" - _, _, _, _display_audit = _fresh_import() + _, _, _, _display_audit, _ = _fresh_import() files = [{"name": "system.log", "lines": 200, "size_kb": 10, "status": "ok"}] summary = { @@ -154,7 +192,7 @@ def test_display_audit_healthy(mock_prax_infrastructure, monkeypatch): def test_display_audit_oversized(mock_prax_infrastructure, monkeypatch): """_display_audit shows oversized files when present.""" - _, _, _, _display_audit = _fresh_import() + _, _, _, _display_audit, _ = _fresh_import() files = [ {"name": "system.log", "lines": 500, "size_kb": 45, "status": "ok"}, @@ -185,7 +223,7 @@ def test_display_audit_oversized(mock_prax_infrastructure, monkeypatch): def test_handle_command_unknown_subcommand(mock_prax_infrastructure, monkeypatch): """Unknown subcommand shows error and help text.""" _ensure_watchdog_mock(monkeypatch) - handle_command, _, _, _ = _fresh_import() + handle_command, _, _, _, _ = _fresh_import() result = handle_command("log-audit", ["bogus"]) assert result is True @@ -200,9 +238,86 @@ def test_handle_command_unknown_subcommand(mock_prax_infrastructure, monkeypatch def test_handle_command_audit_subcommand(mock_prax_infrastructure, monkeypatch): """'audit' subcommand calls scan_log_files and log_health_summary.""" mock_watchdog = _ensure_watchdog_mock(monkeypatch) - handle_command, _, _, _ = _fresh_import() + handle_command, _, _, _, _ = _fresh_import() result = handle_command("log-audit", ["audit"]) assert result is True mock_watchdog.scan_log_files.assert_called_once() mock_watchdog.log_health_summary.assert_called_once() + mock_watchdog.scan_branch_log_files.assert_called_once() + mock_watchdog.branch_log_health_summary.assert_called_once() + + +def test_handle_command_enforce_calls_branch_enforce(mock_prax_infrastructure, monkeypatch): + """'enforce' subcommand calls both system and branch enforcement.""" + mock_watchdog = _ensure_watchdog_mock(monkeypatch) + handle_command, _, _, _, _ = _fresh_import() + + result = handle_command("log-audit", ["enforce"]) + assert result is True + mock_watchdog.enforce_log_limits.assert_called_once() + mock_watchdog.enforce_branch_log_limits.assert_called_once() + + +def test_display_branch_audit_healthy(mock_prax_infrastructure, monkeypatch): + """_display_branch_audit shows healthy status when no unbounded files.""" + _, _, _, _, _display_branch_audit = _fresh_import() + + files = [ + { + "name": "client.log", + "branch": "backup", + "lines": 200, + "size_kb": 40.0, + "size_mb": 0.04, + "has_rotation": True, + "status": "ok", + }, + ] + summary = { + "total_files": 1, + "oversized_count": 0, + "critical_count": 0, + "total_size_mb": 0.04, + "largest_file": "backup/client.log", + "largest_size_mb": 0.04, + "healthy": True, + } + + _display_branch_audit(files, summary) + calls = [str(c) for c in mock_prax_infrastructure.console.print.call_args_list] + assert any("HEALTHY" in c for c in calls) + + +def test_display_branch_audit_critical(mock_prax_infrastructure, monkeypatch): + """_display_branch_audit shows critical unbounded .jsonl files.""" + _, _, _, _, _display_branch_audit = _fresh_import() + + files = [ + { + "name": "engine.jsonl", + "branch": "hooks", + "lines": 200000, + "size_kb": 64512.0, + "size_mb": 63.0, + "has_rotation": False, + "status": "critical", + "path": "/fake/hooks/logs/engine.jsonl", + }, + ] + summary = { + "total_files": 1, + "oversized_count": 1, + "critical_count": 1, + "total_size_mb": 63.0, + "largest_file": "hooks/engine.jsonl", + "largest_size_mb": 63.0, + "healthy": False, + } + + _display_branch_audit(files, summary) + calls = [str(c) for c in mock_prax_infrastructure.console.print.call_args_list] + assert any("engine.jsonl" in c for c in calls) + assert any("hooks" in c for c in calls) + assert any("unrotated" in c for c in calls) + mock_prax_infrastructure.cli.error.assert_called() diff --git a/src/aipass/prax/tests/test_logging_handlers.py b/src/aipass/prax/tests/test_logging_handlers.py index 47d5a3e1..5cdfa65f 100644 --- a/src/aipass/prax/tests/test_logging_handlers.py +++ b/src/aipass/prax/tests/test_logging_handlers.py @@ -24,6 +24,7 @@ import importlib # noqa: F401 — used inside test functions for dynamic module import json import logging import sys +from pathlib import Path from unittest.mock import MagicMock, patch @@ -188,7 +189,7 @@ class TestGetCallingModulePath: """Returns the path from _find_external_caller_path when found.""" from aipass.prax.apps.handlers.logging import introspection - fake_path = "/home/user/src/aipass/flow/apps/flow.py" + fake_path = str(Path.home() / "src" / "aipass" / "flow" / "apps" / "flow.py") with patch.object(introspection, "_find_external_caller_path", return_value=fake_path): result = introspection.get_calling_module_path() assert result == fake_path @@ -342,6 +343,156 @@ class TestTruncateLogFile: assert new == 0 +# ============================================= +# log_watchdog.py -- scan_branch_log_files +# ============================================= + + +def _import_watchdog(tmp_path): + """Fresh-import log_watchdog with mocked config.""" + with patch.dict( + sys.modules, + { + "aipass.prax.apps.handlers.config.load": MagicMock( + PRAX_JSON_DIR=tmp_path / "prax_json", + ), + }, + ): + sys.modules.pop("aipass.prax.apps.handlers.logging.log_watchdog", None) + import aipass.prax.apps.handlers.logging.log_watchdog as lw + + return lw + + +class TestScanBranchLogFiles: + """Tests for log_watchdog.py scan_branch_log_files().""" + + def test_detects_large_jsonl(self, mock_prax_infrastructure, tmp_path): + """Flags .jsonl files exceeding size threshold as critical.""" + lw = _import_watchdog(tmp_path) + + eco = tmp_path / "src" / "aipass" + branch_logs = eco / "hooks" / "logs" + branch_logs.mkdir(parents=True) + + big_jsonl = branch_logs / "engine.jsonl" + big_jsonl.write_text( + "\n".join(f'{{"line": {i}, "padding": "{" " * 200}}}' for i in range(10000)) + "\n", + encoding="utf-8", + ) + + with patch.object(lw, "_get_ecosystem_root", return_value=eco): + with patch.object(lw, "BRANCH_WARN_SIZE_MB", 0.5): + results = lw.scan_branch_log_files() + assert len(results) == 1 + assert results[0]["name"] == "engine.jsonl" + assert results[0]["branch"] == "hooks" + assert not results[0]["has_rotation"] + assert results[0]["status"] in ("warning", "critical") + + def test_ignores_small_rotated_logs(self, mock_prax_infrastructure, tmp_path): + """Small .log files with rotation siblings are status ok.""" + lw = _import_watchdog(tmp_path) + + eco = tmp_path / "src" / "aipass" + branch_logs = eco / "backup" / "logs" + branch_logs.mkdir(parents=True) + + small_log = branch_logs / "client.log" + small_log.write_text("line1\nline2\n", encoding="utf-8") + rotation = branch_logs / "client.log.1" + rotation.write_text("old line\n", encoding="utf-8") + + with patch.object(lw, "_get_ecosystem_root", return_value=eco): + results = lw.scan_branch_log_files() + assert len(results) == 1 + assert results[0]["name"] == "client.log" + assert results[0]["has_rotation"] is True + assert results[0]["status"] == "ok" + + def test_empty_ecosystem_returns_empty(self, mock_prax_infrastructure, tmp_path): + """Returns empty when ecosystem root does not exist.""" + lw = _import_watchdog(tmp_path) + + nonexistent = tmp_path / "no_such_dir" + with patch.object(lw, "_get_ecosystem_root", return_value=nonexistent): + results = lw.scan_branch_log_files() + assert results == [] + + def test_scans_multiple_branches(self, mock_prax_infrastructure, tmp_path): + """Scans logs/ across multiple branches.""" + lw = _import_watchdog(tmp_path) + + eco = tmp_path / "src" / "aipass" + for branch in ("hooks", "backup", "trigger"): + logs = eco / branch / "logs" + logs.mkdir(parents=True) + (logs / "test.log").write_text("line\n" * 10, encoding="utf-8") + + with patch.object(lw, "_get_ecosystem_root", return_value=eco): + results = lw.scan_branch_log_files() + branches = {r["branch"] for r in results} + assert branches == {"hooks", "backup", "trigger"} + + +class TestBranchLogHealthSummary: + """Tests for log_watchdog.py branch_log_health_summary().""" + + def test_healthy_summary(self, mock_prax_infrastructure, tmp_path): + """Returns healthy when all files are ok.""" + lw = _import_watchdog(tmp_path) + + eco = tmp_path / "src" / "aipass" + logs = eco / "prax" / "logs" + logs.mkdir(parents=True) + (logs / "test.log").write_text("line\n" * 5, encoding="utf-8") + + with patch.object(lw, "_get_ecosystem_root", return_value=eco): + summary = lw.branch_log_health_summary() + assert summary["healthy"] is True + assert summary["total_files"] == 1 + assert summary["oversized_count"] == 0 + + def test_empty_summary(self, mock_prax_infrastructure, tmp_path): + """Returns healthy empty summary when no files found.""" + lw = _import_watchdog(tmp_path) + + nonexistent = tmp_path / "nope" + with patch.object(lw, "_get_ecosystem_root", return_value=nonexistent): + summary = lw.branch_log_health_summary() + assert summary["healthy"] is True + assert summary["total_files"] == 0 + + +class TestEnforceBranchLogLimits: + """Tests for log_watchdog.py enforce_branch_log_limits().""" + + def test_truncates_oversized_jsonl(self, mock_prax_infrastructure, tmp_path): + """Truncates .jsonl files that exceed size threshold.""" + lw = _import_watchdog(tmp_path) + + eco = tmp_path / "src" / "aipass" + logs = eco / "hooks" / "logs" + logs.mkdir(parents=True) + + big_jsonl = logs / "engine.jsonl" + big_jsonl.write_text( + "\n".join(f'{{"line": {i}, "padding": "{" " * 200}}}' for i in range(10000)) + "\n", + encoding="utf-8", + ) + + with patch.object(lw, "_get_ecosystem_root", return_value=eco): + with patch.object(lw, "BRANCH_WARN_SIZE_MB", 0.5): + actions = lw.enforce_branch_log_limits(max_lines=1000) + assert len(actions) >= 1 + action = actions[0] + assert action["truncated"] is True + assert action["branch"] == "hooks" + + content = big_jsonl.read_text(encoding="utf-8") + assert "LOG TRUNCATED by PRAX watchdog" in content + + # ============================================= # monitoring.py -- run_monitoring_loop # ============================================= From 5fd8c6a015a6a1319d35a58d51658ded20be46e2 Mon Sep 17 00:00:00 2001 From: AIOSAI Date: Tue, 7 Jul 2026 20:02:45 -0700 Subject: [PATCH 02/73] cadence fresh-context reset + aipass misroute guidance: SessionStart wiring (handler/config/setup.sh), loaders period-5, kernel+navmap aipass-exception, guide-not-crash (drone/aipass) --- .aipass/hooks.json | 8 + .aipass/project_hooks.json | 8 + .aipass/tier0_kernel.md | 4 +- .aipass/tier1_navmap.md | 2 +- .claude/commands/prep.md | 2 +- CHANGELOG.md | 26 +++ setup.sh | 4 + src/aipass/aipass/apps/aipass.py | 8 + src/aipass/aipass/tests/test_aipass_main.py | 32 +++ src/aipass/drone/apps/drone.py | 13 ++ src/aipass/drone/tests/test_cli_routing.py | 64 ++++++ src/aipass/hooks/.seedgo/bypass.json | 35 +++ src/aipass/hooks/README.md | 3 +- .../apps/handlers/lifecycle/session_start.py | 41 ++++ src/aipass/hooks/apps/modules/cadence.py | 2 +- src/aipass/hooks/tests/test_cadence.py | 6 +- src/aipass/hooks/tests/test_session_start.py | 216 ++++++++++++++++++ 17 files changed, 467 insertions(+), 7 deletions(-) create mode 100644 src/aipass/hooks/apps/handlers/lifecycle/session_start.py create mode 100644 src/aipass/hooks/tests/test_session_start.py diff --git a/.aipass/hooks.json b/.aipass/hooks.json index 0e84c6df..f6f3fe27 100644 --- a/.aipass/hooks.json +++ b/.aipass/hooks.json @@ -138,5 +138,13 @@ "matcher": "", "timeout": 120 } + }, + + "SessionStart": { + "cadence_reset": { + "enabled": true, + "handler": "aipass.hooks.apps.handlers.lifecycle.session_start.handle", + "matcher": "" + } } } diff --git a/.aipass/project_hooks.json b/.aipass/project_hooks.json index 2b6fb54e..7743c610 100644 --- a/.aipass/project_hooks.json +++ b/.aipass/project_hooks.json @@ -92,6 +92,14 @@ } }, + "SessionStart": { + "cadence_reset": { + "enabled": true, + "handler": "aipass.hooks.apps.handlers.lifecycle.session_start.handle", + "matcher": "" + } + }, + "PreCompact": { "pre_compact": { "enabled": true, diff --git a/.aipass/tier0_kernel.md b/.aipass/tier0_kernel.md index 79fd173e..32c7ae49 100644 --- a/.aipass/tier0_kernel.md +++ b/.aipass/tier0_kernel.md @@ -1,6 +1,6 @@ # AIPass — Kernel - + You are an AIPass agent — a citizen with identity, memory, and a mailbox. Your branch is your home and address. CWD is your identity: always know which branch you're standing in. The system runs on `drone`. @@ -13,6 +13,8 @@ You are an AIPass agent — a citizen with identity, memory, and a mailbox. Your - `drone @agent` — bare → the agent's live self-map. - `drone systems` — list every agent. +`aipass` is the one exception — the user's own front-door CLI and concierge (onboarding, `doctor`, OS/system help). Run `aipass` / `aipass --help` directly, **never `drone @aipass`** (drone can't resolve it). Serves humans, not agents. + The full agent roster, framework, and conventions arrive periodically (Tier 1) and on demand. Unsure of anything? Fetch it: `drone @agent --help` / the agent's `README.md` / `drone @memory search "query"`. # Don't get lost diff --git a/.aipass/tier1_navmap.md b/.aipass/tier1_navmap.md index 3da3426b..ae0c557d 100644 --- a/.aipass/tier1_navmap.md +++ b/.aipass/tier1_navmap.md @@ -41,7 +41,7 @@ src/aipass// - @drone — command router. Resolves `@agent`, routes commands, enforces tier-based access. Also the only git interface (`drone @git`). - @devpulse — orchestration hub, the user's primary collaborator. Coordinates the other agents, dispatches work, only agent with git write. - - @aipass — the user-facing front door and a system-ops collaborator. Onboarding (`aipass init`), `doctor` diagnostics, help chat, handoff; also partners with the user on host-level health (disk, thermal, docker, config). Concierge to other branches: reads, never writes. + - @aipass — the user's front-door concierge and its OWN CLI, NOT drone-routed: run `aipass` / `aipass --help` directly, never `drone @aipass` (drone can't resolve it). The human's best friend — onboarding (`aipass init`/`install`), `doctor` health, help chat, and OS/system questions ("why's my wifi dropping", "why's CC hogging CPU", "what is drone", "how do I make a project"). Serves humans, not agents — reads, never writes. - @ai_mail — inter-agent email. `dispatch` = send + wake (default for handing work), `email` = no wake, plus inbox/view/reply/close. - @flow — plan lifecycle: create, list, close, templates, registry. Plan types in the Plans section — never create plan files by hand. - @seedgo — code standards and audits. The standard pack, `audit` and `checklist`, the quality gate before and after building. diff --git a/.claude/commands/prep.md b/.claude/commands/prep.md index 69a71810..4f576fcb 100644 --- a/.claude/commands/prep.md +++ b/.claude/commands/prep.md @@ -44,7 +44,7 @@ Quick checks beat assumptions: `ls`/`find` for files, `git ls-files`/`grep` for - Update their execution logs, status, decision logs with current state - If a plan was completed, note it (but don't close — the user does that) -## 3. Git State +## 3. Git State (Devpulse only) - Run `git status` — report uncommitted changes - If there's a logical commit waiting, suggest it (don't commit without asking) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca50f028..385135b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,32 @@ PyPI version — not the changelog header. --- +## [2026-07-07] + +### Added + +- **Fresh-context grounding: cadence reset on new chat / clear / compact.** + Both prompt loaders (tier0 kernel + navmap) now run at period 5, and a new + `SessionStart` hook resets the cadence counter on `startup`/`clear` (skips + `resume` — restored context already carries grounding; `compact` was already + reset via PreCompact). Net effect: the first message of every fresh context + gets full grounding, then every 5th turn after. Wired end-to-end: handler + (`session_start.py`), project config (`.aipass/hooks.json` + the + `project_hooks.json` template for external projects), and `setup.sh` seeds + the provider `SessionStart` entry for new installs. (built by @hooks + + @devpulse) + +### Fixed + +- **`aipass` ≠ drone-routed — misroutes now guide instead of crash.** `aipass` + is the user's front-door CLI, deliberately not resolvable by drone. But + `drone aipass` misdirected, `drone @aipass` crashed with a traceback, and + `aipass @drone` dead-ended. All three now print clear guidance (what aipass + is, what drone is, how to reach each). Kernel + navmap prompts updated so + agents know the exception. (built by @drone + @aipass) + +--- + ## [2026-07-06] ### Fixed diff --git a/setup.sh b/setup.sh index a4a90d93..a654af34 100755 --- a/setup.sh +++ b/setup.sh @@ -662,6 +662,7 @@ else: # UserPromptSubmit: 5 separate entries (EventType:hook_name) to avoid output merging # PreToolUse, PostToolUse, SubagentStop, Stop, Notification: single aggregate entries # PreCompact: 3 hooks x 2 matchers (manual + auto) = 6 entries +# SessionStart: cadence reset on startup/clear (handler skips resume itself) aipass_hooks = { "UserPromptSubmit": [ {"hooks": [{"type": "command", "command": f"{bridge} UserPromptSubmit:tier0_kernel"}]}, @@ -696,6 +697,9 @@ aipass_hooks = { {"matcher": "manual", "hooks": [{"type": "command", "command": f"{bridge} PreCompact:auto_process", "timeout": 120}]}, {"matcher": "auto", "hooks": [{"type": "command", "command": f"{bridge} PreCompact:auto_process", "timeout": 120}]}, ], + "SessionStart": [ + {"hooks": [{"type": "command", "command": f"{bridge} SessionStart:cadence_reset", "timeout": 30}]}, + ], } # Merge, don't replace (DPLAN-0234 Strand C): refresh every AIPass bridge entry diff --git a/src/aipass/aipass/apps/aipass.py b/src/aipass/aipass/apps/aipass.py index 1c4a5daf..36664c53 100644 --- a/src/aipass/aipass/apps/aipass.py +++ b/src/aipass/aipass/apps/aipass.py @@ -106,6 +106,14 @@ def main(): if route_command(command, remaining, modules): return 0 + if command.startswith("@"): + print(f"{command} is a drone routing target, not an aipass command.") + print("aipass is your front-door CLI; drone is the agent router — two separate tools.") + print() + print(f" Reach an agent: drone {command} ... · drone systems") + print(" aipass commands: aipass --help") + return 1 + print(f"Unknown command: {command}") return 1 diff --git a/src/aipass/aipass/tests/test_aipass_main.py b/src/aipass/aipass/tests/test_aipass_main.py index 9f15c655..fa5473bf 100644 --- a/src/aipass/aipass/tests/test_aipass_main.py +++ b/src/aipass/aipass/tests/test_aipass_main.py @@ -252,6 +252,38 @@ class TestMain: assert result == 0 mod.handle_command.assert_called_once_with("doctor", []) + def test_at_prefix_shows_drone_guidance(self) -> None: + """@drone prints guidance pointing to drone, not 'Unknown command'.""" + with patch("aipass.aipass.apps.aipass.sys.argv", ["aipass", "@drone"]): + with patch("aipass.aipass.apps.aipass.discover_modules", return_value=[]): + with patch("builtins.print") as mock_print: + result = main() + assert result == 1 + printed = " ".join(str(a) for call in mock_print.call_args_list for a in call[0]) + assert "@drone" in printed + assert "drone routing target" in printed + assert "Unknown command" not in printed + + def test_at_prefix_uses_actual_name(self) -> None: + """@memory prints guidance with the actual @name the user typed.""" + with patch("aipass.aipass.apps.aipass.sys.argv", ["aipass", "@memory"]): + with patch("aipass.aipass.apps.aipass.discover_modules", return_value=[]): + with patch("builtins.print") as mock_print: + result = main() + assert result == 1 + printed = " ".join(str(a) for call in mock_print.call_args_list for a in call[0]) + assert "@memory" in printed + assert "drone @memory" in printed + + def test_plain_bad_command_still_unknown(self) -> None: + """Non-@ bad command still prints 'Unknown command', not drone guidance.""" + with patch("aipass.aipass.apps.aipass.sys.argv", ["aipass", "frobnicate"]): + with patch("aipass.aipass.apps.aipass.discover_modules", return_value=[]): + with patch("builtins.print") as mock_print: + result = main() + assert result == 1 + mock_print.assert_called_with("Unknown command: frobnicate") + def test_command_with_remaining_args(self) -> None: """Remaining args are passed to route_command.""" mod = MagicMock() diff --git a/src/aipass/drone/apps/drone.py b/src/aipass/drone/apps/drone.py index 02d364f2..82c52c3e 100644 --- a/src/aipass/drone/apps/drone.py +++ b/src/aipass/drone/apps/drone.py @@ -568,6 +568,19 @@ def main() -> int: if command == "rm": return _handle_rm(args[1:]) + # aipass is a user-facing CLI, not a drone-routable branch + if command.lstrip("@") == "aipass": + err_console.print( + "aipass isn't reachable through drone — it's your own front-door CLI," + " the AIPass concierge (onboarding, doctor, help, OS/system questions)." + " drone routes the agent citizens (@git, @devpulse, @memory...);" + " aipass is separate and serves you directly.\n" + "\n" + " Use aipass: aipass · aipass --help\n" + " See agents: drone systems" + ) + return 1 + # @target — route to branch or module if command.startswith("@"): return _handle_target(args) diff --git a/src/aipass/drone/tests/test_cli_routing.py b/src/aipass/drone/tests/test_cli_routing.py index 1f95aaea..872f437d 100644 --- a/src/aipass/drone/tests/test_cli_routing.py +++ b/src/aipass/drone/tests/test_cli_routing.py @@ -839,3 +839,67 @@ class TestCliEntryPoint: ): cli_main() assert exc_info.value.code == 0 + + +# =========================================================================== +# aipass intercept — drone aipass / drone @aipass +# =========================================================================== + + +class TestAipassIntercept: + """'aipass' is a user CLI, not a drone-routable branch.""" + + def test_bare_aipass_shows_guidance(self, capsys: pytest.CaptureFixture[str]) -> None: + """'drone aipass' prints guidance to stderr.""" + from aipass.drone.apps.drone import main + + with patch("sys.argv", ["drone", "aipass"]): + result = main() + assert result == 1 + captured = capsys.readouterr() + assert "aipass isn't reachable through drone" in captured.err + assert "aipass --help" in captured.err + + def test_at_aipass_shows_guidance(self, capsys: pytest.CaptureFixture[str]) -> None: + """'drone @aipass' prints guidance to stderr.""" + from aipass.drone.apps.drone import main + + with patch("sys.argv", ["drone", "@aipass"]): + result = main() + assert result == 1 + captured = capsys.readouterr() + assert "aipass isn't reachable through drone" in captured.err + assert "drone systems" in captured.err + + def test_bare_aipass_no_traceback(self, capsys: pytest.CaptureFixture[str]) -> None: + """No python traceback leaks on 'drone aipass'.""" + from aipass.drone.apps.drone import main + + with patch("sys.argv", ["drone", "aipass"]): + result = main() + assert result == 1 + captured = capsys.readouterr() + assert "Traceback" not in captured.err + assert "ModuleNotFoundError" not in captured.err + + def test_at_aipass_no_at_misdirect(self, capsys: pytest.CaptureFixture[str]) -> None: + """No 'use @aipass' misdirect on 'drone @aipass'.""" + from aipass.drone.apps.drone import main + + with patch("sys.argv", ["drone", "@aipass"]): + result = main() + assert result == 1 + captured = capsys.readouterr() + assert "Use '@aipass'" not in captured.err + + def test_real_branch_still_routes(self) -> None: + """Real branches still route normally after aipass intercept.""" + from aipass.drone.apps.drone import main + + with ( + patch("sys.argv", ["drone", "@git", "status"]), + patch(f"{_DRONE}.is_module", return_value=True), + patch(f"{_DRONE}.route_module_command", return_value={"stdout": "ok", "stderr": "", "exit_code": 0}), + ): + result = main() + assert result == 0 diff --git a/src/aipass/hooks/.seedgo/bypass.json b/src/aipass/hooks/.seedgo/bypass.json index 0535f491..4374e276 100644 --- a/src/aipass/hooks/.seedgo/bypass.json +++ b/src/aipass/hooks/.seedgo/bypass.json @@ -241,6 +241,21 @@ "standard": "json_structure", "reason": "Delegates to @memory's auto_process() via importlib \u2014 no direct JSON file ops needing json_handler." }, + { + "file": "apps/handlers/lifecycle/session_start.py", + "standard": "dead_code", + "reason": "Invoked dynamically by engine via importlib from hooks.json handler path 'aipass.hooks.apps.handlers.lifecycle.session_start.handle' \u2014 not statically imported by design. Wired in SessionStart.cadence_reset." + }, + { + "file": "apps/handlers/lifecycle/session_start.py", + "standard": "unused_function", + "reason": "handle() called dynamically by engine._run_handler via importlib.import_module + getattr from hooks.json. Wired in SessionStart.cadence_reset." + }, + { + "file": "apps/handlers/lifecycle/session_start.py", + "standard": "json_structure", + "reason": "Delegates to cadence.reset_counter() via importlib \u2014 no direct JSON file ops needing json_handler." + }, { "file": "apps/modules/cadence.py", "standard": "dead_code", @@ -1068,6 +1083,26 @@ "file": "tests/test_session_boot.py", "standard": "encapsulation", "reason": "Tests import handlers directly to test implementation details." + }, + { + "file": "tests/test_session_start.py", + "standard": "architecture", + "reason": "Test files live in tests/, not in the 3-layer apps structure." + }, + { + "file": "tests/test_session_start.py", + "standard": "documentation", + "reason": "Test methods use descriptive names as documentation per pytest convention." + }, + { + "file": "tests/test_session_start.py", + "standard": "encapsulation", + "reason": "Tests import handlers directly to test implementation details." + }, + { + "file": "tests/test_session_start.py", + "standard": "meta", + "reason": "Test files do not need Version/Modified metadata headers." } ], "notes": { diff --git a/src/aipass/hooks/README.md b/src/aipass/hooks/README.md index c3bf993e..dc153e09 100644 --- a/src/aipass/hooks/README.md +++ b/src/aipass/hooks/README.md @@ -73,7 +73,8 @@ src/aipass/hooks/ │ │ │ ├── auto_fix.py # Post-edit diagnostics (ruff, pyright, py_compile) │ │ │ ├── auto_watchdog.py # Watchdog arming after dispatch │ │ │ ├── compact.py # Pre-compact memory archival -│ │ │ └── rollover.py # Pre-compact memory rollover +│ │ │ ├── rollover.py # Pre-compact memory rollover +│ │ │ └── session_start.py # Cadence reset on new chat / clear (SessionStart) │ │ └── notification/ # Sound/alert hooks │ │ ├── announce.py # Announcement tone on notification │ │ ├── email.py # Inbox check on prompt diff --git a/src/aipass/hooks/apps/handlers/lifecycle/session_start.py b/src/aipass/hooks/apps/handlers/lifecycle/session_start.py new file mode 100644 index 00000000..d8bb21a3 --- /dev/null +++ b/src/aipass/hooks/apps/handlers/lifecycle/session_start.py @@ -0,0 +1,41 @@ +# =================== AIPass ==================== +# Name: session_start.py +# Version: 1.0.0 +# Description: Resets cadence counter on new chat / clear (SessionStart) +# Branch: hooks +# Layer: apps/handlers/lifecycle +# Created: 2026-07-07 +# Modified: 2026-07-07 +# ============================================= + +"""Resets cadence counter on SessionStart so loaders re-fire at turn 0. + +Fires on source=startup (new chat) and source=clear (/clear). +Skips source=resume — restored context already carries grounding. +source=compact is already handled by PreCompact; a duplicate reset is +harmless (idempotent), so we allow it rather than adding a fragile gate. +""" + +import importlib + +from aipass.prax.apps.modules.logger import system_logger as logger + +_SKIP_SOURCES = frozenset({"resume"}) + + +def handle(hook_data: dict) -> dict: + """Reset cadence counter unless this is a resume.""" + source = hook_data.get("source", "") + + if source in _SKIP_SOURCES: + logger.info("[HOOKS] session_start: skipped cadence reset (source=%s)", source) + return {"stdout": "", "exit_code": 0} + + try: + cadence = importlib.import_module("aipass.hooks.apps.modules.cadence") + cadence.reset_counter(hook_data=hook_data) + logger.info("[HOOKS] session_start: cadence reset (source=%s)", source) + except Exception as exc: + logger.info("[HOOKS] session_start: cadence reset failed: %s", exc) + + return {"stdout": "", "exit_code": 0} diff --git a/src/aipass/hooks/apps/modules/cadence.py b/src/aipass/hooks/apps/modules/cadence.py index 5084e3ee..31c9cc96 100644 --- a/src/aipass/hooks/apps/modules/cadence.py +++ b/src/aipass/hooks/apps/modules/cadence.py @@ -45,7 +45,7 @@ DEFAULTS = { "enabled": True, "period": 5, "loaders": { - "tier0": {"period": 1}, + "tier0": {"period": 5, "offset": 0}, "navmap": {"period": 5, "offset": 0}, "branch": {"offset": 0}, }, diff --git a/src/aipass/hooks/tests/test_cadence.py b/src/aipass/hooks/tests/test_cadence.py index 1a92bce1..3b1e4afd 100644 --- a/src/aipass/hooks/tests/test_cadence.py +++ b/src/aipass/hooks/tests/test_cadence.py @@ -332,7 +332,7 @@ class TestConfig: with patch(f"{MODULE}._CONFIG_PATH", tmp_path / "nonexistent.json"): config = _load_config() - assert config["loaders"]["tier0"]["period"] == 1 + assert config["loaders"]["tier0"]["period"] == 5 assert config["loaders"]["navmap"]["period"] == 5 assert config["loaders"]["navmap"]["offset"] == 0 @@ -709,7 +709,9 @@ class TestPostCompactDeterminism: def test_compact_handler_calls_reset_with_hook_data(self): from aipass.hooks.apps.handlers.lifecycle.compact import handle - hook_data = {"cwd": "/tmp/fake", "session_id": "test-123"} + import tempfile + + hook_data = {"cwd": tempfile.gettempdir() + "/fake", "session_id": "test-123"} with ( patch("importlib.import_module") as mock_import, diff --git a/src/aipass/hooks/tests/test_session_start.py b/src/aipass/hooks/tests/test_session_start.py new file mode 100644 index 00000000..67d4b485 --- /dev/null +++ b/src/aipass/hooks/tests/test_session_start.py @@ -0,0 +1,216 @@ +# =================== AIPass ==================== +# Name: test_session_start.py +# Version: 1.0.0 +# Description: Tests for SessionStart cadence reset handler +# Branch: hooks +# Created: 2026-07-07 +# Modified: 2026-07-07 +# ============================================= + +"""Tests for apps/handlers/lifecycle/session_start.py.""" + +import json +import os +from unittest.mock import patch + +CADENCE_MODULE = "aipass.hooks.apps.modules.cadence" + + +def _reset_cadence_globals(): + import aipass.hooks.apps.modules.cadence as mod + + mod._turn = None + mod._config = None + + +def _write_state(tmp_path, turn, session="test-session"): + import time + + state_file = tmp_path / f"aipass-cadence-{session}.json" + state_file.write_text(json.dumps({"turn": turn, "token": -1})) + old = time.time() - 10 + os.utime(state_file, (old, old)) + return state_file + + +class TestSessionStartHandler: + def setup_method(self): + _reset_cadence_globals() + + def test_startup_resets_cadence(self, tmp_path): + from aipass.hooks.apps.handlers.lifecycle.session_start import handle + + state_file = _write_state(tmp_path, turn=7) + + with ( + patch(f"{CADENCE_MODULE}._GUARD_DIR", tmp_path), + patch.dict("os.environ", {"CLAUDE_CODE_SESSION_ID": "test-session"}), + ): + result = handle({"source": "startup", "session_id": "test-session"}) + + assert result["exit_code"] == 0 + data = json.loads(state_file.read_text()) + assert data["turn"] == -1 + + def test_clear_resets_cadence(self, tmp_path): + from aipass.hooks.apps.handlers.lifecycle.session_start import handle + + state_file = _write_state(tmp_path, turn=3) + + with ( + patch(f"{CADENCE_MODULE}._GUARD_DIR", tmp_path), + patch.dict("os.environ", {"CLAUDE_CODE_SESSION_ID": "test-session"}), + ): + result = handle({"source": "clear", "session_id": "test-session"}) + + assert result["exit_code"] == 0 + data = json.loads(state_file.read_text()) + assert data["turn"] == -1 + + def test_resume_skips_reset(self, tmp_path): + from aipass.hooks.apps.handlers.lifecycle.session_start import handle + + state_file = _write_state(tmp_path, turn=7) + + with ( + patch(f"{CADENCE_MODULE}._GUARD_DIR", tmp_path), + patch.dict("os.environ", {"CLAUDE_CODE_SESSION_ID": "test-session"}), + ): + result = handle({"source": "resume", "session_id": "test-session"}) + + assert result["exit_code"] == 0 + data = json.loads(state_file.read_text()) + assert data["turn"] == 7 + + def test_compact_source_resets(self, tmp_path): + """source=compact is idempotent with PreCompact — allowed.""" + from aipass.hooks.apps.handlers.lifecycle.session_start import handle + + state_file = _write_state(tmp_path, turn=5) + + with ( + patch(f"{CADENCE_MODULE}._GUARD_DIR", tmp_path), + patch.dict("os.environ", {"CLAUDE_CODE_SESSION_ID": "test-session"}), + ): + result = handle({"source": "compact", "session_id": "test-session"}) + + assert result["exit_code"] == 0 + data = json.loads(state_file.read_text()) + assert data["turn"] == -1 + + def test_empty_source_resets(self, tmp_path): + from aipass.hooks.apps.handlers.lifecycle.session_start import handle + + state_file = _write_state(tmp_path, turn=4) + + with ( + patch(f"{CADENCE_MODULE}._GUARD_DIR", tmp_path), + patch.dict("os.environ", {"CLAUDE_CODE_SESSION_ID": "test-session"}), + ): + result = handle({"session_id": "test-session"}) + + assert result["exit_code"] == 0 + data = json.loads(state_file.read_text()) + assert data["turn"] == -1 + + def test_no_stdout_output(self): + from aipass.hooks.apps.handlers.lifecycle.session_start import handle + + with patch("importlib.import_module"): + result = handle({"source": "startup"}) + + assert result["stdout"] == "" + + def test_cadence_import_failure_does_not_crash(self): + from aipass.hooks.apps.handlers.lifecycle.session_start import handle + + with patch("importlib.import_module", side_effect=ImportError("boom")): + result = handle({"source": "startup"}) + + assert result["exit_code"] == 0 + + +class TestSessionStartCadenceIntegration: + """End-to-end: SessionStart reset -> next turn fires all loaders.""" + + def setup_method(self): + _reset_cadence_globals() + + def test_clear_then_all_loaders_fire(self, tmp_path): + from aipass.hooks.apps.handlers.lifecycle.session_start import handle + from aipass.hooks.apps.modules.cadence import should_fire + + config = tmp_path / "cadence.json" + config.write_text( + json.dumps( + { + "enabled": True, + "period": 5, + "loaders": { + "tier0": {"period": 5, "offset": 0}, + "navmap": {"period": 5, "offset": 0}, + "branch": {"offset": 0}, + }, + } + ) + ) + + _write_state(tmp_path, turn=3) + + with ( + patch(f"{CADENCE_MODULE}._GUARD_DIR", tmp_path), + patch.dict("os.environ", {"CLAUDE_CODE_SESSION_ID": "test-session"}), + patch(f"{CADENCE_MODULE}._CONFIG_PATH", config), + ): + handle({"source": "clear", "session_id": "test-session"}) + + _reset_cadence_globals() + + with ( + patch(f"{CADENCE_MODULE}._GUARD_DIR", tmp_path), + patch.dict("os.environ", {"CLAUDE_CODE_SESSION_ID": "test-session"}), + patch(f"{CADENCE_MODULE}._CONFIG_PATH", config), + ): + assert should_fire("tier0") is True + _reset_cadence_globals() + assert should_fire("navmap") is True + _reset_cadence_globals() + assert should_fire("branch") is True + + def test_resume_does_not_reset_counter_continues(self, tmp_path): + from aipass.hooks.apps.handlers.lifecycle.session_start import handle + from aipass.hooks.apps.modules.cadence import should_fire + + config = tmp_path / "cadence.json" + config.write_text( + json.dumps( + { + "enabled": True, + "period": 5, + "loaders": { + "tier0": {"period": 5, "offset": 0}, + "navmap": {"period": 5, "offset": 0}, + }, + } + ) + ) + + _write_state(tmp_path, turn=2) + + with ( + patch(f"{CADENCE_MODULE}._GUARD_DIR", tmp_path), + patch.dict("os.environ", {"CLAUDE_CODE_SESSION_ID": "test-session"}), + patch(f"{CADENCE_MODULE}._CONFIG_PATH", config), + ): + handle({"source": "resume", "session_id": "test-session"}) + + _reset_cadence_globals() + + with ( + patch(f"{CADENCE_MODULE}._GUARD_DIR", tmp_path), + patch.dict("os.environ", {"CLAUDE_CODE_SESSION_ID": "test-session"}), + patch(f"{CADENCE_MODULE}._CONFIG_PATH", config), + ): + assert should_fire("tier0") is False + _reset_cadence_globals() + assert should_fire("navmap") is False From a20d191f87c86f9e6de53b5969a8b02046c00bca Mon Sep 17 00:00:00 2001 From: AIOSAI Date: Tue, 7 Jul 2026 20:07:32 -0700 Subject: [PATCH 03/73] =?UTF-8?q?tests:=20dev-docker=20verify=20script=20(?= =?UTF-8?q?bridge-era,=2019=20assertions)=20=E2=80=94=20proven=20vs=20real?= =?UTF-8?q?=20dev=20clone;=20supersedes=20stale=20docker=5Fclone=5Ftest.sh?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 6 +- tests/docker_dev_verify.sh | 139 +++++++++++++++++++++++++++++++++++++ 2 files changed, 143 insertions(+), 2 deletions(-) create mode 100755 tests/docker_dev_verify.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index 385135b6..fc02bf5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,8 +21,10 @@ PyPI version — not the changelog header. gets full grounding, then every 5th turn after. Wired end-to-end: handler (`session_start.py`), project config (`.aipass/hooks.json` + the `project_hooks.json` template for external projects), and `setup.sh` seeds - the provider `SessionStart` entry for new installs. (built by @hooks + - @devpulse) + the provider `SessionStart` entry for new installs. Proven end-to-end from a + real fresh-user clone of dev in Docker — 19/19 assertions via the new + `tests/docker_dev_verify.sh` (bridge-era; supersedes the stale + `docker_clone_test.sh`). (built by @hooks + @devpulse) ### Fixed diff --git a/tests/docker_dev_verify.sh b/tests/docker_dev_verify.sh new file mode 100755 index 00000000..b5d9214e --- /dev/null +++ b/tests/docker_dev_verify.sh @@ -0,0 +1,139 @@ +#!/usr/bin/env bash +# +# Dev-Docker verify — SOP artifact for PPLAN dev-docker runs. +# Runs INSIDE the container (aipass-test image): real GitHub clone of the +# dev branch, one-command install, then asserts provider hook wiring and +# live-fires the SessionStart cadence reset + misroute guidance. +# +# Host invocation: +# docker run --rm -v "$AIPASS_HOME/tests/docker_dev_verify.sh":/verify.sh:ro \ +# aipass-test:latest bash /verify.sh +# +# Supersedes docker_clone_test.sh (pre-bridge architecture, stale). +# +set -uo pipefail + +PASS=0 +FAIL=0 +ok() { echo " OK $1"; PASS=$((PASS+1)); } +bad() { echo " FAIL $1"; FAIL=$((FAIL+1)); } + +echo "=========================================" +echo " AIPass Dev-Docker Verify (bridge era)" +echo "=========================================" + +# --- Phase 1: real clone of dev --- +echo "--- Phase 1: clone dev from GitHub ---" +rm -rf "$HOME/workspace" && mkdir -p "$HOME/workspace" && cd "$HOME/workspace" +if git clone -b dev --depth 1 https://github.com/AIOSAI/AIPass.git 2>&1 | tail -2; then + ok "clone dev" +else + bad "clone dev" + echo "Cannot continue without a clone." + exit 1 +fi +cd AIPass +echo " HEAD: $(git log -1 --oneline)" + +# --- Phase 2: one-command install --- +echo "--- Phase 2: ./aipass install ---" +if ./aipass install 2>&1 | tail -15; then + ok "installer exit 0" +else + bad "installer exited non-zero" +fi + +SETTINGS="$HOME/.claude/settings.json" +AH="$HOME/workspace/AIPass" +VPY="$AH/.venv/bin/python3" +BRIDGE="$AH/src/aipass/hooks/apps/handlers/bridges/claude.py" + +# --- Phase 3: provider settings assertions --- +echo "--- Phase 3: provider settings ---" +if [ -f "$SETTINGS" ]; then ok "settings.json exists"; else bad "settings.json missing"; fi + +if jq -e '.hooks.SessionStart' "$SETTINGS" > /dev/null 2>&1; then + ok "SessionStart event wired" +else + bad "SessionStart event missing" +fi + +SS_CMD=$(jq -r '.hooks.SessionStart[0].hooks[0].command // ""' "$SETTINGS" 2>/dev/null) +case "$SS_CMD" in + *"bridges/claude.py SessionStart:cadence_reset"*) ok "SessionStart command = bridge cadence_reset" ;; + *) bad "SessionStart command wrong: $SS_CMD" ;; +esac + +SS_TO=$(jq -r '.hooks.SessionStart[0].hooks[0].timeout // 0' "$SETTINGS" 2>/dev/null) +if [ "$SS_TO" = "30" ]; then ok "SessionStart timeout 30"; else bad "SessionStart timeout: $SS_TO"; fi + +if jq -e '.env.AIPASS_HOME' "$SETTINGS" > /dev/null 2>&1; then + ok "AIPASS_HOME in settings env" +else + bad "AIPASS_HOME missing from settings env" +fi + +UPS=$(jq -r '.hooks.UserPromptSubmit | length' "$SETTINGS" 2>/dev/null || echo 0) +if [ "$UPS" -ge 6 ]; then ok "UserPromptSubmit: $UPS entries"; else bad "UserPromptSubmit: $UPS entries (want >=6)"; fi + +PC=$(jq -r '.hooks.PreCompact | length' "$SETTINGS" 2>/dev/null || echo 0) +if [ "$PC" -eq 6 ]; then ok "PreCompact: 6 entries"; else bad "PreCompact: $PC entries (want 6)"; fi + +# --- Phase 4: project hook config --- +echo "--- Phase 4: project hook config ---" +if jq -e '.SessionStart.cadence_reset.enabled == true' "$AH/.aipass/hooks.json" > /dev/null 2>&1; then + ok ".aipass/hooks.json SessionStart.cadence_reset enabled" +else + bad ".aipass/hooks.json SessionStart.cadence_reset missing/disabled" +fi +if jq -e '.SessionStart.cadence_reset.enabled == true' "$AH/.aipass/project_hooks.json" > /dev/null 2>&1; then + ok "project_hooks.json template has SessionStart" +else + bad "project_hooks.json template missing SessionStart" +fi + +# --- Phase 5: live-fire cadence reset --- +echo "--- Phase 5: live-fire SessionStart ---" +export AIPASS_HOME="$AH" +TMPD=$("$VPY" -c "import tempfile; print(tempfile.gettempdir())") + +echo '{"source":"startup","session_id":"dockerstartup"}' | "$VPY" "$BRIDGE" SessionStart:cadence_reset +TURN=$(jq -r '.turn // "none"' "$TMPD/aipass-cadence-dockerstartup.json" 2>/dev/null || echo "none") +if [ "$TURN" = "-1" ]; then ok "startup reset -> turn -1"; else bad "startup reset: turn=$TURN"; fi + +echo '{"source":"resume","session_id":"dockerresume"}' | "$VPY" "$BRIDGE" SessionStart:cadence_reset +if [ ! -f "$TMPD/aipass-cadence-dockerresume.json" ]; then + ok "resume skipped (no state written)" +else + bad "resume wrote state (should skip)" +fi + +PERIODS=$("$VPY" -c " +from aipass.hooks.apps.modules.cadence import _load_config +c = _load_config() +t = c['loaders']['tier0'].get('period', c['period']) +n = c['loaders']['navmap'].get('period', c['period']) +print(t, n)" 2>/dev/null) +if [ "$PERIODS" = "5 5" ]; then ok "cadence periods tier0=5 navmap=5"; else bad "cadence periods: $PERIODS (want '5 5')"; fi + +# --- Phase 6: misroute guidance (guide, never crash) --- +echo "--- Phase 6: misroute guidance ---" +DRONE="$AH/.venv/bin/drone" +AIPASS_BIN="$AH/.venv/bin/aipass" + +OUT=$("$DRONE" aipass 2>&1 || true) +if echo "$OUT" | grep -qi "traceback"; then bad "'drone aipass' crashed"; else ok "'drone aipass' no crash"; fi +if echo "$OUT" | grep -qi "aipass"; then ok "'drone aipass' mentions aipass guidance"; else bad "'drone aipass' output unhelpful"; fi + +OUT=$("$DRONE" @aipass 2>&1 || true) +if echo "$OUT" | grep -qi "traceback"; then bad "'drone @aipass' crashed"; else ok "'drone @aipass' no crash"; fi + +OUT=$("$AIPASS_BIN" @drone 2>&1 || true) +if echo "$OUT" | grep -qi "traceback"; then bad "'aipass @drone' crashed"; else ok "'aipass @drone' no crash"; fi +if echo "$OUT" | grep -qi "drone"; then ok "'aipass @drone' mentions drone guidance"; else bad "'aipass @drone' output unhelpful"; fi + +# --- Summary --- +echo "=========================================" +echo " Results: $PASS passed, $FAIL failed" +echo "=========================================" +[ "$FAIL" -eq 0 ] From 195b9f081c2fc93665b9a58a857bd99544e46a8a Mon Sep 17 00:00:00 2001 From: AIOSAI Date: Tue, 7 Jul 2026 21:18:39 -0700 Subject: [PATCH 04/73] =?UTF-8?q?backup:=20drive-sync=20respects=20.backup?= =?UTF-8?q?ignore=20on=20sync=20path=20+=20PosixPath=20log=20fix=20?= =?UTF-8?q?=E2=80=94=20stale-store=20junk=20can't=20cause=208hr=20syncs=20?= =?UTF-8?q?(built=20by=20@backup)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .aipass/tier1_navmap.md | 2 +- CHANGELOG.md | 13 ++++ .../backup/apps/handlers/json/json_handler.py | 2 +- src/aipass/backup/apps/modules/drive_sync.py | 11 ++- .../backup/tests/test_drive_pipeline.py | 76 +++++++++++++++++-- src/aipass/backup/tests/test_json_handler.py | 19 +++++ 6 files changed, 114 insertions(+), 9 deletions(-) diff --git a/.aipass/tier1_navmap.md b/.aipass/tier1_navmap.md index ae0c557d..a3e3d62a 100644 --- a/.aipass/tier1_navmap.md +++ b/.aipass/tier1_navmap.md @@ -55,7 +55,7 @@ src/aipass// - @skills — capability framework. Discoverable, self-contained skill units any agent can run; consume AIPass services as opt-in imports (e.g. the Telegram skill). - @daemon — task scheduler. Cron-triggered firing; each branch owns its `.daemon/schedule.json`, the daemon discovers and fires. - @commons — the social space. Where branches post, comment, vote, and gather as a community. - - @backup — local-first backups. Snapshots + versioning + restore for any directory; optional Google Drive sync (planned). `.backup/` is a shared runtime namespace — @memory rollover and @flow (plan archive) also write there. + - @backup — local-first backups. Snapshots + versioning + restore for any directory; optional Google Drive sync (live, per-file mirror — slow on huge file counts, respect `.backupignore`). `.backup/` is a shared runtime namespace — @memory rollover and @flow (plan archive) also write there. # Daily commands diff --git a/CHANGELOG.md b/CHANGELOG.md index fc02bf5a..660de090 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,19 @@ PyPI version — not the changelog header. ## [2026-07-07] +### Fixed + +- **Drive sync now respects `.backupignore` on the sync path.** The ignore spec + was applied at backup time only — anything already inside `.backup/versioned/` + got uploaded regardless. Real case: Vera-Studio's store carried 37K legacy + `node_modules` files (92% of the store), turning a KB-sized sync into a 7-8 + hour crawl (Drive uploads are per-file API round-trips — latency-bound, not + bandwidth-bound; the clean store syncs in ~13 min). `drive_sync` now re-filters + store files through the project's `.backupignore` before upload and logs the + ignored count. Also fixed: `json_handler.log_operation` crashed on `Path` + objects (`PosixPath is not JSON serializable`) — now serializes with + `default=str`. 2 new tests, backup suite 247 green. (built by @backup) + ### Added - **Fresh-context grounding: cadence reset on new chat / clear / compact.** diff --git a/src/aipass/backup/apps/handlers/json/json_handler.py b/src/aipass/backup/apps/handlers/json/json_handler.py index f8998939..b95631ca 100644 --- a/src/aipass/backup/apps/handlers/json/json_handler.py +++ b/src/aipass/backup/apps/handlers/json/json_handler.py @@ -29,7 +29,7 @@ def log_operation(operation: str, data: dict) -> None: log_file = log_dir / "operations.jsonl" try: with open(log_file, "a", encoding="utf-8") as f: - f.write(json.dumps(entry) + "\n") + f.write(json.dumps(entry, default=str) + "\n") except OSError as e: logger.warning(f"Failed to write operation log: {e}") diff --git a/src/aipass/backup/apps/modules/drive_sync.py b/src/aipass/backup/apps/modules/drive_sync.py index d84f2e35..7e769765 100644 --- a/src/aipass/backup/apps/modules/drive_sync.py +++ b/src/aipass/backup/apps/modules/drive_sync.py @@ -30,6 +30,7 @@ if sys.platform == "win32": from aipass.prax import logger from aipass.cli.apps.modules import console +from aipass.backup.apps.handlers.ignore.patterns import is_ignored, load_spec from aipass.backup.apps.handlers.json import json_handler from aipass.backup.apps.handlers.path.builder import build_versioned_store from aipass.backup.apps.modules.display import show_drive_result @@ -116,8 +117,14 @@ def run_drive_sync( logger.warning(f"[backup] {result['error']}") return result - # 3. Scan for ALL files (no dotfile filter — the store is already filtered by .backupignore) - all_files = [f for f in store_path.rglob("*") if f.is_file()] + # 3. Scan store and re-filter through .backupignore (legacy stores may + # contain files swept in before an ignore rule was added). + spec = load_spec(str(project_root)) + raw_files = [f for f in store_path.rglob("*") if f.is_file()] + all_files = [f for f in raw_files if not is_ignored(str(f.relative_to(store_path)), spec)] + ignored_count = len(raw_files) - len(all_files) + if ignored_count: + logger.info(f"[backup] Drive sync: filtered {ignored_count} ignored files from store") result["total"] = len(all_files) diff --git a/src/aipass/backup/tests/test_drive_pipeline.py b/src/aipass/backup/tests/test_drive_pipeline.py index 364ae5fa..4141a71a 100644 --- a/src/aipass/backup/tests/test_drive_pipeline.py +++ b/src/aipass/backup/tests/test_drive_pipeline.py @@ -549,13 +549,13 @@ class TestDriveUpload: result = mod.upload_single_file(client, missing, "testproj", tmp_path) assert result is False - def test_upload_batch_empty(self) -> None: + def test_upload_batch_empty(self, tmp_path: Path) -> None: """Empty file list returns success immediately.""" mod = _fresh_import("aipass.backup.apps.handlers.drive.upload") client_mod = _fresh_import("aipass.backup.apps.handlers.drive.client") client = client_mod.DriveClient() - result = mod.upload_batch(client, [], "proj", Path("/tmp"), {}) + result = mod.upload_batch(client, [], "proj", tmp_path, {}) assert result["success"] is True assert result["uploaded"] == 0 assert result["failed"] == 0 @@ -603,7 +603,7 @@ class TestDriveUpload: mod = _fresh_import("aipass.backup.apps.handlers.drive.upload") client_mod = _fresh_import("aipass.backup.apps.handlers.drive.client") - mod.MEDIA_UPLOAD_AVAILABLE = False + mod.MEDIA_UPLOAD_AVAILABLE = False # type: ignore[attr-defined] client = client_mod.DriveClient() client._drive_service = MagicMock() @@ -813,6 +813,59 @@ class TestDriveSync: assert result["uploaded"] == 3 mock_upload_mod.upload_batch.assert_called_once() + def test_run_drive_sync_filters_ignored_files(self, tmp_path: Path) -> None: + """Files matching .backupignore are excluded from upload list.""" + project = tmp_path / "project" + project.mkdir() + bs = project / ".backup" / "versioned" + + (bs / "src" / "app.py" / "app.py").parent.mkdir(parents=True) + (bs / "src" / "app.py" / "app.py").write_text("code", encoding="utf-8") + (bs / "node_modules" / "pkg" / "index.js" / "index.js").parent.mkdir(parents=True) + (bs / "node_modules" / "pkg" / "index.js" / "index.js").write_text("junk", encoding="utf-8") + (bs / "node_modules" / "other" / "lib.js" / "lib.js").parent.mkdir(parents=True) + (bs / "node_modules" / "other" / "lib.js" / "lib.js").write_text("junk2", encoding="utf-8") + + ignore_file = project / ".backupignore" + ignore_file.write_text("node_modules/\n", encoding="utf-8") + + mod = _fresh_import("aipass.backup.apps.modules.drive_sync") + mock_class, mock_inst = self._make_mock_client_class(authenticate_rv=True) + mock_client_module = MagicMock() + mock_client_module.DriveClient = mock_class + + mock_tracker_mod = MagicMock() + mock_tracker_mod.load_tracker.return_value = {} + mock_tracker_mod.check_needs_upload.return_value = True + mock_tracker_mod.save_tracker = MagicMock() + + mock_upload_mod = MagicMock() + mock_upload_mod.upload_batch.return_value = { + "success": True, + "uploaded": 1, + "failed": 0, + } + + with ( + patch.dict( + sys.modules, + { + "aipass.backup.apps.handlers.drive.client": mock_client_module, + "aipass.backup.apps.handlers.drive.tracker": mock_tracker_mod, + "aipass.backup.apps.handlers.drive.upload": mock_upload_mod, + }, + ), + patch.object(mod, "build_versioned_store", return_value=bs), + ): + result = mod.run_drive_sync(str(project), show_panels=False) + + assert result["total"] == 1 + uploaded_files = mock_upload_mod.upload_batch.call_args[0][1] + names = [f.name for f in uploaded_files] + assert "app.py" in names + assert "index.js" not in names + assert "lib.js" not in names + def test_handle_command_help(self) -> None: """--help returns True.""" mod = _fresh_import("aipass.backup.apps.modules.drive_sync") @@ -838,14 +891,17 @@ class TestDriveCheckModule: """Tests for drive_check module.""" def test_handle_command_primary(self) -> None: + """Primary command returns True.""" mod = _fresh_import("aipass.backup.apps.modules.drive_check") assert mod.handle_command("drive_check", []) is True def test_handle_command_help(self) -> None: + """--help returns True.""" mod = _fresh_import("aipass.backup.apps.modules.drive_check") assert mod.handle_command("drive_check", ["--help"]) is True def test_handle_command_wrong(self) -> None: + """Wrong command returns False.""" mod = _fresh_import("aipass.backup.apps.modules.drive_check") assert mod.handle_command("wrong", []) is False @@ -879,14 +935,17 @@ class TestDriveStatsModule: """Tests for drive_stats module.""" def test_handle_command_primary(self) -> None: + """Primary command returns True.""" mod = _fresh_import("aipass.backup.apps.modules.drive_stats") assert mod.handle_command("drive_stats", []) is True def test_handle_command_help(self) -> None: + """--help returns True.""" mod = _fresh_import("aipass.backup.apps.modules.drive_stats") assert mod.handle_command("drive_stats", ["--help"]) is True def test_handle_command_wrong(self) -> None: + """Wrong command returns False.""" mod = _fresh_import("aipass.backup.apps.modules.drive_stats") assert mod.handle_command("wrong", []) is False @@ -913,21 +972,24 @@ class TestDriveClearModule: """Tests for drive_clear module.""" def test_handle_command_primary(self) -> None: + """Primary command returns True.""" mod = _fresh_import("aipass.backup.apps.modules.drive_clear") assert mod.handle_command("drive_clear", []) is True def test_handle_command_help(self) -> None: + """--help returns True.""" mod = _fresh_import("aipass.backup.apps.modules.drive_clear") assert mod.handle_command("drive_clear", ["--help"]) is True def test_handle_command_wrong(self) -> None: + """Wrong command returns False.""" mod = _fresh_import("aipass.backup.apps.modules.drive_clear") assert mod.handle_command("wrong", []) is False - def test_run_drive_clear_no_force(self) -> None: + def test_run_drive_clear_no_force(self, tmp_path: Path) -> None: """Without --force, returns False.""" mod = _fresh_import("aipass.backup.apps.modules.drive_clear") - result = mod.run_drive_clear("/tmp/project", force=False) + result = mod.run_drive_clear(str(tmp_path / "project"), force=False) assert result is False def test_run_drive_clear_with_force(self, tmp_path: Path) -> None: @@ -1101,24 +1163,28 @@ class TestCommandRouting: """Verify drive commands route by underscore names.""" def test_drive_sync_routes_underscore(self) -> None: + """drive_sync accepts underscore, rejects hyphen.""" mod = _fresh_import("aipass.backup.apps.modules.drive_sync") assert mod.PRIMARY_COMMAND == "drive_sync" assert mod.handle_command("drive_sync", []) is True assert mod.handle_command("drive-sync", []) is False def test_drive_check_routes_underscore(self) -> None: + """drive_check accepts underscore, rejects hyphen.""" mod = _fresh_import("aipass.backup.apps.modules.drive_check") assert mod.PRIMARY_COMMAND == "drive_check" assert mod.handle_command("drive_check", []) is True assert mod.handle_command("drive-check", []) is False def test_drive_stats_routes_underscore(self) -> None: + """drive_stats accepts underscore, rejects hyphen.""" mod = _fresh_import("aipass.backup.apps.modules.drive_stats") assert mod.PRIMARY_COMMAND == "drive_stats" assert mod.handle_command("drive_stats", []) is True assert mod.handle_command("drive-stats", []) is False def test_drive_clear_routes_underscore(self) -> None: + """drive_clear accepts underscore, rejects hyphen.""" mod = _fresh_import("aipass.backup.apps.modules.drive_clear") assert mod.PRIMARY_COMMAND == "drive_clear" assert mod.handle_command("drive_clear", []) is True diff --git a/src/aipass/backup/tests/test_json_handler.py b/src/aipass/backup/tests/test_json_handler.py index 6a2f9277..53bd42c0 100644 --- a/src/aipass/backup/tests/test_json_handler.py +++ b/src/aipass/backup/tests/test_json_handler.py @@ -127,6 +127,25 @@ class TestLogOperation: """ assert callable(json_handler.log_operation) + def test_log_operation_handles_path_objects(self, tmp_path: Path) -> None: + """log_operation serializes pathlib.Path values via default=str.""" + log_dir = tmp_path / "logs" + log_dir.mkdir() + with patch( + "aipass.backup.apps.handlers.json.json_handler.Path", + ) as mock_path: + mock_resolve = mock_path.return_value.resolve.return_value + mock_resolve.parents.__getitem__ = lambda self, i: tmp_path + mock_path.return_value.__truediv__ = Path.__truediv__ + json_handler.log_operation( + "test_op", + {"project_root": Path("/some/project")}, + ) + log_file = log_dir / "operations.jsonl" + if log_file.exists(): + entry = json.loads(log_file.read_text(encoding="utf-8").strip()) + assert entry["project_root"] == "/some/project" + class TestEnsureAndGetPath: """Token coverage for standard json_handler API that backup doesn't implement. From 08d87d95e98be33f7f8888d6228b755f191b6133 Mon Sep 17 00:00:00 2001 From: AIOSAI Date: Thu, 9 Jul 2026 11:35:53 -0700 Subject: [PATCH 05/73] =?UTF-8?q?hooks:=20macOS=20session=20lock-out=20fix?= =?UTF-8?q?=20=E2=80=94=20/proc=E2=86=92ps=20portable=20ancestry=20walk,?= =?UTF-8?q?=20actionable=20boot/presence-gate=20messages,=20dedupe=20doubl?= =?UTF-8?q?ed=20--permission-mode=20(built=20by=20@hooks)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 16 ++++ .../apps/handlers/lifecycle/session_boot.py | 44 ++++++--- .../apps/handlers/security/presence_gate.py | 6 +- src/aipass/hooks/tests/test_presence_gate.py | 3 +- src/aipass/hooks/tests/test_session_boot.py | 94 ++++++++++++++++++- 5 files changed, 143 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 660de090..009c1482 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,22 @@ PyPI version — not the changelog header. --- +## [2026-07-09] + +### Fixed + +- **macOS session lock-out: the boot wrapper can now see tmux sessions on + macOS.** `session_boot` decided whether a live Claude session lived inside + tmux by walking the process tree through `/proc//status` — Linux-only. + On macOS (no `/proc`) that walk always failed, so the wrapper concluded every + live session was "outside tmux" and refused to attach, locking the user out of + their own session in an unbreakable loop. Replaced the `/proc` read with a + portable `ps -o ppid=` ancestry walk (Linux + macOS). Also: both the boot + warning and the presence-gate block now spell out the exact recovery command + (`kill && claude`, `command claude --resume`) instead of a vague "kill it + first", and the wrapper no longer doubles `--permission-mode` when the user + passes it explicitly. New/updated tests, hooks suite 791 green. (built by @hooks) + ## [2026-07-07] ### Fixed diff --git a/src/aipass/hooks/apps/handlers/lifecycle/session_boot.py b/src/aipass/hooks/apps/handlers/lifecycle/session_boot.py index 68b583a5..9d431509 100644 --- a/src/aipass/hooks/apps/handlers/lifecycle/session_boot.py +++ b/src/aipass/hooks/apps/handlers/lifecycle/session_boot.py @@ -97,24 +97,34 @@ def _find_tmux_session_for_pid(pid: int) -> str | None: return None +def _get_ppid(pid: int) -> int | None: + """Get parent PID portably (Linux + macOS). Returns None on failure.""" + try: + result = subprocess.run( + ["ps", "-o", "ppid=", "-p", str(pid)], + capture_output=True, + text=True, + timeout=5, + ) + if result.returncode == 0 and result.stdout.strip(): + return int(result.stdout.strip()) + except (OSError, ValueError, subprocess.TimeoutExpired) as exc: + logger.info("[SESSION_BOOT] ppid lookup failed for PID %d: %s", pid, exc) + return None + + def _is_descendant(target_pid: int, ancestor_pid: int) -> bool: - """Check if target_pid is a descendant of ancestor_pid via /proc.""" + """Check if target_pid is a descendant of ancestor_pid via process tree walk.""" pid = target_pid for _ in range(20): if pid == ancestor_pid: return True if pid <= 1: return False - try: - for line in Path(f"/proc/{pid}/status").read_text().splitlines(): - if line.startswith("PPid:"): - pid = int(line.split()[1]) - break - else: - return False - except OSError as exc: - logger.info("[SESSION_BOOT] Cannot read /proc/%d/status: %s", pid, exc) + ppid = _get_ppid(pid) + if ppid is None: return False + pid = ppid return False @@ -132,9 +142,11 @@ def boot(cwd: str | None = None, extra_args: list[str] | None = None) -> dict: branch = Path(cwd).name claude_bin = _resolve_claude_binary() + defaults = _DEFAULT_ARGS if not (extra_args and "--permission-mode" in extra_args) else [] + if os.environ.get("TMUX"): logger.info("[SESSION_BOOT] Already inside tmux — running claude directly") - claude_cmd = [claude_bin] + _DEFAULT_ARGS + claude_cmd = [claude_bin] + defaults if extra_args: claude_cmd.extend(extra_args) os.execvp(claude_bin, claude_cmd) @@ -162,14 +174,20 @@ def boot(cwd: str | None = None, extra_args: list[str] | None = None) -> dict: return { "exit_code": 1, "action": "warn", - "error": f"Live session at PID {pid} is not in tmux. Kill it first or attach to its terminal.", + "error": ( + f"{branch} already has a live Claude session (PID {pid}) running outside tmux" + f" — Claude allows one session per branch.\n" + f" • Reattach in its own terminal, OR\n" + f" • Reclaim it here: kill {pid} && claude\n" + f" • Or bypass this wrapper: command claude --resume" + ), } if _tmux_session_exists(branch): logger.info("[SESSION_BOOT] Killing stale tmux session '%s'", branch) subprocess.run(["tmux", "kill-session", "-t", branch], check=False) - claude_cmd = [claude_bin] + _DEFAULT_ARGS + claude_cmd = [claude_bin] + defaults if extra_args: claude_cmd.extend(extra_args) diff --git a/src/aipass/hooks/apps/handlers/security/presence_gate.py b/src/aipass/hooks/apps/handlers/security/presence_gate.py index 36d5a688..20d31985 100644 --- a/src/aipass/hooks/apps/handlers/security/presence_gate.py +++ b/src/aipass/hooks/apps/handlers/security/presence_gate.py @@ -80,7 +80,11 @@ def handle(hook_data: dict) -> dict: occ_pid = occupant.get("pid", "?") occ_name = occupant.get("name", "") - reason = f"{branch} already live at PID {occ_pid}{f' ({occ_name})' if occ_name else ''} — attach, do not spawn." + reason = ( + f"{branch} is already live at PID {occ_pid}{f' ({occ_name})' if occ_name else ''}" + f" — Claude allows one session per branch." + f" Attach to that session, or run `kill {occ_pid}` to reclaim the branch, then retry." + ) logger.warning("[presence_gate] BLOCKED: %s", reason) return { "exit_code": 2, diff --git a/src/aipass/hooks/tests/test_presence_gate.py b/src/aipass/hooks/tests/test_presence_gate.py index a0cf924f..98a4cbbd 100644 --- a/src/aipass/hooks/tests/test_presence_gate.py +++ b/src/aipass/hooks/tests/test_presence_gate.py @@ -132,7 +132,8 @@ class TestHandle: result = presence_gate.handle({"cwd": str(branch_dir)}) parsed = json.loads(result["stdout"]) assert "devpulse" in parsed["reason"] - assert "attach" in parsed["reason"].lower() + assert "kill 5000" in parsed["reason"] + assert "one session per branch" in parsed["reason"].lower() def test_gate_error_allows(self): with patch.dict(os.environ, {"AIPASS_SESSION_TYPE": "interactive"}, clear=True): diff --git a/src/aipass/hooks/tests/test_session_boot.py b/src/aipass/hooks/tests/test_session_boot.py index 29a9f168..83d00a3c 100644 --- a/src/aipass/hooks/tests/test_session_boot.py +++ b/src/aipass/hooks/tests/test_session_boot.py @@ -59,6 +59,28 @@ class TestFindTmuxSessionForPid: assert session_boot._find_tmux_session_for_pid(9999) is None +class TestGetPpid: + def test_returns_parent_pid(self): + mock_result = MagicMock(returncode=0, stdout=" 1234\n") + with patch(f"{_MOD}.subprocess.run", return_value=mock_result): + assert session_boot._get_ppid(5678) == 1234 + + def test_returns_none_on_failure(self): + mock_result = MagicMock(returncode=1, stdout="") + with patch(f"{_MOD}.subprocess.run", return_value=mock_result): + assert session_boot._get_ppid(5678) is None + + def test_returns_none_on_oserror(self): + with patch(f"{_MOD}.subprocess.run", side_effect=OSError("no ps")): + assert session_boot._get_ppid(5678) is None + + def test_returns_none_on_timeout(self): + import subprocess + + with patch(f"{_MOD}.subprocess.run", side_effect=subprocess.TimeoutExpired("ps", 5)): + assert session_boot._get_ppid(5678) is None + + class TestIsDescendant: def test_direct_match(self): assert session_boot._is_descendant(100, 100) is True @@ -66,13 +88,22 @@ class TestIsDescendant: def test_pid_one_not_descendant(self): assert session_boot._is_descendant(1, 999) is False - def test_proc_walk(self): - statuses = {200: "Name:\tpython3\nPPid:\t100\n"} - with patch("pathlib.Path.read_text", side_effect=lambda: statuses.get(200, "")): + def test_walks_via_ps(self): + def mock_ppid(pid): + return {200: 150, 150: 100}.get(pid) + + with patch.object(session_boot, "_get_ppid", side_effect=mock_ppid): assert session_boot._is_descendant(200, 100) is True - def test_proc_not_found(self): - with patch("pathlib.Path.read_text", side_effect=OSError("no such file")): + def test_not_descendant(self): + def mock_ppid(pid): + return {200: 150, 150: 1}.get(pid) + + with patch.object(session_boot, "_get_ppid", side_effect=mock_ppid): + assert session_boot._is_descendant(200, 100) is False + + def test_ppid_none_stops_walk(self): + with patch.object(session_boot, "_get_ppid", return_value=None): assert session_boot._is_descendant(200, 100) is False @@ -135,6 +166,8 @@ class TestBoot: result = session_boot.boot(cwd=str(tmp_path)) assert result["exit_code"] == 1 assert result["action"] == "warn" + assert "kill 1234" in result["error"] + assert "command claude --resume" in result["error"] def test_no_live_session_starts_fresh(self, tmp_path): with ( @@ -210,3 +243,54 @@ class TestMain: ): session_boot.main() mock_boot.assert_called_once_with(extra_args=None) + + +class TestPermissionModeDedupe: + def test_no_extra_args_includes_default(self, tmp_path): + with ( + patch.dict("os.environ", {"TMUX": "/tmp/tmux-1000/default,1,0"}), + patch.object(session_boot, "_resolve_claude_binary", return_value="/usr/local/bin/claude"), + patch(f"{_MOD}.os.execvp") as mock_exec, + ): + session_boot.boot(cwd=str(tmp_path)) + cmd = mock_exec.call_args[0][1] + assert cmd.count("--permission-mode") == 1 + assert "bypassPermissions" in cmd + + def test_extra_args_with_permission_mode_no_double(self, tmp_path): + with ( + patch.dict("os.environ", {"TMUX": "/tmp/tmux-1000/default,1,0"}), + patch.object(session_boot, "_resolve_claude_binary", return_value="/usr/local/bin/claude"), + patch(f"{_MOD}.os.execvp") as mock_exec, + ): + session_boot.boot(cwd=str(tmp_path), extra_args=["--permission-mode", "default"]) + cmd = mock_exec.call_args[0][1] + assert cmd.count("--permission-mode") == 1 + assert "default" in cmd + assert "bypassPermissions" not in cmd + + def test_extra_args_without_permission_mode_gets_default(self, tmp_path): + with ( + patch.dict("os.environ", {"TMUX": "/tmp/tmux-1000/default,1,0"}), + patch.object(session_boot, "_resolve_claude_binary", return_value="/usr/local/bin/claude"), + patch(f"{_MOD}.os.execvp") as mock_exec, + ): + session_boot.boot(cwd=str(tmp_path), extra_args=["--resume"]) + cmd = mock_exec.call_args[0][1] + assert cmd.count("--permission-mode") == 1 + assert "bypassPermissions" in cmd + assert "--resume" in cmd + + def test_fresh_start_dedupes_too(self, tmp_path): + with ( + patch.dict("os.environ", {}, clear=True), + patch.object(session_boot, "_resolve_claude_binary", return_value="/usr/local/bin/claude"), + patch.object(session_boot, "_find_tmux", return_value="/usr/bin/tmux"), + patch.object(session_boot, "_find_live_sessions", return_value=[]), + patch.object(session_boot, "_tmux_session_exists", return_value=False), + patch(f"{_MOD}.os.execvp") as mock_exec, + ): + session_boot.boot(cwd=str(tmp_path), extra_args=["--permission-mode", "acceptEdits"]) + cmd = mock_exec.call_args[0][1] + assert cmd.count("--permission-mode") == 1 + assert "acceptEdits" in cmd From d0a31fd862ad924a1129b72a44d0eabe548772b9 Mon Sep 17 00:00:00 2001 From: AIOSAI Date: Thu, 9 Jul 2026 13:16:36 -0700 Subject: [PATCH 06/73] =?UTF-8?q?hooks:=20boot-shim=20installer=20resolves?= =?UTF-8?q?=20venv=20python=20from=20script=20location=20=E2=80=94=20kills?= =?UTF-8?q?=20hardcoded=20/home/patrick=20path=20(POSIX=20+=20Windows=20aw?= =?UTF-8?q?are)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 009c1482..e6835540 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,12 @@ PyPI version — not the changelog header. (`kill && claude`, `command claude --resume`) instead of a vague "kill it first", and the wrapper no longer doubles `--permission-mode` when the user passes it explicitly. New/updated tests, hooks suite 791 green. (built by @hooks) +- **Boot-shim installer no longer bakes a hardcoded user path.** + `install_boot_shim.sh` hardcoded `/home/patrick/Projects/AIPass/.venv/bin/python` + into the `claude()` shell function — wrong on any other machine or user. It now + resolves the venv interpreter from the script's own location (POSIX + `.venv/bin/python`, Windows/git-bash `.venv/Scripts/python.exe`, else PATH + `python3`) and bakes the correct one at install time. ## [2026-07-07] From 5fea5bbf449551fab3f441636a0f567d799b6902 Mon Sep 17 00:00:00 2001 From: AIOSAI Date: Thu, 9 Jul 2026 16:12:44 -0700 Subject: [PATCH 07/73] =?UTF-8?q?seedgo+hooks:=20json=5Fhandler=20empty-gu?= =?UTF-8?q?ard=20(#667)=20+=20fix=20silent=20hook-wiring=20break=20?= =?UTF-8?q?=E2=80=94=20new=20wire=5Fverify=20guard=20fails=20loud=20on=20e?= =?UTF-8?q?mpty/orphaned/dup=20provider=20hook=20events=20(the=20real=20bu?= =?UTF-8?q?g:=20half-wired=20hooks=20written=20silently);=20presence=5Fgat?= =?UTF-8?q?e=20marked=20provider=5Fwired:false=20(dormant=20by=20design,?= =?UTF-8?q?=20not=20a=20break);=20snapshot=20fixture=20corrected=20(drop?= =?UTF-8?q?=20presence=5Fgate,=20add=20SessionStart:cadence=5Freset);=20lo?= =?UTF-8?q?ad=5Fjson=20empty-guard.=20Live=20SessionStart=20orphan=20re-wi?= =?UTF-8?q?red=20separately.=201138=20seedgo=20+=20831=20hooks=20green?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .aipass/hooks.json | 3 +- src/aipass/hooks/.seedgo/bypass.json | 30 ++ src/aipass/hooks/README.md | 6 +- src/aipass/hooks/apps/modules/hookstatus.py | 1 + src/aipass/hooks/apps/modules/wire_verify.py | 233 +++++++++++ src/aipass/hooks/tests/test_wire_verify.py | 380 ++++++++++++++++++ .../seedgo/apps/handlers/json/json_handler.py | 17 +- .../fixtures/provider_hooks_snapshot.json | 19 +- src/aipass/seedgo/tests/test_json_handler.py | 41 ++ 9 files changed, 717 insertions(+), 13 deletions(-) create mode 100644 src/aipass/hooks/apps/modules/wire_verify.py create mode 100644 src/aipass/hooks/tests/test_wire_verify.py diff --git a/.aipass/hooks.json b/.aipass/hooks.json index f6f3fe27..a35c4000 100644 --- a/.aipass/hooks.json +++ b/.aipass/hooks.json @@ -6,7 +6,8 @@ "presence_gate": { "enabled": true, "handler": "aipass.hooks.apps.handlers.security.presence_gate.handle", - "matcher": "" + "matcher": "", + "provider_wired": false }, "identity_injector": { "enabled": true, diff --git a/src/aipass/hooks/.seedgo/bypass.json b/src/aipass/hooks/.seedgo/bypass.json index 4374e276..e246f446 100644 --- a/src/aipass/hooks/.seedgo/bypass.json +++ b/src/aipass/hooks/.seedgo/bypass.json @@ -366,6 +366,11 @@ "standard": "json_structure", "reason": "Read-only config viewer \u2014 delegates JSON loading to config/loader.py, no direct JSON file ops." }, + { + "file": "apps/modules/wire_verify.py", + "standard": "json_structure", + "reason": "Reads ~/.claude/settings.json (external provider settings) with stdlib json \u2014 not branch data storage needing json_handler." + }, { "file": "apps/modules/cadence.py", "standard": "modules", @@ -1103,6 +1108,31 @@ "file": "tests/test_session_start.py", "standard": "meta", "reason": "Test files do not need Version/Modified metadata headers." + }, + { + "file": "tests/test_wire_verify.py", + "standard": "architecture", + "reason": "Test files live in tests/, not in the 3-layer apps structure." + }, + { + "file": "tests/test_wire_verify.py", + "standard": "documentation", + "reason": "Test methods use descriptive names as documentation per pytest convention." + }, + { + "file": "tests/test_wire_verify.py", + "standard": "encapsulation", + "reason": "Tests import modules directly to test implementation details." + }, + { + "file": "tests/test_wire_verify.py", + "standard": "meta", + "reason": "Test files do not need Version/Modified metadata headers." + }, + { + "file": "tests/test_wire_verify.py", + "standard": "help_text", + "reason": "Test fixture _BRIDGE_CMD contains 'python3' as part of a mock provider command string — not user-facing help text." } ], "notes": { diff --git a/src/aipass/hooks/README.md b/src/aipass/hooks/README.md index dc153e09..6a69124a 100644 --- a/src/aipass/hooks/README.md +++ b/src/aipass/hooks/README.md @@ -26,6 +26,7 @@ Every hook event flows through one engine. Platform bridges normalize the event | `drone @hooks hooksound off` | Mute all hook sounds | | `drone @hooks hooksound on` | Unmute all hook sounds | | `drone @hooks cadence` | Show prompt injection cadence config and state | +| `drone @hooks verify` | Cross-check provider settings vs project hook config | | `drone @hooks --help` | Full help reference | | `drone @hooks --version` | Version info | @@ -54,7 +55,8 @@ src/aipass/hooks/ │ │ ├── hooksound.py # Sound control (drone @hooks hooksound on/off) │ │ ├── hookstatus.py # Config viewer (drone @hooks status) │ │ ├── presence.py # Branch presence — claim/release/refresh for .ai_central/PRESENCE.central.json -│ │ └── sandbox.py # Kernel sandbox — srt/bwrap wrapper + per-role policy generator +│ │ ├── sandbox.py # Kernel sandbox — srt/bwrap wrapper + per-role policy generator +│ │ └── wire_verify.py # Wire verification — provider ↔ project hook wiring checker │ ├── handlers/ │ │ ├── bridges/ # One per provider (thin normalization) │ │ │ └── claude.py # Claude Code bridge @@ -86,7 +88,7 @@ src/aipass/hooks/ │ └── diagnostics.py # JSONL logging for hook execution ├── logs/ │ └── engine.jsonl # JSONL diagnostics (every hook execution) -└── tests/ # 705 tests across 25 test files +└── tests/ # 825 tests across 27 test files ``` ## How It Works diff --git a/src/aipass/hooks/apps/modules/hookstatus.py b/src/aipass/hooks/apps/modules/hookstatus.py index 9b084ab9..179770f3 100644 --- a/src/aipass/hooks/apps/modules/hookstatus.py +++ b/src/aipass/hooks/apps/modules/hookstatus.py @@ -27,6 +27,7 @@ EVENT_TYPES = [ "SubagentStop", "Stop", "Notification", + "SessionStart", "PreCompact", ] diff --git a/src/aipass/hooks/apps/modules/wire_verify.py b/src/aipass/hooks/apps/modules/wire_verify.py new file mode 100644 index 00000000..365be802 --- /dev/null +++ b/src/aipass/hooks/apps/modules/wire_verify.py @@ -0,0 +1,233 @@ +# =================== AIPass ==================== +# Name: wire_verify.py +# Version: 1.0.0 +# Description: Wire verification — cross-checks provider settings vs project hook config +# Branch: hooks +# Layer: apps/modules +# Created: 2026-07-09 +# Modified: 2026-07-09 +# ============================================= + +"""Wire verification — catches silent hook-wiring breaks. + +Cross-checks ~/.claude/settings.json (provider hooks) against +.aipass/hooks.json (project hook config). Detects: + - Empty provider hook arrays (event key exists but nothing fires) + - Enabled handlers with no provider bridge entry (handler never dispatched) + - Duplicate provider entries (handler fires multiple times) + - Orphaned provider entries (bridge entry with no project config handler) + +Invoked via: drone @hooks verify +""" + +import json +from pathlib import Path + +from aipass.cli.apps.modules import err_console +from aipass.hooks.apps.handlers.config.loader import find_project_config +from aipass.prax.apps.modules.logger import system_logger as logger + +CONSOLE = err_console + +HELP_COMMANDS = [ + ("verify", "Cross-check provider settings vs project hook config"), +] + +_BRIDGE_MARKER = "bridges/claude.py" +_META_KEYS = frozenset({"_comment", "hooks_enabled"}) + + +def _read_provider_hooks(path=None): + """Read hook events from provider settings. Returns {event: [entries]}.""" + settings_path = Path(path) if path else Path.home() / ".claude" / "settings.json" + try: + raw = settings_path.read_text(encoding="utf-8") + data = json.loads(raw) + return data.get("hooks", {}) + except (OSError, json.JSONDecodeError) as exc: + logger.info("[WIRE_VERIFY] cannot read provider settings: %s", exc) + return {} + + +def _extract_bridge_arg(entry): + """Extract the bridge event arg from a provider entry's command string. + + Returns e.g. 'UserPromptSubmit:tier0_kernel' or 'Stop', or None if not a bridge entry. + """ + for hook in entry.get("hooks", []): + cmd = hook.get("command", "") + if _BRIDGE_MARKER not in cmd: + continue + parts = cmd.split() + for i, part in enumerate(parts): + if part.endswith("claude.py") or _BRIDGE_MARKER in part: + if i + 1 < len(parts): + return parts[i + 1] + return None + + +def _build_provider_index(provider_hooks, errors): + """Parse provider hook entries into a lookup index. + + Returns {event: {"filtered": {hook_name: {matcher: count}}, "unfiltered": int, "empty": bool}}. + Appends to *errors* for empty arrays. + """ + index = {} + for event, entries in provider_hooks.items(): + idx = {"filtered": {}, "unfiltered": 0, "empty": False} + if not entries: + errors.append(f"{event}: provider entry exists but hooks array is EMPTY — nothing fires") + idx["empty"] = True + for entry in entries: + arg = _extract_bridge_arg(entry) + if arg is None: + continue + if ":" in arg: + hook_name = arg.split(":", 1)[1] + matcher = entry.get("matcher", "") + if hook_name not in idx["filtered"]: + idx["filtered"][hook_name] = {} + idx["filtered"][hook_name][matcher] = idx["filtered"][hook_name].get(matcher, 0) + 1 + else: + idx["unfiltered"] += 1 + index[event] = idx + return index + + +def _check_event_wiring(event_type, hooks_group, pidx, errors, warnings, info): + """Check one project config event against its provider index entry.""" + enabled_hooks = { + name: defn for name, defn in hooks_group.items() if isinstance(defn, dict) and defn.get("enabled", False) + } + if not enabled_hooks: + return + + provider_wired_hooks = { + name: defn for name, defn in enabled_hooks.items() if defn.get("provider_wired", True) is not False + } + + if pidx is None: + if provider_wired_hooks: + errors.append( + f"{event_type}: {len(provider_wired_hooks)} enabled handler(s) in project config" + f" but NO provider event entry — handlers never fire" + ) + return + + if pidx["empty"]: + return + + filtered = pidx["filtered"] + if pidx["unfiltered"] > 0: + if pidx["unfiltered"] > 1: + warnings.append(f"{event_type}: {pidx['unfiltered']} duplicate unfiltered provider entries") + info.append(f"{event_type}: unfiltered bridge, {len(enabled_hooks)} enabled hooks OK") + else: + for hook_name, hook_defn in enabled_hooks.items(): + if hook_defn.get("provider_wired", True) is False: + continue + if hook_name not in filtered: + errors.append( + f"{event_type}:{hook_name}: enabled in project config" + " but no provider bridge entry — handler never fires" + ) + continue + for matcher, count in filtered[hook_name].items(): + if count > 1: + warnings.append( + f"{event_type}:{hook_name}: {count} duplicate provider entries (matcher={matcher or 'none'})" + ) + + for hook_name in filtered: + if hook_name not in hooks_group: + warnings.append( + f"{event_type}:{hook_name}: provider entry exists but no handler in project config (orphaned)" + ) + + +def verify_wiring(provider_path=None, project_config=None): + """Cross-check provider settings against project hook config. + + Returns dict with keys: errors (list), warnings (list), info (list), ok (bool). + """ + errors = [] + warnings = [] + info = [] + + provider_hooks = _read_provider_hooks(provider_path) + if not provider_hooks: + errors.append("No provider hooks found in ~/.claude/settings.json") + return {"errors": errors, "warnings": warnings, "info": info, "ok": False} + + config = project_config if project_config is not None else find_project_config() + if config is None: + errors.append("No .aipass/hooks.json found in directory tree") + return {"errors": errors, "warnings": warnings, "info": info, "ok": False} + + provider_index = _build_provider_index(provider_hooks, errors) + + for event_type, hooks_group in config.items(): + if event_type in _META_KEYS or not isinstance(hooks_group, dict): + continue + pidx = provider_index.get(event_type) + _check_event_wiring(event_type, hooks_group, pidx, errors, warnings, info) + + for event in provider_hooks: + if event not in config and not provider_index.get(event, {}).get("empty"): + info.append(f"{event}: provider-only event (no project config section)") + + return { + "errors": errors, + "warnings": warnings, + "info": info, + "ok": len(errors) == 0, + } + + +def _render_results(results): + """Render verification results to console.""" + CONSOLE.print() + + if results["ok"]: + CONSOLE.print("[bold green]✓ Wire check passed[/bold green]") + else: + CONSOLE.print("[bold red]✗ Wire check FAILED[/bold red]") + + CONSOLE.print() + + for error in results["errors"]: + CONSOLE.print(f" [red]ERROR[/red] {error}") + + for warning in results["warnings"]: + CONSOLE.print(f" [yellow]WARN[/yellow] {warning}") + + for item in results["info"]: + CONSOLE.print(f" [dim]OK[/dim] {item}") + + CONSOLE.print() + CONSOLE.print(f"[bold]{len(results['errors'])} errors, {len(results['warnings'])} warnings[/bold]") + + +def print_introspection(): + """Print module structure for drone routing.""" + CONSOLE.print("[bold cyan]wire_verify[/bold cyan] — Provider ↔ project hook wiring checker") + + +def handle_command(command, args) -> bool: + """Route verify commands from drone @hooks.""" + if command != "verify": + return False + + if args and args[0] in ("--help", "-h", "help"): + CONSOLE.print("[bold cyan]wire_verify[/bold cyan] — Provider ↔ project hook wiring checker") + CONSOLE.print() + CONSOLE.print(" drone @hooks verify Cross-check provider settings vs project config") + CONSOLE.print() + CONSOLE.print("Reads ~/.claude/settings.json and .aipass/hooks.json,") + CONSOLE.print("verifies every enabled handler has a working provider bridge entry.") + CONSOLE.print("Exits non-zero on any ERROR finding.") + return True + + results = verify_wiring() + _render_results(results) + return True diff --git a/src/aipass/hooks/tests/test_wire_verify.py b/src/aipass/hooks/tests/test_wire_verify.py new file mode 100644 index 00000000..9d8f8264 --- /dev/null +++ b/src/aipass/hooks/tests/test_wire_verify.py @@ -0,0 +1,380 @@ +"""Tests for wire_verify module — provider ↔ project hook wiring checker.""" + +import json +from unittest.mock import patch + +from aipass.hooks.apps.modules import wire_verify + +_BRIDGE_CMD = "$AIPASS_HOME/.venv/bin/python3 $AIPASS_HOME/src/aipass/hooks/apps/handlers/bridges/claude.py" + + +def _provider_entry(event_arg, timeout=None, matcher=None): + hook = {"type": "command", "command": f"{_BRIDGE_CMD} {event_arg}"} + if timeout: + hook["timeout"] = timeout + entry: dict = {"hooks": [hook]} + if matcher is not None: + entry["matcher"] = matcher + return entry + + +GOOD_PROVIDER = { + "UserPromptSubmit": [ + _provider_entry("UserPromptSubmit:identity_injector"), + _provider_entry("UserPromptSubmit:branch_prompt"), + ], + "Stop": [_provider_entry("Stop")], + "PreToolUse": [_provider_entry("PreToolUse")], +} + +GOOD_PROJECT = { + "hooks_enabled": True, + "UserPromptSubmit": { + "identity_injector": {"enabled": True, "handler": "x.handle", "matcher": ""}, + "branch_prompt": {"enabled": True, "handler": "y.handle", "matcher": ""}, + }, + "Stop": { + "stop_sound": {"enabled": True, "handler": "s.handle", "matcher": ""}, + }, + "PreToolUse": { + "tool_sound": {"enabled": True, "handler": "t.handle", "matcher": "Bash|Edit"}, + }, +} + + +class TestExtractBridgeArg: + def test_filtered_arg(self): + entry = _provider_entry("UserPromptSubmit:tier0_kernel") + assert wire_verify._extract_bridge_arg(entry) == "UserPromptSubmit:tier0_kernel" + + def test_unfiltered_arg(self): + entry = _provider_entry("Stop") + assert wire_verify._extract_bridge_arg(entry) == "Stop" + + def test_no_bridge_marker(self): + entry = {"hooks": [{"command": "echo hello"}]} + assert wire_verify._extract_bridge_arg(entry) is None + + def test_empty_hooks(self): + assert wire_verify._extract_bridge_arg({"hooks": []}) is None + + def test_no_hooks_key(self): + assert wire_verify._extract_bridge_arg({}) is None + + +class TestBuildProviderIndex: + def test_builds_filtered_index(self): + provider = { + "UserPromptSubmit": [ + _provider_entry("UserPromptSubmit:identity_injector"), + _provider_entry("UserPromptSubmit:branch_prompt"), + ], + } + errors = [] + idx = wire_verify._build_provider_index(provider, errors) + assert errors == [] + assert idx["UserPromptSubmit"]["filtered"]["identity_injector"] == {"": 1} + assert idx["UserPromptSubmit"]["filtered"]["branch_prompt"] == {"": 1} + assert idx["UserPromptSubmit"]["unfiltered"] == 0 + + def test_builds_unfiltered_index(self): + provider = {"Stop": [_provider_entry("Stop")]} + errors = [] + idx = wire_verify._build_provider_index(provider, errors) + assert idx["Stop"]["unfiltered"] == 1 + assert idx["Stop"]["filtered"] == {} + + def test_empty_array_errors(self): + provider = {"SessionStart": []} + errors = [] + idx = wire_verify._build_provider_index(provider, errors) + assert len(errors) == 1 + assert "EMPTY" in errors[0] + assert idx["SessionStart"]["empty"] is True + + def test_duplicate_filtered_counted(self): + provider = { + "PreCompact": [ + _provider_entry("PreCompact:pre_compact"), + _provider_entry("PreCompact:pre_compact"), + ], + } + errors = [] + idx = wire_verify._build_provider_index(provider, errors) + assert idx["PreCompact"]["filtered"]["pre_compact"] == {"": 2} + + def test_distinct_matchers_not_duplicate(self): + provider = { + "PreCompact": [ + _provider_entry("PreCompact:pre_compact", matcher="manual"), + _provider_entry("PreCompact:pre_compact", matcher="auto"), + ], + } + errors = [] + idx = wire_verify._build_provider_index(provider, errors) + assert idx["PreCompact"]["filtered"]["pre_compact"] == {"manual": 1, "auto": 1} + + +class TestCheckEventWiring: + def test_unfiltered_ok(self): + pidx = {"filtered": {}, "unfiltered": 1, "empty": False} + hooks_group = {"stop_sound": {"enabled": True, "handler": "x"}} + errors, warnings, info = [], [], [] + wire_verify._check_event_wiring("Stop", hooks_group, pidx, errors, warnings, info) + assert errors == [] + assert any("unfiltered" in i for i in info) + + def test_missing_provider_event(self): + hooks_group = {"cadence_reset": {"enabled": True, "handler": "x"}} + errors, warnings, info = [], [], [] + wire_verify._check_event_wiring("SessionStart", hooks_group, None, errors, warnings, info) + assert len(errors) == 1 + assert "NO provider event entry" in errors[0] + + def test_missing_per_hook_entry(self): + pidx = {"filtered": {"identity_injector": {"": 1}}, "unfiltered": 0, "empty": False} + hooks_group = { + "identity_injector": {"enabled": True, "handler": "x"}, + "presence_gate": {"enabled": True, "handler": "y"}, + } + errors, warnings, info = [], [], [] + wire_verify._check_event_wiring("UserPromptSubmit", hooks_group, pidx, errors, warnings, info) + assert len(errors) == 1 + assert "presence_gate" in errors[0] + assert "never fires" in errors[0] + + def test_duplicate_per_hook_warns(self): + pidx = {"filtered": {"pre_compact": {"": 2}}, "unfiltered": 0, "empty": False} + hooks_group = {"pre_compact": {"enabled": True, "handler": "x"}} + errors, warnings, info = [], [], [] + wire_verify._check_event_wiring("PreCompact", hooks_group, pidx, errors, warnings, info) + assert errors == [] + assert len(warnings) == 1 + assert "duplicate" in warnings[0] + + def test_distinct_matchers_no_warning(self): + pidx = {"filtered": {"pre_compact": {"manual": 1, "auto": 1}}, "unfiltered": 0, "empty": False} + hooks_group = {"pre_compact": {"enabled": True, "handler": "x"}} + errors, warnings, info = [], [], [] + wire_verify._check_event_wiring("PreCompact", hooks_group, pidx, errors, warnings, info) + assert errors == [] + assert warnings == [] + + def test_orphaned_provider_entry(self): + pidx = {"filtered": {"ghost_hook": {"": 1}}, "unfiltered": 0, "empty": False} + hooks_group = {"real_hook": {"enabled": True, "handler": "x"}} + errors, warnings, info = [], [], [] + wire_verify._check_event_wiring("UserPromptSubmit", hooks_group, pidx, errors, warnings, info) + assert any("orphaned" in w for w in warnings) + + def test_disabled_hooks_skipped(self): + pidx = {"filtered": {}, "unfiltered": 0, "empty": False} + hooks_group = {"disabled_hook": {"enabled": False, "handler": "x"}} + errors, warnings, info = [], [], [] + wire_verify._check_event_wiring("UserPromptSubmit", hooks_group, pidx, errors, warnings, info) + assert errors == [] + + def test_empty_provider_skipped(self): + pidx = {"filtered": {}, "unfiltered": 0, "empty": True} + hooks_group = {"cadence_reset": {"enabled": True, "handler": "x"}} + errors, warnings, info = [], [], [] + wire_verify._check_event_wiring("SessionStart", hooks_group, pidx, errors, warnings, info) + assert errors == [] + + def test_duplicate_unfiltered_warns(self): + pidx = {"filtered": {}, "unfiltered": 3, "empty": False} + hooks_group = {"stop_sound": {"enabled": True, "handler": "x"}} + errors, warnings, info = [], [], [] + wire_verify._check_event_wiring("Stop", hooks_group, pidx, errors, warnings, info) + assert len(warnings) == 1 + assert "duplicate unfiltered" in warnings[0] + + def test_provider_wired_false_skips_error(self): + pidx = {"filtered": {"identity_injector": {"": 1}}, "unfiltered": 0, "empty": False} + hooks_group = { + "identity_injector": {"enabled": True, "handler": "x"}, + "presence_gate": {"enabled": True, "handler": "y", "provider_wired": False}, + } + errors, warnings, info = [], [], [] + wire_verify._check_event_wiring("UserPromptSubmit", hooks_group, pidx, errors, warnings, info) + assert errors == [] + + def test_provider_wired_false_no_event_no_error(self): + hooks_group = { + "presence_gate": {"enabled": True, "handler": "y", "provider_wired": False}, + } + errors, warnings, info = [], [], [] + wire_verify._check_event_wiring("UserPromptSubmit", hooks_group, None, errors, warnings, info) + assert errors == [] + + +class TestVerifyWiring: + def test_all_good(self, tmp_path): + settings = tmp_path / "settings.json" + settings.write_text(json.dumps({"hooks": GOOD_PROVIDER})) + result = wire_verify.verify_wiring(provider_path=settings, project_config=GOOD_PROJECT) + assert result["ok"] is True + assert result["errors"] == [] + + def test_empty_provider_array(self, tmp_path): + provider = {**GOOD_PROVIDER, "SessionStart": []} + settings = tmp_path / "settings.json" + settings.write_text(json.dumps({"hooks": provider})) + project = { + **GOOD_PROJECT, + "SessionStart": { + "cadence_reset": {"enabled": True, "handler": "x"}, + }, + } + result = wire_verify.verify_wiring(provider_path=settings, project_config=project) + assert result["ok"] is False + assert any("EMPTY" in e for e in result["errors"]) + + def test_missing_provider_file(self, tmp_path): + result = wire_verify.verify_wiring( + provider_path=tmp_path / "nonexistent.json", + project_config=GOOD_PROJECT, + ) + assert result["ok"] is False + assert any("No provider" in e for e in result["errors"]) + + def test_no_project_config(self, tmp_path): + settings = tmp_path / "settings.json" + settings.write_text(json.dumps({"hooks": GOOD_PROVIDER})) + with patch.object(wire_verify, "find_project_config", return_value=None): + result = wire_verify.verify_wiring(provider_path=settings) + assert result["ok"] is False + assert any("hooks.json" in e for e in result["errors"]) + + def test_missing_per_hook_entry_is_error(self, tmp_path): + provider = { + "UserPromptSubmit": [ + _provider_entry("UserPromptSubmit:identity_injector"), + ], + } + settings = tmp_path / "settings.json" + settings.write_text(json.dumps({"hooks": provider})) + project = { + "hooks_enabled": True, + "UserPromptSubmit": { + "identity_injector": {"enabled": True, "handler": "x"}, + "presence_gate": {"enabled": True, "handler": "y"}, + }, + } + result = wire_verify.verify_wiring(provider_path=settings, project_config=project) + assert result["ok"] is False + assert any("presence_gate" in e for e in result["errors"]) + + def test_provider_wired_false_passes(self, tmp_path): + settings = tmp_path / "settings.json" + settings.write_text(json.dumps({"hooks": GOOD_PROVIDER})) + project = { + **GOOD_PROJECT, + "UserPromptSubmit": { + **GOOD_PROJECT["UserPromptSubmit"], + "presence_gate": {"enabled": True, "handler": "z", "provider_wired": False}, + }, + } + result = wire_verify.verify_wiring(provider_path=settings, project_config=project) + assert result["ok"] is True + assert not any("presence_gate" in e for e in result["errors"]) + + def test_distinct_matchers_no_dupe_warning(self, tmp_path): + provider = { + **GOOD_PROVIDER, + "PreCompact": [ + _provider_entry("PreCompact:pre_compact", matcher="manual"), + _provider_entry("PreCompact:pre_compact", matcher="auto"), + ], + } + settings = tmp_path / "settings.json" + settings.write_text(json.dumps({"hooks": provider})) + project = { + **GOOD_PROJECT, + "PreCompact": { + "pre_compact": {"enabled": True, "handler": "x"}, + }, + } + result = wire_verify.verify_wiring(provider_path=settings, project_config=project) + assert result["ok"] is True + assert not any("duplicate" in w for w in result["warnings"]) + + def test_provider_only_event_info(self, tmp_path): + provider = {**GOOD_PROVIDER, "CustomEvent": [_provider_entry("CustomEvent")]} + settings = tmp_path / "settings.json" + settings.write_text(json.dumps({"hooks": provider})) + result = wire_verify.verify_wiring(provider_path=settings, project_config=GOOD_PROJECT) + assert any("provider-only" in i for i in result["info"]) + + def test_meta_keys_ignored(self, tmp_path): + settings = tmp_path / "settings.json" + settings.write_text(json.dumps({"hooks": GOOD_PROVIDER})) + project = {**GOOD_PROJECT, "_comment": "test", "hooks_enabled": True} + result = wire_verify.verify_wiring(provider_path=settings, project_config=project) + assert result["ok"] is True + + +class TestReadProviderHooks: + def test_reads_file(self, tmp_path): + settings = tmp_path / "settings.json" + settings.write_text(json.dumps({"hooks": {"Stop": []}})) + result = wire_verify._read_provider_hooks(settings) + assert "Stop" in result + + def test_missing_file_returns_empty(self, tmp_path): + result = wire_verify._read_provider_hooks(tmp_path / "missing.json") + assert result == {} + + def test_malformed_json_returns_empty(self, tmp_path): + settings = tmp_path / "settings.json" + settings.write_text("not json{{{") + result = wire_verify._read_provider_hooks(settings) + assert result == {} + + +class TestHandleCommand: + def test_returns_false_for_non_verify(self): + assert wire_verify.handle_command("status", []) is False + + def test_routes_verify(self): + mock_result = {"ok": True, "errors": [], "warnings": [], "info": []} + with patch.object(wire_verify, "verify_wiring", return_value=mock_result): + assert wire_verify.handle_command("verify", []) is True + + def test_help_flag(self): + assert wire_verify.handle_command("verify", ["--help"]) is True + + def test_help_word(self): + assert wire_verify.handle_command("verify", ["help"]) is True + + +class TestRenderResults: + def test_renders_pass(self): + from io import StringIO + + from rich.console import Console + + buf = StringIO() + test_console = Console(file=buf, force_terminal=False) + with patch.object(wire_verify, "CONSOLE", test_console): + wire_verify._render_results({"ok": True, "errors": [], "warnings": [], "info": ["x"]}) + output = buf.getvalue() + assert "passed" in output + + def test_renders_fail(self): + from io import StringIO + + from rich.console import Console + + buf = StringIO() + test_console = Console(file=buf, force_terminal=False) + with patch.object(wire_verify, "CONSOLE", test_console): + wire_verify._render_results({"ok": False, "errors": ["bad"], "warnings": [], "info": []}) + output = buf.getvalue() + assert "FAILED" in output + assert "bad" in output + + +class TestPrintIntrospection: + def test_runs_without_error(self): + wire_verify.print_introspection() diff --git a/src/aipass/seedgo/apps/handlers/json/json_handler.py b/src/aipass/seedgo/apps/handlers/json/json_handler.py index 08646a59..c5ddc7a6 100755 --- a/src/aipass/seedgo/apps/handlers/json/json_handler.py +++ b/src/aipass/seedgo/apps/handlers/json/json_handler.py @@ -125,14 +125,27 @@ 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 an empty/whitespace file — e.g. a concurrent writer caught + mid-truncate in the TOCTOU window between ensure_json_exists() and this + read. Rather than raising JSONDecodeError, fall back to the type's default + template so callers always get a valid structure. A non-empty but malformed + file still raises (fail honestly — that is real corruption, not a race). + """ 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) + content = f.read() + + if not content.strip(): + logger.warning("JSON file empty, using default template: %s", json_path) + return _create_default(json_type, module_name) + + return json.loads(content) def save_json(module_name: str, json_type: str, data: Any) -> bool: diff --git a/src/aipass/seedgo/tests/fixtures/provider_hooks_snapshot.json b/src/aipass/seedgo/tests/fixtures/provider_hooks_snapshot.json index f46db711..8bdc07e3 100644 --- a/src/aipass/seedgo/tests/fixtures/provider_hooks_snapshot.json +++ b/src/aipass/seedgo/tests/fixtures/provider_hooks_snapshot.json @@ -1,13 +1,5 @@ { "UserPromptSubmit": [ - { - "hooks": [ - { - "type": "command", - "command": "$AIPASS_HOME/.venv/bin/python3 $AIPASS_HOME/src/aipass/hooks/apps/handlers/bridges/claude.py UserPromptSubmit:presence_gate" - } - ] - }, { "hooks": [ { @@ -171,5 +163,16 @@ } ] } + ], + "SessionStart": [ + { + "hooks": [ + { + "type": "command", + "command": "$AIPASS_HOME/.venv/bin/python3 $AIPASS_HOME/src/aipass/hooks/apps/handlers/bridges/claude.py SessionStart:cadence_reset", + "timeout": 30 + } + ] + } ] } diff --git a/src/aipass/seedgo/tests/test_json_handler.py b/src/aipass/seedgo/tests/test_json_handler.py index e82a7bd0..43b20558 100644 --- a/src/aipass/seedgo/tests/test_json_handler.py +++ b/src/aipass/seedgo/tests/test_json_handler.py @@ -620,6 +620,47 @@ def test_load_json_empty_file(tmp_path: Path) -> None: assert isinstance(result, dict), "load_json must return dict even for empty file" +def test_load_json_empty_at_read_survives_race(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """#667: empty file at load_json's OWN read. + + The single-threaded case above passes because ensure_json_exists repairs the + empty file first. The real bug is a TOCTOU race: ensure_json_exists reports + OK, then a concurrent writer truncates the file before load_json re-reads it. + Simulate by stubbing ensure_json_exists to pass without repairing. + """ + json_dir = _json_dir_as_path(tmp_path) + json_dir.mkdir(parents=True, exist_ok=True) + # whitespace-only — what a writer caught mid-truncate can leave behind + (json_dir / "raced_config.json").write_text(" \n", encoding="utf-8") + monkeypatch.setattr(json_handler, "ensure_json_exists", lambda *a, **k: True) + result = json_handler.load_json("raced", "config") + assert isinstance(result, dict), "empty-at-read must fall back to default, not crash" + + +def test_load_json_empty_at_read_log_returns_list(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """#667: empty-at-read for a log falls back to the [] default, not a crash.""" + json_dir = _json_dir_as_path(tmp_path) + json_dir.mkdir(parents=True, exist_ok=True) + (json_dir / "raced_log.json").write_text("", encoding="utf-8") + monkeypatch.setattr(json_handler, "ensure_json_exists", lambda *a, **k: True) + result = json_handler.load_json("raced", "log") + assert result == [], "empty-at-read log must fall back to the [] default" + + +def test_load_json_malformed_nonempty_still_raises(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """#667: a non-empty but malformed file still raises (fail honestly). + + The guard only swallows empty/whitespace (a race artifact). Real corruption + must surface, not be masked by a silent default. + """ + json_dir = _json_dir_as_path(tmp_path) + json_dir.mkdir(parents=True, exist_ok=True) + (json_dir / "corrupt_config.json").write_text("{bad json", encoding="utf-8") + monkeypatch.setattr(json_handler, "ensure_json_exists", lambda *a, **k: True) + with pytest.raises(json.JSONDecodeError): + json_handler.load_json("corrupt", "config") + + def test_get_json_path_returns_pathlib_path(tmp_path: Path) -> None: """paths_return_path: get_json_path returns a pathlib.Path instance.""" result = json_handler.get_json_path("pathmod", "config") From cdbd1dc8211b92f8c29671de22e32b047ff252d5 Mon Sep 17 00:00:00 2001 From: AIOSAI Date: Thu, 9 Jul 2026 16:25:35 -0700 Subject: [PATCH 08/73] =?UTF-8?q?setup.sh=20+=20aipass=20doctor:=20recurre?= =?UTF-8?q?nce-prevention=20for=20silent=20hook-wiring=20break=20=E2=80=94?= =?UTF-8?q?=20setup.sh=20merge=20now=20drops=20orphaned=20empty=20hook=20e?= =?UTF-8?q?vents=20(the=20exact=20bug:=20an=20event=20left=20as=20[]=20fir?= =?UTF-8?q?es=20nothing,=20written=20silently)=20and=20announces=20the=20d?= =?UTF-8?q?rop;=20aipass=20doctor=20surfaces=20a=20wire=5Fverify=20check?= =?UTF-8?q?=20under=20Services=20+=20re-verifies=20after=20--fix.=20Proven?= =?UTF-8?q?:=20orphan=20dropped=20/=20valid=20kept=20/=20user=20hooks=20pr?= =?UTF-8?q?eserved;=20doctor=20shows=20green.=20629=20aipass=20green?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- setup.sh | 10 +++- src/aipass/aipass/apps/modules/doctor.py | 8 +++ src/aipass/aipass/apps/modules/doctor_wire.py | 51 +++++++++++++++++- src/aipass/aipass/tests/test_doctor.py | 53 +++++++++++++++++++ 4 files changed, 120 insertions(+), 2 deletions(-) diff --git a/setup.sh b/setup.sh index a654af34..a91445c4 100755 --- a/setup.sh +++ b/setup.sh @@ -712,7 +712,15 @@ for event in set(existing_hooks) | set(aipass_hooks): entry for entry in existing_hooks.get(event, []) if "bridges/claude.py" not in json.dumps(entry) ] - merged_hooks[event] = aipass_hooks.get(event, []) + user_entries + merged = aipass_hooks.get(event, []) + user_entries + # Never emit an empty hook event. If this event had only stale AIPass bridge + # entries (no current aipass_hooks definition AND no user-wired hooks), the + # filter above orphans it to [] — a half-wired event that fires nothing, + # written silently. Drop the key instead and say so, so the state stays honest. + if not merged: + print(f" ! dropped orphaned hook event (no live entries): {event}") + continue + merged_hooks[event] = merged settings["hooks"] = merged_hooks # Inject AIPASS_HOME into env block so dispatched agents find AIPass diff --git a/src/aipass/aipass/apps/modules/doctor.py b/src/aipass/aipass/apps/modules/doctor.py index 6b6d276c..e37da7ed 100644 --- a/src/aipass/aipass/apps/modules/doctor.py +++ b/src/aipass/aipass/apps/modules/doctor.py @@ -60,6 +60,7 @@ from aipass.aipass.apps.modules.doctor_fix import ( ) from aipass.aipass.apps.modules.doctor_wire import ( _auto_wire_provider, + check_wire_verify, prompt_auto_wire, reconcile_stale_deny, ) @@ -468,6 +469,9 @@ def _check_services(verbose: bool = False) -> List[CheckResult]: manifest_checks = _check_provider_manifest() results.extend(manifest_checks) + # wire_verify guard — catch empty/orphaned/duplicate provider hook entries + results.extend(CheckResult(*r) for r in check_wire_verify()) + # stale rm deny rules — detect only (fix runs in run_doctor when --fix) for tup in reconcile_stale_deny(fix=False): results.append(CheckResult(*tup)) @@ -901,6 +905,10 @@ def run_doctor(verbose: bool = False, interactive: bool = False, fix: bool = Fal services = groups.get("Services", []) groups["Services"] = [r for r in services if r.label != "rm deny migration"] + stale_results + wire_recheck = [CheckResult(*r) for r in check_wire_verify()] + services = groups.get("Services", []) + groups["Services"] = [r for r in services if r.label != "wire verify"] + wire_recheck + pass_count = 0 warn_count = 0 error_count = 0 diff --git a/src/aipass/aipass/apps/modules/doctor_wire.py b/src/aipass/aipass/apps/modules/doctor_wire.py index a3bfd319..cf697fae 100644 --- a/src/aipass/aipass/apps/modules/doctor_wire.py +++ b/src/aipass/aipass/apps/modules/doctor_wire.py @@ -20,9 +20,10 @@ from __future__ import annotations import json import shutil +import subprocess from datetime import datetime, timezone from pathlib import Path -from typing import Dict, List +from typing import Dict, List, NamedTuple from aipass.cli.apps.modules import console from aipass.prax import logger @@ -304,3 +305,51 @@ def handle_command(command: str, args: list[str]) -> bool: json_handler.log_operation("doctor_wire_noop", {"command": command}) return False + + +# ============================================================================= +# WIRE VERIFY GUARD (doctor check row) +# ============================================================================= + + +class WireCheckResult(NamedTuple): + """Single doctor check result (mirrors doctor.CheckResult without importing it).""" + + label: str + glyph: str + detail: str + remediation: str + + +_GLYPH_PASS = "[green]✓[/green]" +_GLYPH_FAIL = "[red]✗[/red]" +_GLYPH_WARN = "[yellow]![/yellow]" + + +def check_wire_verify() -> list[WireCheckResult]: + """Run the hooks wire_verify guard — catch empty/orphaned/duplicate provider entries.""" + try: + proc = subprocess.run( + ["drone", "@hooks", "verify"], + capture_output=True, + text=True, + timeout=10, + ) + if proc.returncode == 0: + return [WireCheckResult("wire verify", _GLYPH_PASS, "provider hooks wired correctly", "")] + lines = [ln.strip() for ln in proc.stdout.splitlines() if ln.strip()] + detail = lines[-1] if lines else "errors detected" + return [ + WireCheckResult( + "wire verify", + _GLYPH_FAIL, + detail, + "Run 'aipass doctor --fix' to re-wire, then re-run doctor to confirm", + ) + ] + except FileNotFoundError as exc: + logger.warning("[doctor] drone not found for wire_verify: %s", exc) + return [WireCheckResult("wire verify", _GLYPH_WARN, "drone not found", "")] + except subprocess.TimeoutExpired as exc: + logger.warning("[doctor] wire_verify timed out: %s", exc) + return [WireCheckResult("wire verify", _GLYPH_WARN, "timed out", "")] diff --git a/src/aipass/aipass/tests/test_doctor.py b/src/aipass/aipass/tests/test_doctor.py index 12e142cc..3b9bfeda 100644 --- a/src/aipass/aipass/tests/test_doctor.py +++ b/src/aipass/aipass/tests/test_doctor.py @@ -774,3 +774,56 @@ class TestReconcileStaleDeny: results = reconcile_stale_deny(fix=False) assert len(results) == 1 assert results[0][1] == GLYPH_PASS + + +class TestCheckWireVerify: + """Tests for check_wire_verify() — hooks wire_verify guard.""" + + def test_pass_on_zero_exit(self) -> None: + """Exit 0 from drone @hooks verify produces a PASS row.""" + from aipass.aipass.apps.modules.doctor_wire import check_wire_verify + + fake = MagicMock(returncode=0, stdout="✓ Wire check passed\n\n0 errors, 0 warnings\n") + with patch("aipass.aipass.apps.modules.doctor_wire.subprocess.run", return_value=fake): + results = check_wire_verify() + assert len(results) == 1 + assert results[0].label == "wire verify" + assert results[0].glyph == "[green]✓[/green]" + + def test_fail_on_nonzero_exit(self) -> None: + """Non-zero exit from drone @hooks verify produces a FAIL row.""" + from aipass.aipass.apps.modules.doctor_wire import check_wire_verify + + fake = MagicMock(returncode=1, stdout="ERROR empty array\n2 errors, 0 warnings\n") + with patch("aipass.aipass.apps.modules.doctor_wire.subprocess.run", return_value=fake): + results = check_wire_verify() + assert len(results) == 1 + assert results[0].glyph == "[red]✗[/red]" + assert "errors" in results[0].detail + + def test_warn_on_drone_not_found(self) -> None: + """FileNotFoundError (drone missing) produces a WARN row.""" + from aipass.aipass.apps.modules.doctor_wire import check_wire_verify + + with patch( + "aipass.aipass.apps.modules.doctor_wire.subprocess.run", + side_effect=FileNotFoundError("drone"), + ): + results = check_wire_verify() + assert len(results) == 1 + assert results[0].glyph == "[yellow]![/yellow]" + + def test_warn_on_timeout(self) -> None: + """TimeoutExpired produces a WARN row.""" + import subprocess as sp + + from aipass.aipass.apps.modules.doctor_wire import check_wire_verify + + with patch( + "aipass.aipass.apps.modules.doctor_wire.subprocess.run", + side_effect=sp.TimeoutExpired(cmd="drone", timeout=10), + ): + results = check_wire_verify() + assert len(results) == 1 + assert results[0].glyph == "[yellow]![/yellow]" + assert "timed out" in results[0].detail From f0fc282630269b33d587b379240786d1ad4feece Mon Sep 17 00:00:00 2001 From: AIOSAI Date: Thu, 9 Jul 2026 16:27:25 -0700 Subject: [PATCH 09/73] CHANGELOG: silent hook-wiring break fix + json_handler empty-guard (#667) + wire_verify checker under 2026-07-09 --- CHANGELOG.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e6835540..b144e6a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,33 @@ PyPI version — not the changelog header. resolves the venv interpreter from the script's own location (POSIX `.venv/bin/python`, Windows/git-bash `.venv/Scripts/python.exe`, else PATH `python3`) and bakes the correct one at install time. +- **Silent hook-wiring break: provider settings could be left half-wired with no + warning.** A stale `setup.sh` merge orphaned the `SessionStart` hook event to an + empty `[]` — the key existed but nothing fired — written silently, and it went + unnoticed for weeks because CI skips the provider-settings snapshot test (it + needs `~/.claude/settings.json`, absent in CI). Root cause: the merge stripped + every AIPass bridge entry per event, then re-added only events still present in + its own hook list, orphaning any event it no longer defined. The merge now drops + such an event entirely (and says so) instead of emitting an empty array. Also + corrected the stale snapshot fixture (dropped the dormant `presence_gate`, which + by design ships wired only in project config, and added + `SessionStart:cadence_reset`) and marked `presence_gate` `provider_wired: false` + so the wiring checker knows it is intentionally not provider-wired. +- **`json_handler.load_json` crashed on an empty/whitespace file (#667).** Under + concurrent audit + tests a writer could truncate a JSON file in the window + between `ensure_json_exists` and `load_json`'s own read, raising + `JSONDecodeError`. `load_json` now guards an empty/whitespace read and falls back + to the type's default template; a non-empty but malformed file still raises (fail + honestly). 3 new tests, red-green proven. + +### Added + +- **`drone @hooks verify` — hook-wiring integrity checker.** Cross-checks + `~/.claude/settings.json` against `.aipass/hooks.json` and fails loud on empty + provider hook arrays, orphaned entries, enabled handlers with no provider bridge, + and duplicate (matcher-aware) entries — so a half-wired hook can never rot + silently again. `aipass doctor` now runs this check under Services and re-verifies + after `--fix`. 40+ new tests. (built by @hooks + @aipass) ## [2026-07-07] From 9a06a2fa477e9c1cf3b1cda4dbabdc32ff69a2fc Mon Sep 17 00:00:00 2001 From: AIOSAI Date: Thu, 9 Jul 2026 16:32:06 -0700 Subject: [PATCH 10/73] =?UTF-8?q?hooks:=20wire=5Fverify=20introspection=20?= =?UTF-8?q?gate=20=E2=80=94=20handle=5Fcommand=20no-args=20path=20now=20pr?= =?UTF-8?q?ints=20introspection=20first=20(seedgo=2085%->100%;=20this=20wa?= =?UTF-8?q?s=20the=20CI=20seedgo-audit=20red=20on=20the=20100%=20gate).=20?= =?UTF-8?q?Verify=20behavior=20+=20non-zero-on-error=20exit=20preserved,?= =?UTF-8?q?=20831=20hooks=20green?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/aipass/hooks/apps/modules/wire_verify.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/aipass/hooks/apps/modules/wire_verify.py b/src/aipass/hooks/apps/modules/wire_verify.py index 365be802..2b554d5b 100644 --- a/src/aipass/hooks/apps/modules/wire_verify.py +++ b/src/aipass/hooks/apps/modules/wire_verify.py @@ -218,7 +218,13 @@ def handle_command(command, args) -> bool: if command != "verify": return False - if args and args[0] in ("--help", "-h", "help"): + if not args: + print_introspection() + results = verify_wiring() + _render_results(results) + return True + + if args[0] in ("--help", "-h", "help"): CONSOLE.print("[bold cyan]wire_verify[/bold cyan] — Provider ↔ project hook wiring checker") CONSOLE.print() CONSOLE.print(" drone @hooks verify Cross-check provider settings vs project config") @@ -228,6 +234,4 @@ def handle_command(command, args) -> bool: CONSOLE.print("Exits non-zero on any ERROR finding.") return True - results = verify_wiring() - _render_results(results) - return True + return False From 1edea552bf7bdc1c470686351aae715769bff050 Mon Sep 17 00:00:00 2001 From: AIOSAI Date: Thu, 9 Jul 2026 16:53:14 -0700 Subject: [PATCH 11/73] =?UTF-8?q?backup:=20fix=20Windows-incompat=20test?= =?UTF-8?q?=20=E2=80=94=20test=5Flog=5Foperation=5Fhandles=5Fpath=5Fobject?= =?UTF-8?q?s=20asserted=20a=20hardcoded=20POSIX=20'/some/project';=20defau?= =?UTF-8?q?lt=3Dstr=20serializes=20with=20platform=20separators,=20so=20co?= =?UTF-8?q?mpare=20against=20str(Path(...)).=20Pre-existing=20(S284=20195b?= =?UTF-8?q?9f0),=20the=20sole=20repo-wide=20Windows=20CI=20red?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/aipass/backup/tests/test_json_handler.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/aipass/backup/tests/test_json_handler.py b/src/aipass/backup/tests/test_json_handler.py index 53bd42c0..d982363d 100644 --- a/src/aipass/backup/tests/test_json_handler.py +++ b/src/aipass/backup/tests/test_json_handler.py @@ -144,7 +144,9 @@ class TestLogOperation: log_file = log_dir / "operations.jsonl" if log_file.exists(): entry = json.loads(log_file.read_text(encoding="utf-8").strip()) - assert entry["project_root"] == "/some/project" + # default=str serializes via str(Path(...)) — platform-native separators, + # so compare against the same (POSIX "/some/project", Windows "\some\project"). + assert entry["project_root"] == str(Path("/some/project")) class TestEnsureAndGetPath: From d2d1adca0aad2feebb027edd325154956a8221e3 Mon Sep 17 00:00:00 2001 From: AIOSAI Date: Thu, 9 Jul 2026 19:38:17 -0700 Subject: [PATCH 12/73] #661 exit-code foundation: @cli resolve_exit + error() auto-trip (inert) + @seedgo output_routing checker + devpulse reference adoption MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - @cli: process failure-flag + resolve_exit(handled)->0/1/2; error() auto-trips the flag; inert until a branch adopts it; 10 tests, seedgo 100% - @seedgo: new output_routing standard (39th checker) flags user-facing status output bypassing cli helpers; 254-site per-branch migration checklist; precise (0 FP, dogfooded on devpulse); 1178 tests - devpulse: first adopter — main()->reset_command_state()+resolve_exit(); feedback module+handlers migrated raw-red/logger.error->error(); exit 2/0/1 verified; 380 tests green; seedgo 100% - CHANGELOG updated. Fleet migration (remaining 13 branches) to follow. Tracked in DPLAN-0236. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01HeaAmr3oj2ZexB6ng311Vj --- CHANGELOG.md | 17 + src/aipass/cli/apps/modules/__init__.py | 8 + src/aipass/cli/apps/modules/display.py | 35 ++ src/aipass/cli/tests/test_display.py | 59 +++ .../devpulse/.aipass/aipass_local_prompt.md | 5 + src/aipass/devpulse/apps/devpulse.py | 5 +- .../apps/handlers/feedback/compose.py | 4 +- .../devpulse/apps/handlers/feedback/inbox.py | 6 +- src/aipass/devpulse/apps/modules/feedback.py | 14 +- .../aipass_standards/output_routing.md | 87 ++++ .../aipass_standards/output_routing_check.py | 222 +++++++++ .../output_routing_content.py | 106 +++++ .../seedgo/tests/test_output_routing.py | 424 ++++++++++++++++++ 13 files changed, 978 insertions(+), 14 deletions(-) create mode 100644 src/aipass/seedgo/apps/handlers/aipass_standards/output_routing.md create mode 100644 src/aipass/seedgo/apps/handlers/aipass_standards/output_routing_check.py create mode 100644 src/aipass/seedgo/apps/handlers/aipass_standards/output_routing_content.py create mode 100644 src/aipass/seedgo/tests/test_output_routing.py diff --git a/CHANGELOG.md b/CHANGELOG.md index b144e6a8..c74eee59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,23 @@ PyPI version — not the changelog header. ## [2026-07-09] +### Added + +- **Exit-code contract foundation — failing commands can now exit non-zero + (issue #661, in progress).** CLI error paths printed an error but returned exit + `0`, so `$?`-checking callers (core to running `drone` as a subprocess) were + told success on failure. The dispatch contract was a 2-state bool (`handled` / + `not-mine`) with no way to say "handled *and* failed". `@cli` now exposes a + process-level failure flag + `resolve_exit(handled)` (→ `0`/`1`/`2`), and + `error()` auto-trips the flag — so any failure routed through `error()` gets a + correct non-zero exit with zero per-site edits, and it can't regress. Inert + until a branch's `main()` adopts it. `@seedgo` added an `output_routing` + standard (39th checker) flagging user-facing status output that bypasses the + cli helpers — 254 sites across 14 branches, the migration checklist. `devpulse` + is the first adopter (`main()`→`resolve_exit`, feedback migrated to `error()`, + exit `2`/`0`/`1` verified, 100% seedgo). Fleet migration to follow. + (built by @cli + @seedgo) + ### Fixed - **macOS session lock-out: the boot wrapper can now see tmux sessions on diff --git a/src/aipass/cli/apps/modules/__init__.py b/src/aipass/cli/apps/modules/__init__.py index 566d4749..764cc9fd 100644 --- a/src/aipass/cli/apps/modules/__init__.py +++ b/src/aipass/cli/apps/modules/__init__.py @@ -26,6 +26,9 @@ from aipass.cli.apps.modules.display import console, err_console # Display functions from aipass.cli.apps.modules.display import header, success, error, warning, fatal, section +# Exit-code failure-flag API +from aipass.cli.apps.modules.display import mark_command_failed, command_failed, reset_command_state, resolve_exit + # Operation templates from aipass.cli.apps.modules.templates import operation_start, operation_complete @@ -40,6 +43,11 @@ __all__ = [ "warning", "fatal", "section", + # Exit-code failure-flag API + "mark_command_failed", + "command_failed", + "reset_command_state", + "resolve_exit", # Templates "operation_start", "operation_complete", diff --git a/src/aipass/cli/apps/modules/display.py b/src/aipass/cli/apps/modules/display.py index 333fec27..cc7cde91 100755 --- a/src/aipass/cli/apps/modules/display.py +++ b/src/aipass/cli/apps/modules/display.py @@ -48,6 +48,36 @@ err_console = Console(stderr=True, force_terminal=sys.stderr.isatty()) # Stderr _TRIGGER = None _TRIGGER_LOADED = False +# Process-level command failure flag — mutable container avoids global statement +_CMD_STATE = {"failed": False} + + +def mark_command_failed() -> None: + """Set the process-level failure flag (called automatically by error()).""" + _CMD_STATE["failed"] = True + + +def command_failed() -> bool: + """Return whether mark_command_failed() has been called since last reset.""" + return _CMD_STATE["failed"] + + +def reset_command_state() -> None: + """Reset the failure flag to False (for tests and main() entry).""" + _CMD_STATE["failed"] = False + + +def resolve_exit(handled: bool) -> int: + """Map handled/failed state to an exit code. + + Returns 1 if not handled, 2 if handled but failed, 0 otherwise. + """ + if not handled: + return 1 + if _CMD_STATE["failed"]: + return 2 + return 0 + # ============================================================================ # MODULE PATTERN FUNCTIONS (SEEDGO compliant) @@ -341,6 +371,7 @@ def error(message: str, suggestion: str | None = None) -> None: Example: error('Branch not found', suggestion='Check branch name spelling') """ + mark_command_failed() err_console.print(f"❌ [red bold]{message}[/red bold]") if suggestion: err_console.print(f" [yellow]→ Try: {suggestion}[/yellow]") @@ -411,6 +442,10 @@ __all__ = [ "warning", "fatal", "section", + "mark_command_failed", + "command_failed", + "reset_command_state", + "resolve_exit", ] # ============================================================================ diff --git a/src/aipass/cli/tests/test_display.py b/src/aipass/cli/tests/test_display.py index ebdf1eb7..bd561b05 100644 --- a/src/aipass/cli/tests/test_display.py +++ b/src/aipass/cli/tests/test_display.py @@ -522,3 +522,62 @@ class TestInfrastructureMocking: module_key = "aipass.cli.apps.modules.display" assert module_key in sys.modules assert sys.modules[module_key] is display + + +# ============================================================================= +# Exit-code failure-flag tests +# ============================================================================= + + +class TestCommandState: + """Verify the process-level failure flag and resolve_exit truth table.""" + + def setup_method(self): + display.reset_command_state() + + def teardown_method(self): + display.reset_command_state() + + def test_initial_state_is_not_failed(self): + assert display.command_failed() is False + + def test_mark_command_failed_sets_flag(self): + display.mark_command_failed() + assert display.command_failed() is True + + def test_reset_command_state_clears_flag(self): + display.mark_command_failed() + display.reset_command_state() + assert display.command_failed() is False + + def test_resolve_exit_not_handled(self): + assert display.resolve_exit(handled=False) == 1 + + def test_resolve_exit_handled_ok(self): + assert display.resolve_exit(handled=True) == 0 + + def test_resolve_exit_handled_failed(self): + display.mark_command_failed() + assert display.resolve_exit(handled=True) == 2 + + def test_resolve_exit_not_handled_ignores_flag(self): + display.mark_command_failed() + assert display.resolve_exit(handled=False) == 1 + + def test_error_trips_failure_flag(self): + cons, _ = _make_capture_console() + with patch.object(display, "err_console", cons): + display.error("something broke") + assert display.command_failed() is True + + def test_warning_does_not_trip_flag(self): + cons, _ = _make_capture_console() + with patch.object(display, "err_console", cons): + display.warning("just a warning") + assert display.command_failed() is False + + def test_success_does_not_trip_flag(self): + cons, _ = _make_capture_console() + with patch.object(display, "CONSOLE", cons): + display.success("all good") + assert display.command_failed() is False diff --git a/src/aipass/devpulse/.aipass/aipass_local_prompt.md b/src/aipass/devpulse/.aipass/aipass_local_prompt.md index c35c8e70..49436858 100644 --- a/src/aipass/devpulse/.aipass/aipass_local_prompt.md +++ b/src/aipass/devpulse/.aipass/aipass_local_prompt.md @@ -79,6 +79,11 @@ drone @flow create . "Subject" aplan # APLAN (FPLAN/DPLAN drone @flow list open # active plans ``` +# Dispatch — in-flight comms + + - **Steer a working agent with `email` (no wake), NOT `dispatch`.** `dispatch` = send **+ wake** (hand NEW work to a sleeping agent). An agent already running is awake, so `drone @ai_mail email @target "Subject" "Msg"` reaches it mid-task via its hook — no re-wake, no interrupt. Forgot something / need to correct a brief / add context → **email it in-flight**, don't re-dispatch. (`drone @ai_mail --help`) + - **No backticks in dispatch/email body strings** — bash runs `` `word` `` as command-substitution and silently eats it. Use single quotes or plain text (hit this live: a backtick'd word vanished from a brief). + # Watchdog Devpulse module. After dispatch, arm as a background task — it polls the dispatch lock and exits when the agent finishes. Resolves @target → branch path → `.ai_mail.local/.dispatch.lock`. Default timeout 1800s; `drone @devpulse watchdog --help` for the full reference. diff --git a/src/aipass/devpulse/apps/devpulse.py b/src/aipass/devpulse/apps/devpulse.py index cc62ce17..ae1bd565 100644 --- a/src/aipass/devpulse/apps/devpulse.py +++ b/src/aipass/devpulse/apps/devpulse.py @@ -35,7 +35,7 @@ if sys.platform == "win32": _reconfigure(encoding="utf-8", errors="replace") from aipass.prax import logger -from aipass.cli.apps.modules import err_console +from aipass.cli.apps.modules import err_console, resolve_exit, reset_command_state console = err_console @@ -161,13 +161,14 @@ def _handle_command(command: str, args: list) -> bool: def main(): """Main entry point - routes commands or shows help.""" + reset_command_state() args = sys.argv[1:] if len(args) == 0: print_introspection() return 0 - return 0 if _handle_command(args[0], args[1:]) else 1 + return resolve_exit(_handle_command(args[0], args[1:])) if __name__ == "__main__": diff --git a/src/aipass/devpulse/apps/handlers/feedback/compose.py b/src/aipass/devpulse/apps/handlers/feedback/compose.py index 84b751ef..0b8e4225 100644 --- a/src/aipass/devpulse/apps/handlers/feedback/compose.py +++ b/src/aipass/devpulse/apps/handlers/feedback/compose.py @@ -25,7 +25,7 @@ from aipass.devpulse.apps.handlers.feedback.storage import ( generate_id, ) -from aipass.cli.apps.modules import err_console +from aipass.cli.apps.modules import err_console, error from aipass.devpulse.apps.handlers.json import json_handler console = err_console @@ -133,7 +133,7 @@ def reply_to(msg_id: str, body: str) -> bool: break if msg is None: - console.print(f"[red]Message {msg_id} not found.[/red]") + error(f"Message {msg_id} not found.") return False now = datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%S") diff --git a/src/aipass/devpulse/apps/handlers/feedback/inbox.py b/src/aipass/devpulse/apps/handlers/feedback/inbox.py index ac6cad9d..b7ab8abc 100644 --- a/src/aipass/devpulse/apps/handlers/feedback/inbox.py +++ b/src/aipass/devpulse/apps/handlers/feedback/inbox.py @@ -17,7 +17,7 @@ from rich.table import Table from aipass.devpulse.apps.handlers.feedback.storage import load_inbox, save_inbox -from aipass.cli.apps.modules import err_console +from aipass.cli.apps.modules import err_console, error from aipass.devpulse.apps.handlers.json import json_handler console = err_console @@ -67,7 +67,7 @@ def view_message(msg_id: str) -> None: msg = _find_message(messages, msg_id) if msg is None: - console.print(f"[red]Message {msg_id} not found.[/red]") + error(f"Message {msg_id} not found.") return # Mark as read @@ -105,7 +105,7 @@ def clear_message(msg_id: str) -> None: msg = _find_message(messages, msg_id) if msg is None: - console.print(f"[red]Message {msg_id} not found.[/red]") + error(f"Message {msg_id} not found.") return was_unread = not msg.get("read") diff --git a/src/aipass/devpulse/apps/modules/feedback.py b/src/aipass/devpulse/apps/modules/feedback.py index 6dc7e297..876c3b9f 100644 --- a/src/aipass/devpulse/apps/modules/feedback.py +++ b/src/aipass/devpulse/apps/modules/feedback.py @@ -27,7 +27,7 @@ from aipass.devpulse.apps.handlers.feedback.compose import ( ) from aipass.prax import logger -from aipass.cli.apps.modules import err_console +from aipass.cli.apps.modules import err_console, error from aipass.devpulse.apps.handlers.json import json_handler console = err_console @@ -93,14 +93,14 @@ def handle_command(command: str, args: list[str]) -> bool: if subcommand == "view": if not sub_args: - console.print("[red]Usage: feedback view [/red]") + error("Usage: feedback view ") return True view_message(sub_args[0]) return True if subcommand == "reply": if len(sub_args) < 2: - console.print('[red]Usage: feedback reply "message"[/red]') + error('Usage: feedback reply "message"') return True msg_id = sub_args[0] body = " ".join(sub_args[1:]) @@ -112,7 +112,7 @@ def handle_command(command: str, args: list[str]) -> bool: if subcommand == "clear": if not sub_args: - logger.error("Usage: feedback clear | feedback clear --all") + error("Usage: feedback clear | feedback clear --all") return True if sub_args[0] == "--all": clear_all_read() @@ -120,8 +120,8 @@ def handle_command(command: str, args: list[str]) -> bool: clear_message(sub_args[0]) return True - console.print(f"[red]Unknown feedback subcommand: {subcommand}[/red]") - console.print("Use [bold]feedback --help[/bold] for usage.") + logger.warning("[feedback] unknown subcommand: %s", subcommand) + error(f"Unknown feedback subcommand: {subcommand}", suggestion="Use 'feedback --help' for usage") return True @@ -138,7 +138,7 @@ def _handle_send(args: list[str]) -> bool: bool: Always True (command was handled). """ if len(args) < 2: - console.print('[red]Usage: feedback send "subject" "body"[/red]') + error('Usage: feedback send "subject" "body"') console.print("[dim]Tip: from_branch is auto-detected or pass as first arg.[/dim]") return True diff --git a/src/aipass/seedgo/apps/handlers/aipass_standards/output_routing.md b/src/aipass/seedgo/apps/handlers/aipass_standards/output_routing.md new file mode 100644 index 00000000..6010a17d --- /dev/null +++ b/src/aipass/seedgo/apps/handlers/aipass_standards/output_routing.md @@ -0,0 +1,87 @@ +# Output Routing Standard + +**Status:** Active +**Date:** 2026-07-09 + +--- + +## What This Standard Is + +User-facing error, success, and warning output must route through `@cli`'s semantic helpers (`error()`, `success()`, `warning()`) instead of raw `console.print()` with status markup or emojis. + +## Why It Matters + +1. **Consistent formatting** — all agents display errors, successes, and warnings the same way. +2. **Exit-code correctness** — `error()` carries the GitHub #661 failure-flag fix. Raw `console.print("[red]...")` bypasses it, causing error paths to exit 0. +3. **Stderr routing** — `error()` and `warning()` write to stderr; raw `console.print()` writes to stdout. + +## What the Checker Scans For + +Detects `console.print()` or `err_console.print()` calls containing status indicators: + +- `[red]` or `[bold red]` markup (error-style output) +- Status emojis: `❌ ✅ ✓ ✗ ✘ ⚠ ✔` +- `[green]` paired with check emojis (`✓ ✔ ✅`) +- `[yellow]` paired with warning indicators (`⚠`, "warning", "WARN", "FAIL") + +### Exclusions + +- Lines inside docstrings (triple-quoted regions) +- Comment lines (`# ...`) +- `__init__.py` files +- Test files (`test_*.py`, `*_test.py`, `conftest.py`) +- `console.print()` with no status markup (tables, panels, informational text) +- Non-status color like `[cyan]`, `[dim]`, `[blue]` + +## Code Examples + +### Violation + +```python +console.print(f"[red]Error: {msg}[/red]") +console.print("[bold red]Failed to process[/bold red]") +console.print(f"[green]✓[/green] Task complete") +console.print(f"❌ Something went wrong") +``` + +### Fix + +```python +from aipass.cli.apps.modules import error, success, warning + +error(f"Error: {msg}") +error("Failed to process") +success("Task complete") +error("Something went wrong") +``` + +## Scoring + +- Single check per file: pass (0 violations) or fail (any violations) +- Score: 100 if passed, 0 if failed +- Threshold: score >= 75 to pass overall +- Line-level bypass filtering is supported + +## Bypass + +Add an entry to `.seedgo/bypass.json`: + +```json +{"standard": "output_routing", "file": "path/to/file.py"} +``` + +Or bypass specific lines: + +```json +{"standard": "output_routing", "file": "file.py", "lines": [42, 78]} +``` + +## Audit Scope + +`AUDIT_SCOPE = all_files` — runs against every `.py` file in the branch. Skips `__init__.py` and test files. + +## Reference + +- Checker: `output_routing_check.py` +- Standards pack: seedgo standards (output_routing) +- Related: GitHub #661 (error paths exit 0) diff --git a/src/aipass/seedgo/apps/handlers/aipass_standards/output_routing_check.py b/src/aipass/seedgo/apps/handlers/aipass_standards/output_routing_check.py new file mode 100644 index 00000000..9c2091e9 --- /dev/null +++ b/src/aipass/seedgo/apps/handlers/aipass_standards/output_routing_check.py @@ -0,0 +1,222 @@ +# =================== AIPass ==================== +# Name: output_routing_check.py +# Description: Output Routing Standards Checker Handler +# Version: 1.0.0 +# Created: 2026-07-09 +# Modified: 2026-07-09 +# ============================================= + +""" +Output Routing Standards Checker Handler + +Detects user-facing error/success/warning output that bypasses @cli's +semantic helpers (error(), success(), warning()) by using raw +console.print() with status markup or status emojis. +""" + +import re +import sys +from pathlib import Path +from typing import Dict + +from aipass.prax import logger +from aipass.seedgo.apps.handlers.json import json_handler +from aipass.seedgo.apps.handlers.bypass.utils import is_bypassed + +if sys.stdout and hasattr(sys.stdout, "reconfigure"): + sys.stdout.reconfigure(encoding="utf-8") # type: ignore[attr-defined] +if sys.stderr and hasattr(sys.stderr, "reconfigure"): + sys.stderr.reconfigure(encoding="utf-8") # type: ignore[attr-defined] + +AUDIT_SCOPE = "all_files" + +_TEST_FILE_RE = re.compile(r"^(test_.+|.+_test|conftest)\.py$") + +# console.print( or err_console.print( at any indentation +_CONSOLE_PRINT_RE = re.compile(r"(?:console|err_console)\.print\(") + +# Status color markup — error indicators +_RED_MARKUP_RE = re.compile(r"\[(?:bold\s+)?red(?:\s+bold)?\]") + +# Status color markup — warning indicators +_YELLOW_STATUS_RE = re.compile(r"\[yellow\].*(?:⚠|[Ww]arning|WARN|FAIL)") + +# Status emojis: ❌ ✅ ✓ ✗ ✘ ⚠ ✔ +_STATUS_EMOJI_RE = re.compile(r"[❌✅✓✗✘⚠✔]") + +# Green check pattern: [green] followed by check emoji +_GREEN_CHECK_RE = re.compile(r"\[green\].*[✓✔✅]") + + +def _is_status_console_print(code: str) -> bool: + if not _CONSOLE_PRINT_RE.search(code): + return False + if _RED_MARKUP_RE.search(code): + return True + if _STATUS_EMOJI_RE.search(code): + return True + if _YELLOW_STATUS_RE.search(code): + return True + if _GREEN_CHECK_RE.search(code): + return True + return False + + +def _scan_file(file_path: Path) -> tuple[list[int], str | None]: + try: + source = file_path.read_text(encoding="utf-8", errors="ignore") + except OSError as exc: + logger.info("Cannot read %s: %s", file_path, exc) + return [], f"cannot read: {exc}" + + lines = source.splitlines() + hit_lines: list[int] = [] + in_docstring = False + docstring_char: str | None = None + + for lineno, line in enumerate(lines, start=1): + stripped = line.strip() + + for tq in ('"""', "'''"): + count = line.count(tq) + if count == 0: + continue + if not in_docstring: + in_docstring = True + docstring_char = tq + if count >= 2: + in_docstring = False + docstring_char = None + elif docstring_char == tq: + in_docstring = False + docstring_char = None + + if in_docstring: + continue + + if stripped.startswith("#"): + continue + + code_part = line.split("#")[0] + + if _is_status_console_print(code_part): + hit_lines.append(lineno) + + return hit_lines, None + + +def check_module(module_path: str, bypass_rules: list | None = None) -> Dict: + """Check a Python file for user-facing output bypassing @cli helpers.""" + path = Path(module_path) + + if is_bypassed(module_path, "output_routing", bypass_rules=bypass_rules): + return { + "passed": True, + "checks": [ + { + "name": "Bypassed", + "passed": True, + "message": "Standard bypassed via .seedgo/bypass.json", + } + ], + "score": 100, + "standard": "OUTPUT_ROUTING", + } + + if path.name == "__init__.py": + return { + "passed": True, + "checks": [ + { + "name": "Output routing", + "passed": True, + "message": "__init__.py skipped", + } + ], + "score": 100, + "standard": "OUTPUT_ROUTING", + } + + if _TEST_FILE_RE.match(path.name): + return { + "passed": True, + "checks": [ + { + "name": "Output routing", + "passed": True, + "message": "Test file skipped", + } + ], + "score": 100, + "standard": "OUTPUT_ROUTING", + } + + if not path.exists(): + return { + "passed": False, + "checks": [ + { + "name": "File exists", + "passed": False, + "message": f"File not found: {module_path}", + } + ], + "score": 0, + "standard": "OUTPUT_ROUTING", + } + + hit_lines, error = _scan_file(path) + + if error is not None: + return { + "passed": False, + "checks": [ + { + "name": "File readable", + "passed": False, + "message": f"Error reading file: {error}", + } + ], + "score": 0, + "standard": "OUTPUT_ROUTING", + } + + non_bypassed = [ln for ln in hit_lines if not is_bypassed(module_path, "output_routing", ln, bypass_rules)] + + checks: list[Dict] = [] + + if not non_bypassed: + checks.append( + { + "name": "Output routing", + "passed": True, + "message": "All user-facing status output uses @cli helpers", + } + ) + else: + sample = ", ".join(str(ln) for ln in non_bypassed[:5]) + suffix = f" (and {len(non_bypassed) - 5} more)" if len(non_bypassed) > 5 else "" + checks.append( + { + "name": "Output routing", + "passed": False, + "message": f"{len(non_bypassed)} raw status output(s) on lines {sample}{suffix}", + } + ) + + passed_checks = sum(1 for c in checks if c["passed"]) + total_checks = len(checks) + score = int(passed_checks / total_checks * 100) if total_checks > 0 else 0 + overall_passed = score >= 75 + + json_handler.log_operation( + "check_completed", + {"file": str(module_path), "score": score, "standard": "output_routing"}, + ) + + return { + "passed": overall_passed, + "checks": checks, + "score": score, + "standard": "OUTPUT_ROUTING", + } diff --git a/src/aipass/seedgo/apps/handlers/aipass_standards/output_routing_content.py b/src/aipass/seedgo/apps/handlers/aipass_standards/output_routing_content.py new file mode 100644 index 00000000..0ec57e87 --- /dev/null +++ b/src/aipass/seedgo/apps/handlers/aipass_standards/output_routing_content.py @@ -0,0 +1,106 @@ +# =================== AIPass ==================== +# Name: output_routing_content.py +# Description: Output Routing Standards Content Handler +# Version: 1.0.0 +# Created: 2026-07-09 +# Modified: 2026-07-09 +# ============================================= + +""" +Output Routing Standards Content Handler + +Provides formatted Output Routing standards content. +Module orchestrates, handler implements. +""" + +import sys + +from aipass.seedgo.apps.handlers.json import json_handler + +if sys.stdout and hasattr(sys.stdout, "reconfigure"): + sys.stdout.reconfigure(encoding="utf-8") # type: ignore[attr-defined] +if sys.stderr and hasattr(sys.stderr, "reconfigure"): + sys.stderr.reconfigure(encoding="utf-8") # type: ignore[attr-defined] + + +def get_output_routing_standards() -> str: + """Return formatted output_routing standards content with Rich markup + + Returns: + str: Formatted standards text with Rich styling + """ + lines = [ + "[bold cyan]CORE PRINCIPLE:[/bold cyan]", + " User-facing error, success, and warning output MUST route through", + " @cli's semantic helpers — [dim]error()[/dim], [dim]success()[/dim],", + " [dim]warning()[/dim] — not raw [red]console.print()[/red] with", + " status markup or emojis.", + "", + "[bold cyan]WHY IT MATTERS:[/bold cyan]", + " 1. [yellow]Consistent formatting[/yellow] across all agents", + " 2. [yellow]Exit-code correctness[/yellow] — error() carries the #661", + " failure-flag fix; raw markup bypasses it (errors exit 0)", + " 3. [yellow]Stderr routing[/yellow] — error()/warning() write to stderr;", + " raw console.print() writes to stdout", + "", + "[bold cyan]WHAT IT CHECKS:[/bold cyan]", + " Scans every .py file for [dim]console.print()[/dim] or", + " [dim]err_console.print()[/dim] calls containing status indicators:", + "", + " [red]Flagged patterns:[/red]", + ' - [dim]console.print(f"[red]Error: ...[/red]")[/dim] → use error()', + ' - [dim]console.print("[bold red]Failed[/bold red]")[/dim] → use error()', + ' - [dim]console.print("[green]...[/green]")[/dim] with check emojis → use success()', + ' - [dim]console.print("...")[/dim] with status emojis → use helpers', + "", + " [green]NOT flagged (legitimate Rich usage):[/green]", + " - [dim]console.print(table)[/dim] — Rich Table objects", + " - [dim]console.print(Panel(...))[/dim] — decorative panels", + ' - [dim]console.print(f"[cyan]Info...[/cyan]")[/dim] — non-status color', + ' - [dim]console.print(f"[dim]...[/dim]")[/dim] — decorative formatting', + " - Lines inside docstrings, comments, test files", + "", + "[bold cyan]VIOLATIONS:[/bold cyan]", + "", + " [red]Bad — raw status output:[/red]", + ' [dim]console.print(f"[red]Error: {msg}[/red]")[/dim]', + ' [dim]console.print("[green]...[/green] Done")[/dim]', + ' [dim]console.print("... Failed")[/dim]', + "", + " [green]Good — use @cli helpers:[/green]", + " [dim]from aipass.cli.apps.modules import error, success, warning[/dim]", + ' [dim]error(f"Error: {msg}")[/dim]', + ' [dim]success("Done")[/dim]', + ' [dim]warning("Check configuration")[/dim]', + "", + "[bold cyan]HOW TO FIX:[/bold cyan]", + " 1. Import the helpers: [dim]from aipass.cli.apps.modules import error, success, warning[/dim]", + ' 2. Replace [dim]console.print(f"[red]...")[/dim] with [dim]error(msg)[/dim]', + " 3. Replace status-emoji prints with [dim]success(msg)[/dim] or [dim]warning(msg)[/dim]", + " 4. Keep [dim]console.print()[/dim] for tables, panels, and non-status output", + "", + "[yellow]SCOPE:[/yellow]", + " AUDIT_SCOPE = [bold]all_files[/bold]", + " Runs against every .py file in the branch.", + " Skips __init__.py and test files (test_*.py, *_test.py, conftest.py).", + "", + "[bold cyan]SCORING:[/bold cyan]", + " Single check per file: [green]pass[/green] (0 violations) or [red]fail[/red]", + " Score: 100 if passed, 0 if failed", + " Threshold: score >= 75 to pass overall", + " Line-level bypass filtering is supported.", + "", + "[bold cyan]BYPASS:[/bold cyan]", + " Add an entry to [dim].seedgo/bypass.json[/dim]:", + ' [dim]{"standard": "output_routing", "file": "path/to/file.py"}[/dim]', + " Or bypass specific lines:", + ' [dim]{"standard": "output_routing", "file": "file.py", "lines": [42]}[/dim]', + "", + "[bold cyan]REFERENCE:[/bold cyan]", + " [dim]See: seedgo standards pack (output_routing)[/dim]", + " [dim]Checker: output_routing_check.py[/dim]", + " [dim]Related: GitHub #661 (error paths exit 0)[/dim]", + ] + + json_handler.log_operation("standard_content_queried", {"standard": "output_routing"}) + return "\n".join(lines) diff --git a/src/aipass/seedgo/tests/test_output_routing.py b/src/aipass/seedgo/tests/test_output_routing.py new file mode 100644 index 00000000..8c199c72 --- /dev/null +++ b/src/aipass/seedgo/tests/test_output_routing.py @@ -0,0 +1,424 @@ +"""Tests for output_routing_check.py.""" + +# =================== META ==================== +# Name: test_output_routing.py +# Description: Unit tests for output_routing_check +# Version: 1.0.0 +# Created: 2026-07-09 +# Modified: 2026-07-09 +# ============================================= + +import pytest +from unittest.mock import MagicMock + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture(autouse=True) +def _mock_infrastructure(monkeypatch): + """Mock heavy infrastructure imports for standards checkers.""" + import sys + + mock_logger = MagicMock() + mock_json_handler = MagicMock() + mock_json_handler.log_operation = MagicMock(return_value=True) + + prax_mod = MagicMock() + prax_mod.logger = mock_logger + monkeypatch.setitem(sys.modules, "aipass.prax", prax_mod) + + json_pkg = MagicMock() + json_pkg.json_handler = mock_json_handler + monkeypatch.setitem(sys.modules, "aipass.seedgo.apps.handlers.json", json_pkg) + json_mod = MagicMock() + json_mod.log_operation = mock_json_handler.log_operation + monkeypatch.setitem(sys.modules, "aipass.seedgo.apps.handlers.json.json_handler", json_mod) + + from aipass.seedgo.apps.handlers.bypass.utils import is_bypassed as real_is_bypassed + + bypass_pkg = MagicMock() + bypass_utils = MagicMock() + bypass_utils.is_bypassed = real_is_bypassed + bypass_pkg.utils = bypass_utils + bypass_ignore = MagicMock() + bypass_ignore.get_template_ignore_patterns = MagicMock(return_value=[]) + bypass_pkg.ignore_handler = bypass_ignore + monkeypatch.setitem(sys.modules, "aipass.seedgo.apps.handlers.bypass", bypass_pkg) + monkeypatch.setitem(sys.modules, "aipass.seedgo.apps.handlers.bypass.utils", bypass_utils) + monkeypatch.setitem( + sys.modules, + "aipass.seedgo.apps.handlers.bypass.ignore_handler", + bypass_ignore, + ) + + for mod_name in [ + "aipass.seedgo.apps.handlers.aipass_standards.output_routing_check", + ]: + monkeypatch.delitem(sys.modules, mod_name, raising=False) + + +# =========================================================================== +# 1. _is_status_console_print — detection logic +# =========================================================================== + + +class TestIsStatusConsolePrint: + """Tests for the _is_status_console_print helper.""" + + def test_red_markup_detected(self): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import ( + _is_status_console_print, + ) + + assert _is_status_console_print('console.print(f"[red]Error: {msg}[/red]")') + + def test_bold_red_detected(self): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import ( + _is_status_console_print, + ) + + assert _is_status_console_print('console.print("[bold red]Failed[/bold red]")') + + def test_red_bold_order_detected(self): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import ( + _is_status_console_print, + ) + + assert _is_status_console_print('console.print("[red bold]Error[/red bold]")') + + def test_status_emoji_cross_detected(self): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import ( + _is_status_console_print, + ) + + assert _is_status_console_print('console.print("❌ Something failed")') + + def test_status_emoji_check_detected(self): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import ( + _is_status_console_print, + ) + + assert _is_status_console_print('console.print("✅ Done")') + + def test_status_emoji_checkmark_detected(self): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import ( + _is_status_console_print, + ) + + assert _is_status_console_print('console.print("✓ Complete")') + + def test_status_emoji_cross_mark_detected(self): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import ( + _is_status_console_print, + ) + + assert _is_status_console_print('console.print("✗ Failed")') + + def test_green_check_pattern_detected(self): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import ( + _is_status_console_print, + ) + + assert _is_status_console_print('console.print("[green]✓[/green] Done")') + + def test_yellow_warning_detected(self): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import ( + _is_status_console_print, + ) + + assert _is_status_console_print('console.print("[yellow]⚠ Warning: check config[/yellow]")') + + def test_err_console_detected(self): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import ( + _is_status_console_print, + ) + + assert _is_status_console_print('err_console.print(f"[red]Error[/red]")') + + def test_plain_console_print_not_flagged(self): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import ( + _is_status_console_print, + ) + + assert not _is_status_console_print('console.print("Hello world")') + + def test_cyan_markup_not_flagged(self): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import ( + _is_status_console_print, + ) + + assert not _is_status_console_print('console.print(f"[cyan]Info: {msg}[/cyan]")') + + def test_dim_markup_not_flagged(self): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import ( + _is_status_console_print, + ) + + assert not _is_status_console_print('console.print(f"[dim]{details}[/dim]")') + + def test_table_object_not_flagged(self): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import ( + _is_status_console_print, + ) + + assert not _is_status_console_print("console.print(table)") + + def test_panel_object_not_flagged(self): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import ( + _is_status_console_print, + ) + + assert not _is_status_console_print("console.print(Panel(title))") + + def test_no_console_print_not_flagged(self): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import ( + _is_status_console_print, + ) + + assert not _is_status_console_print('logger.info("[red]error[/red]")') + + def test_green_without_check_emoji_not_flagged(self): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import ( + _is_status_console_print, + ) + + assert not _is_status_console_print('console.print("[green]name[/green]")') + + def test_yellow_without_warning_not_flagged(self): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import ( + _is_status_console_print, + ) + + assert not _is_status_console_print('console.print("[yellow]note[/yellow]")') + + +# =========================================================================== +# 2. _scan_file — file scanning +# =========================================================================== + + +class TestScanFile: + """Tests for the _scan_file helper.""" + + def test_detects_red_markup(self, tmp_path): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import _scan_file + + f = tmp_path / "test.py" + f.write_text('console.print(f"[red]Error: {e}[/red]")\n', encoding="utf-8") + lines, err = _scan_file(f) + assert err is None + assert lines == [1] + + def test_skips_docstrings(self, tmp_path): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import _scan_file + + f = tmp_path / "test.py" + f.write_text( + '"""\nconsole.print("[red]error[/red]")\n"""\npass\n', + encoding="utf-8", + ) + lines, err = _scan_file(f) + assert err is None + assert lines == [] + + def test_skips_comments(self, tmp_path): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import _scan_file + + f = tmp_path / "test.py" + f.write_text('# console.print("[red]error[/red]")\n', encoding="utf-8") + lines, err = _scan_file(f) + assert err is None + assert lines == [] + + def test_skips_inline_comment(self, tmp_path): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import _scan_file + + f = tmp_path / "test.py" + f.write_text('x = 1 # console.print("[red]error[/red]")\n', encoding="utf-8") + lines, err = _scan_file(f) + assert err is None + assert lines == [] + + def test_detects_multiple_lines(self, tmp_path): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import _scan_file + + f = tmp_path / "test.py" + f.write_text( + 'x = 1\nconsole.print("[red]a[/red]")\ny = 2\nconsole.print("✅ done")\n', + encoding="utf-8", + ) + lines, err = _scan_file(f) + assert err is None + assert lines == [2, 4] + + def test_clean_file(self, tmp_path): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import _scan_file + + f = tmp_path / "test.py" + f.write_text('console.print("[cyan]info[/cyan]")\nprint("hello")\n', encoding="utf-8") + lines, err = _scan_file(f) + assert err is None + assert lines == [] + + def test_unreadable_file(self, tmp_path): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import _scan_file + + f = tmp_path / "missing.py" + lines, err = _scan_file(f) + assert err is not None + assert lines == [] + + +# =========================================================================== +# 3. check_module — full checker +# =========================================================================== + + +class TestCheckModule: + """Tests for check_module.""" + + def test_clean_file_passes(self, tmp_path): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import check_module + + f = tmp_path / "clean.py" + f.write_text('from aipass.cli.apps.modules import error\nerror("fail")\n', encoding="utf-8") + result = check_module(str(f)) + assert result["passed"] is True + assert result["score"] == 100 + assert result["standard"] == "OUTPUT_ROUTING" + + def test_violation_detected(self, tmp_path): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import check_module + + f = tmp_path / "bad.py" + f.write_text('console.print(f"[red]Error: {e}[/red]")\n', encoding="utf-8") + result = check_module(str(f)) + assert result["passed"] is False + assert result["score"] == 0 + + def test_init_py_skipped(self, tmp_path): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import check_module + + f = tmp_path / "__init__.py" + f.write_text('console.print("[red]error[/red]")\n', encoding="utf-8") + result = check_module(str(f)) + assert result["passed"] is True + assert result["score"] == 100 + + def test_test_file_skipped(self, tmp_path): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import check_module + + f = tmp_path / "test_something.py" + f.write_text('console.print("[red]error[/red]")\n', encoding="utf-8") + result = check_module(str(f)) + assert result["passed"] is True + assert result["score"] == 100 + + def test_conftest_skipped(self, tmp_path): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import check_module + + f = tmp_path / "conftest.py" + f.write_text('console.print("[red]error[/red]")\n', encoding="utf-8") + result = check_module(str(f)) + assert result["passed"] is True + assert result["score"] == 100 + + def test_bypass_returns_100(self, tmp_path): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import check_module + + f = tmp_path / "bypassed.py" + f.write_text('console.print("[red]error[/red]")\n', encoding="utf-8") + bypass = [{"standard": "output_routing", "file": str(f)}] + result = check_module(str(f), bypass_rules=bypass) + assert result["passed"] is True + assert result["score"] == 100 + + def test_missing_file(self, tmp_path): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import check_module + + result = check_module(str(tmp_path / "no_such.py")) + assert result["passed"] is False + assert result["score"] == 0 + + def test_line_bypass_all_lines_pass(self, tmp_path): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import check_module + + f = tmp_path / "partial.py" + f.write_text( + 'console.print("[red]a[/red]")\nconsole.print("[red]b[/red]")\n', + encoding="utf-8", + ) + bypass = [{"standard": "output_routing", "file": "partial.py", "lines": [1, 2]}] + result = check_module(str(f), bypass_rules=bypass) + assert result["passed"] is True + + def test_violation_message_shows_lines(self, tmp_path): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import check_module + + f = tmp_path / "multi.py" + f.write_text('console.print("[red]a[/red]")\nconsole.print("✅ b")\n', encoding="utf-8") + result = check_module(str(f)) + assert "2 raw" in result["checks"][0]["message"] + assert "1, 2" in result["checks"][0]["message"] + + def test_more_than_five_violations_truncated(self, tmp_path): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import check_module + + f = tmp_path / "many.py" + lines = [f'console.print("[red]err{i}[/red]")\n' for i in range(8)] + f.write_text("".join(lines), encoding="utf-8") + result = check_module(str(f)) + assert "and 3 more" in result["checks"][0]["message"] + + +# =========================================================================== +# 4. False-positive avoidance +# =========================================================================== + + +class TestFalsePositiveAvoidance: + """Verify that legitimate patterns are NOT flagged.""" + + def test_table_print_not_flagged(self, tmp_path): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import check_module + + f = tmp_path / "tables.py" + f.write_text("console.print(table)\nconsole.print(Panel(content))\n", encoding="utf-8") + result = check_module(str(f)) + assert result["passed"] is True + + def test_blue_markup_not_flagged(self, tmp_path): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import check_module + + f = tmp_path / "blue.py" + f.write_text('console.print("[blue]Processing...[/blue]")\n', encoding="utf-8") + result = check_module(str(f)) + assert result["passed"] is True + + def test_empty_console_print_not_flagged(self, tmp_path): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import check_module + + f = tmp_path / "blank.py" + f.write_text('console.print("")\nconsole.print()\n', encoding="utf-8") + result = check_module(str(f)) + assert result["passed"] is True + + def test_docstring_with_markup_not_flagged(self, tmp_path): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import check_module + + f = tmp_path / "docs.py" + content = '"""\nconsole.print("[red]error[/red]")\n"""\ndef foo(): pass\n' + f.write_text(content, encoding="utf-8") + result = check_module(str(f)) + assert result["passed"] is True + + def test_green_text_without_emoji_not_flagged(self, tmp_path): + from aipass.seedgo.apps.handlers.aipass_standards.output_routing_check import check_module + + f = tmp_path / "green.py" + f.write_text('console.print("[green]branch_name[/green]")\n', encoding="utf-8") + result = check_module(str(f)) + assert result["passed"] is True From 26a5f3a2eed14df28217be824cb83d59f12332f0 Mon Sep 17 00:00:00 2001 From: AIOSAI Date: Thu, 9 Jul 2026 20:50:47 -0700 Subject: [PATCH 13/73] =?UTF-8?q?#663=20doctor:=20isatty=20guard=20?= =?UTF-8?q?=E2=80=94=20aipass=20doctor=20no=20longer=20hangs=20on=20non-in?= =?UTF-8?q?teractive/blocking=20stdin.=20prompt=5Fauto=5Fwire=20now=20decl?= =?UTF-8?q?ines=20auto-wire=20when=20stdin=20isn't=20a=20tty=20(was:=20inp?= =?UTF-8?q?ut()=20blocked=20forever=20on=20a=20stdin=20that=20never=20EOFs?= =?UTF-8?q?=20=E2=80=94=20read=20as=20a=20crash=20to=20CI/subprocess=20cal?= =?UTF-8?q?lers=20of=20the=20flagship=20health=20command).=20+3=20regressi?= =?UTF-8?q?on=20tests;=20output=5Frouting=20migrated=20to=20cli=20success(?= =?UTF-8?q?).=20Live-repro=20proven:=20blocking=20non-tty=20stdin=20comple?= =?UTF-8?q?tes=20instead=20of=20hanging.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 10 +++ src/aipass/aipass/apps/modules/doctor_wire.py | 17 +++-- src/aipass/aipass/tests/test_doctor.py | 65 +++++++++++++++++++ 3 files changed, 86 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c74eee59..c6980115 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,16 @@ PyPI version — not the changelog header. ### Fixed +- **`aipass doctor` no longer hangs on non-interactive stdin (issue #663).** The + auto-wire `[y/N]` prompt called `input()` with no tty guard, so a caller with a + blocking-but-idle stdin (a script, CI job, or subprocess whose stdin never + sends EOF) hung `doctor` indefinitely — reading as a crash from the flagship + "check my system" command a new user runs first. `prompt_auto_wire` now guards + the prompt with `sys.stdin.isatty()`: a non-tty stdin declines the auto-wire + (prints the manual-wire warning) instead of blocking. Verified against the + exact repro — a blocking non-tty stdin that never EOFs now completes instead of + hanging until killed. Adds 3 regression tests. + - **macOS session lock-out: the boot wrapper can now see tmux sessions on macOS.** `session_boot` decided whether a live Claude session lived inside tmux by walking the process tree through `/proc//status` — Linux-only. diff --git a/src/aipass/aipass/apps/modules/doctor_wire.py b/src/aipass/aipass/apps/modules/doctor_wire.py index cf697fae..81084267 100644 --- a/src/aipass/aipass/apps/modules/doctor_wire.py +++ b/src/aipass/aipass/apps/modules/doctor_wire.py @@ -21,11 +21,12 @@ from __future__ import annotations import json import shutil import subprocess +import sys from datetime import datetime, timezone from pathlib import Path from typing import Dict, List, NamedTuple -from aipass.cli.apps.modules import console +from aipass.cli.apps.modules import console, success from aipass.prax import logger from aipass.aipass.apps.handlers.json import json_handler @@ -206,16 +207,20 @@ def prompt_auto_wire( console.print(f"\n[bold]{', '.join(parts)} missing[/bold]") console.print("[dim]Review details: .claude/hooks/README.md[/dim]") - try: - answer = input("Auto-wire provider settings? [y/N]: ").strip().lower() - except (EOFError, KeyboardInterrupt) as exc: - logger.info("[doctor] auto-wire prompt interrupted: %s", type(exc).__name__) + if not sys.stdin.isatty(): + logger.info("[doctor] non-interactive stdin — auto-wire prompt skipped, treating as decline") answer = "n" + else: + try: + answer = input("Auto-wire provider settings? [y/N]: ").strip().lower() + except (EOFError, KeyboardInterrupt) as exc: + logger.info("[doctor] auto-wire prompt interrupted: %s", type(exc).__name__) + answer = "n" if answer in ("y", "yes"): actions = _auto_wire_provider(manifest_path, interactive=True) for action in actions: - console.print(f"[green]✓[/green] {action}") + success(action) return bool(actions) _print_manual_wire_warning(missing_hooks, missing_env, missing_deny, missing_ask) diff --git a/src/aipass/aipass/tests/test_doctor.py b/src/aipass/aipass/tests/test_doctor.py index 3b9bfeda..3727ad78 100644 --- a/src/aipass/aipass/tests/test_doctor.py +++ b/src/aipass/aipass/tests/test_doctor.py @@ -827,3 +827,68 @@ class TestCheckWireVerify: assert len(results) == 1 assert results[0].glyph == "[yellow]![/yellow]" assert "timed out" in results[0].detail + + +# ============================================================================= +# prompt_auto_wire — non-interactive stdin guard (issue #663) +# ============================================================================= + + +class TestPromptAutoWireIsatty: + """Guard: non-tty stdin must not block on input() (#663).""" + + @staticmethod + def _args() -> dict: + return { + "manifest_path": MagicMock(), + "missing_hooks": ["some_hook"], + "missing_env": [], + "missing_deny": [], + "missing_ask": [], + } + + def test_non_tty_stdin_skips_prompt_and_declines(self) -> None: + """Non-tty stdin must NOT call input() — it declines and warns instead.""" + from aipass.aipass.apps.modules import doctor_wire + + with ( + patch.object(doctor_wire.sys, "stdin") as mock_stdin, + patch("builtins.input") as mock_input, + patch.object(doctor_wire, "_print_manual_wire_warning") as mock_warn, + ): + mock_stdin.isatty.return_value = False + result = doctor_wire.prompt_auto_wire(**self._args()) + + assert result is False + mock_input.assert_not_called() + mock_warn.assert_called_once() + + def test_tty_stdin_prompts_and_respects_decline(self) -> None: + """Tty stdin still prompts; a 'n' answer declines.""" + from aipass.aipass.apps.modules import doctor_wire + + with ( + patch.object(doctor_wire.sys, "stdin") as mock_stdin, + patch("builtins.input", return_value="n") as mock_input, + patch.object(doctor_wire, "_print_manual_wire_warning"), + ): + mock_stdin.isatty.return_value = True + result = doctor_wire.prompt_auto_wire(**self._args()) + + assert result is False + mock_input.assert_called_once() + + def test_tty_stdin_accepts_and_wires(self) -> None: + """Tty stdin with a 'y' answer runs the wire and returns True.""" + from aipass.aipass.apps.modules import doctor_wire + + with ( + patch.object(doctor_wire.sys, "stdin") as mock_stdin, + patch("builtins.input", return_value="y"), + patch.object(doctor_wire, "_auto_wire_provider", return_value=["wired hook"]) as mock_wire, + ): + mock_stdin.isatty.return_value = True + result = doctor_wire.prompt_auto_wire(**self._args()) + + assert result is True + mock_wire.assert_called_once() From bc0d403da9ab06e33a6d5aa5ed4a5743b217a8bd Mon Sep 17 00:00:00 2001 From: AIOSAI Date: Thu, 9 Jul 2026 21:15:18 -0700 Subject: [PATCH 14/73] =?UTF-8?q?#662=20flow:=20close=20no=20longer=20fals?= =?UTF-8?q?e-reports=20'timed=20out=20after=2030s'=20on=20a=20committed=20?= =?UTF-8?q?close.=20close=5Fplan=5Fimpl=20now=20honors=20spawn=5Fbackgroun?= =?UTF-8?q?d=20=E2=80=94=20single=20close=20fires=20the=20detached=20=5Fsp?= =?UTF-8?q?awn=5Fbackground=5Frunner=20(same=20path=20as=20close=5Fall)=20?= =?UTF-8?q?and=20returns=20right=20after=20archive;=20the=20synchronous=20?= =?UTF-8?q?30s=20'drone=20@memory=20process-plans'=20that=20drone=20was=20?= =?UTF-8?q?killing=20is=20gone.=20Removed=20cross-handler=20imports=20(arc?= =?UTF-8?q?hive/trigger=20injected).=20Fix=20built=20by=20@flow,=20verifie?= =?UTF-8?q?d=20by=20devpulse:=20730=20flow=20tests=20green=20(+2),=20seedg?= =?UTF-8?q?o=2031/31=20x3,=20live=20repro=20close=3D5.1s=20exit=200=20(was?= =?UTF-8?q?=2030s-timeout->false=20exit=201).?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 15 +++ .../flow/apps/handlers/plan/close_helpers.py | 38 +++++++ .../flow/apps/handlers/plan/close_ops.py | 101 ++++++------------ src/aipass/flow/apps/modules/close_plan.py | 15 ++- src/aipass/flow/tests/test_close_ops.py | 94 ++++++++++++---- 5 files changed, 172 insertions(+), 91 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c6980115..b75c9340 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,21 @@ PyPI version — not the changelog header. ### Fixed +- **`drone @flow close` no longer reports a false "timed out after 30s" on a + successful close (issue #662).** A single-plan close committed early (plan + marked closed, file archived) and then ran memory vectorization + *synchronously* — `drone @memory process-plans` — inline. On the cold first + close of a session that crossed drone's 30s executor timeout, so drone killed + the flow subprocess and returned exit `1` **after** the close had fully + committed. An autonomous agent reading that exit code would retry or abandon an + already-closed plan. `close_plan_impl` now honors its long-existing + `spawn_background` flag: single close fires the already-detached + `_spawn_background_runner` (the same path `close_all` uses) and returns + immediately after archive; vectorization runs in the background. Also removed + the handler's cross-handler imports (archive/trigger now injected). Verified + live: a real close returns in ~5s at exit 0 ("Vectorizing in background") vs + the prior 30s-timeout risk. 730 flow tests green (+2 new). + - **`aipass doctor` no longer hangs on non-interactive stdin (issue #663).** The auto-wire `[y/N]` prompt called `input()` with no tty guard, so a caller with a blocking-but-idle stdin (a script, CI job, or subprocess whose stdin never diff --git a/src/aipass/flow/apps/handlers/plan/close_helpers.py b/src/aipass/flow/apps/handlers/plan/close_helpers.py index c84049de..72d3c4e4 100644 --- a/src/aipass/flow/apps/handlers/plan/close_helpers.py +++ b/src/aipass/flow/apps/handlers/plan/close_helpers.py @@ -20,6 +20,7 @@ Usage: _find_unregistered_plan_file, _self_heal_unregistered_plan, _spawn_background_runner, + _cleanup_orphaned_plan, ) """ @@ -253,6 +254,43 @@ def _self_heal_unregistered_plan( return actual_key, registry +def _cleanup_orphaned_plan( + plan_file: Path, + plan_label: str, + plan_info: Dict[str, Any], + registry: Dict[str, Any], + save_registry: Any, + reg_file: Any, + messages: List[Dict[str, Any]], + archive_plan: Any = None, +) -> None: + """Archive an orphaned .md file (registry-closed but file never moved).""" + if archive_plan is None: + logger.warning(f"[{MODULE_NAME}] archive_plan not injected, skipping orphan cleanup for {plan_label}") + messages.append({"type": "warning", "text": " Orphan cleanup skipped — archive_plan not available"}) + return + + messages.append({"type": "dim", "text": f" Cleaning up: moving {plan_file.name} to processed_plans/"}) + try: + if archive_plan(plan_file): + logger.info(f"[{MODULE_NAME}] Cleaned up orphaned file for {plan_label}: {plan_file}") + plan_info["processed"] = True + plan_info["processed_date"] = datetime.now(timezone.utc).isoformat() + plan_info["cleanup_completed"] = True + plan_info["cleanup_date"] = datetime.now(timezone.utc).isoformat() + if reg_file: + save_registry(registry, registry_file=reg_file) + else: + save_registry(registry) + messages.append({"type": "success", "text": " Orphaned file archived successfully"}) + else: + logger.warning(f"[{MODULE_NAME}] Failed to archive orphaned file for {plan_label}: {plan_file}") + messages.append({"type": "error_text", "text": " Failed to move orphaned file — manual cleanup required"}) + except Exception as e: + logger.warning(f"[{MODULE_NAME}] Error cleaning orphaned file for {plan_label}: {e}") + messages.append({"type": "error_text", "text": f" Error during cleanup: {e}"}) + + def _spawn_background_runner(): """Spawn post_close_runner.py as a fully detached background process""" bg_runner = FLOW_ROOT / "apps" / "modules" / "post_close_runner.py" diff --git a/src/aipass/flow/apps/handlers/plan/close_ops.py b/src/aipass/flow/apps/handlers/plan/close_ops.py index 661c83e2..06c0f6e4 100644 --- a/src/aipass/flow/apps/handlers/plan/close_ops.py +++ b/src/aipass/flow/apps/handlers/plan/close_ops.py @@ -19,7 +19,6 @@ Usage: """ import sys -import subprocess from pathlib import Path from datetime import datetime, timezone from typing import Dict, Any, List @@ -28,6 +27,7 @@ from aipass.prax import logger from aipass.flow.apps.handlers.json import json_handler from aipass.flow.apps.handlers.plan.close_helpers import ( + PROCESSED_PLANS_DIR, _extract_prefix, _resolve_registry_file, _find_plan_across_registries, @@ -35,6 +35,7 @@ from aipass.flow.apps.handlers.plan.close_helpers import ( _find_unregistered_plan_file, _self_heal_unregistered_plan, _spawn_background_runner, + _cleanup_orphaned_plan, ) MODULE_NAME = "close_plan" @@ -62,6 +63,8 @@ def close_plan_impl( push_to_plans_central: Any = None, push_flow_to_branch_dashboard: Any = None, close_all_plans_fn: Any = None, + archive_plan_fn: Any = None, + trigger_fire_fn: Any = None, ) -> Dict[str, Any]: """ Implement plan closure workflow @@ -187,30 +190,16 @@ def close_plan_impl( "text": f"{plan_label} already closed on {closed_date} — orphaned .md file detected", } ) - messages.append({"type": "dim", "text": f" Cleaning up: moving {plan_file.name} to processed_plans/"}) - try: - from aipass.flow.apps.handlers.mbank.process import archive_plan - - if archive_plan(plan_file): - logger.info(f"[{MODULE_NAME}] Cleaned up orphaned file for {plan_label}: {plan_file}") - # Update registry flags that were missed on the failed first close - plan_info["processed"] = True - plan_info["processed_date"] = datetime.now(timezone.utc).isoformat() - plan_info["cleanup_completed"] = True - plan_info["cleanup_date"] = datetime.now(timezone.utc).isoformat() - if reg_file: - save_registry(registry, registry_file=reg_file) - else: - save_registry(registry) - messages.append({"type": "success", "text": " Orphaned file archived successfully"}) - else: - logger.warning(f"[{MODULE_NAME}] Failed to archive orphaned file for {plan_label}: {plan_file}") - messages.append( - {"type": "error_text", "text": " Failed to move orphaned file — manual cleanup required"} - ) - except Exception as e: - logger.warning(f"[{MODULE_NAME}] Error cleaning orphaned file for {plan_label}: {e}") - messages.append({"type": "error_text", "text": f" Error during cleanup: {e}"}) + _cleanup_orphaned_plan( + plan_file, + plan_label, + plan_info, + registry, + save_registry, + reg_file, + messages, + archive_plan=archive_plan_fn, + ) return { "success": True, "messages": messages, @@ -320,15 +309,12 @@ def close_plan_impl( # --- Step 3/5: Archive plan to processed_plans --- messages.append({"type": "step", "text": "[3/5] Archiving plan..."}) try: - from aipass.flow.apps.handlers.mbank.process import archive_plan, PROCESSED_PLANS_DIR - - # If file is already in processed_plans (found via relocation search), skip move if plan_file.exists() and plan_file.parent == PROCESSED_PLANS_DIR: archive_success = True logger.info(f"[{MODULE_NAME}] {plan_label} already in processed_plans/, skipping move") messages.append({"type": "dim", "text": " Already in processed_plans/ — skipping move"}) else: - archive_success = archive_plan(plan_file) + archive_success = archive_plan_fn(plan_file) if archive_plan_fn else False if archive_success: plan_info["processed"] = True @@ -349,35 +335,17 @@ def close_plan_impl( logger.error(f"[{MODULE_NAME}] Archive error for {plan_label}: {e}") messages.append({"type": "warning", "text": f" Archive error: {e}"}) - # --- Vector intake + verification --- - # Trigger memory's plan processor via drone (no cross-branch imports) - try: - subprocess.run( - ["drone", "@memory", "process-plans"], - capture_output=True, - timeout=30, - ) - except Exception as e: - logger.warning(f"[{MODULE_NAME}] Best-effort drone @memory process-plans failed: {e}") - - # Verify vectorization via memory's verify module - try: - from aipass.memory.apps.modules.verify import is_plan_vectorized # type: ignore[import-not-found] - - result = is_plan_vectorized(plan_label) - if result.get("found"): - chunk_count = result.get("count", 0) - logger.info(f"[{MODULE_NAME}] Vectorized: {plan_label} ({chunk_count} chunks)") - messages.append({"type": "dim", "text": f" Vectorized: {chunk_count} chunks in chroma"}) - else: - logger.warning(f"[{MODULE_NAME}] NOT vectorized: {plan_label}") - messages.append({"type": "warning", "text": " NOT vectorized — check drone @memory process-plans"}) - except ImportError: - logger.warning(f"[{MODULE_NAME}] Vector verify unavailable — memory verify module not found") - messages.append({"type": "warning", "text": " Vector status: unknown (memory verify not available)"}) - except Exception as vec_err: - logger.warning(f"[{MODULE_NAME}] Vector verify failed: {vec_err}") - messages.append({"type": "warning", "text": f" Vector status: unknown ({vec_err})"}) + # --- Vector intake (background) --- + if spawn_background: + try: + _spawn_background_runner() + logger.info(f"[{MODULE_NAME}] Spawned background vectorization for {plan_label}") + messages.append({"type": "dim", "text": " Vectorizing in background"}) + except Exception as e: + logger.warning(f"[{MODULE_NAME}] Background vectorization failed to start: {e}") + messages.append( + {"type": "warning", "text": " Background vectorization failed to start — will retry on next close"} + ) # --- Step 4/5: Update dashboards --- messages.append({"type": "step", "text": "[4/5] Updating dashboards..."}) @@ -416,23 +384,18 @@ def close_plan_impl( logger.warning(f"[{MODULE_NAME}] CLOSED_PLANS update failed (non-critical): {e}") # Fire trigger event for plan closure - try: - from aipass.trigger.apps.modules.core import trigger - - trigger.fire("plan_closed", plan_number=plan_key, location=str(plan_file.parent)) - except ImportError: - logger.info(f"[{MODULE_NAME}] Trigger module not available, skipping event fire") - except Exception as e: - logger.warning(f"[{MODULE_NAME}] Trigger fire failed (non-critical): {e}") + if trigger_fire_fn is not None: + try: + trigger_fire_fn("plan_closed", plan_number=plan_key, location=str(plan_file.parent)) + except Exception as e: + logger.warning(f"[{MODULE_NAME}] Trigger fire failed (non-critical): {e}") # --- VERIFY: Physical state check for self-healed plans --- if plan_info.get("self_healed"): messages.append({"type": "step", "text": "[VERIFY] Checking physical state..."}) try: - from aipass.flow.apps.handlers.mbank.process import PROCESSED_PLANS_DIR as _VERIFY_DIR - original_source = Path(plan_info.get("file_path", "")) - dest = _VERIFY_DIR / original_source.name + dest = PROCESSED_PLANS_DIR / original_source.name if dest.exists(): messages.append({"type": "dim", "text": f" [OK] File in processed_plans/: {original_source.name}"}) else: diff --git a/src/aipass/flow/apps/modules/close_plan.py b/src/aipass/flow/apps/modules/close_plan.py index e7b4a1fc..4626576e 100644 --- a/src/aipass/flow/apps/modules/close_plan.py +++ b/src/aipass/flow/apps/modules/close_plan.py @@ -67,12 +67,21 @@ from aipass.flow.apps.handlers.dashboard.update_local import update_dashboard_lo from aipass.flow.apps.handlers.dashboard.push_central import push_to_plans_central from aipass.flow.apps.handlers.dashboard.push_branch_dashboard import push_flow_to_branch_dashboard -# Internal: Memory template check (lightweight, no API calls) -from aipass.flow.apps.handlers.mbank.process import is_template_content +# Internal: Memory template check + archive (lightweight, no API calls) +from aipass.flow.apps.handlers.mbank.process import is_template_content, archive_plan # Internal: Close operations handler (implementation) from aipass.flow.apps.handlers.plan.close_ops import close_plan_impl, close_all_plans_impl +# Internal: Trigger (optional — may not be installed) +try: + from aipass.trigger.apps.modules.core import trigger as _trigger_module + + _trigger_fire = _trigger_module.fire +except ImportError: + logger.info("[close_plan] Trigger module not available, plan events will be skipped") + _trigger_fire = None + # ============================================= # CONFIGURATION # ============================================= @@ -264,6 +273,8 @@ def close_plan( push_to_plans_central=push_to_plans_central, push_flow_to_branch_dashboard=push_flow_to_branch_dashboard, close_all_plans_fn=close_all_plans, + archive_plan_fn=archive_plan, + trigger_fire_fn=_trigger_fire, ) # Handle dict result from handler diff --git a/src/aipass/flow/tests/test_close_ops.py b/src/aipass/flow/tests/test_close_ops.py index 2201ae3f..c1f41016 100644 --- a/src/aipass/flow/tests/test_close_ops.py +++ b/src/aipass/flow/tests/test_close_ops.py @@ -49,6 +49,8 @@ def _make_deps(**overrides) -> dict: "push_to_plans_central": MagicMock(return_value=True), "push_flow_to_branch_dashboard": MagicMock(return_value=True), "close_all_plans_fn": MagicMock(), + "archive_plan_fn": MagicMock(return_value=True), + "trigger_fire_fn": MagicMock(), } deps.update(overrides) return deps @@ -188,8 +190,7 @@ class TestClosePlanImplAlreadyClosedOrphan: @patch("aipass.flow.apps.handlers.plan.close_ops._resolve_registry_file", return_value=None) @patch("aipass.flow.apps.handlers.plan.close_ops._find_plan_across_registries", return_value=None) - @patch("aipass.flow.apps.handlers.plan.close_ops.archive_plan", create=True) - def test_already_closed_orphan_cleanup(self, mock_archive, _mock_find, _mock_resolve, tmp_path): + def test_already_closed_orphan_cleanup(self, _mock_find, _mock_resolve, tmp_path): close_plan_impl = _import_close_plan_impl() # Create orphan file on disk @@ -209,9 +210,7 @@ class TestClosePlanImplAlreadyClosedOrphan: deps["load_registry"].return_value = registry deps["validate_plan_exists"].return_value = (True, None) - # Patch archive_plan inside the function (lazy import) - with patch("aipass.flow.apps.handlers.mbank.process.archive_plan", return_value=True): - result = close_plan_impl(plan_num="2", **deps) + result = close_plan_impl(plan_num="2", **deps) assert result["success"] is True assert result["plan_key"] == "2" @@ -257,7 +256,7 @@ class TestClosePlanImplSuccess: @patch("aipass.flow.apps.handlers.plan.close_ops._resolve_registry_file", return_value=None) @patch("aipass.flow.apps.handlers.plan.close_ops._find_plan_across_registries", return_value=None) - @patch("aipass.flow.apps.handlers.plan.close_ops.subprocess") + @patch("aipass.flow.apps.handlers.plan.close_helpers.subprocess") def test_successful_close(self, mock_subprocess, _mock_find, _mock_resolve, tmp_path): close_plan_impl = _import_close_plan_impl() @@ -279,7 +278,6 @@ class TestClosePlanImplSuccess: deps["validate_plan_exists"].return_value = (True, None) with ( - patch("aipass.flow.apps.handlers.mbank.process.archive_plan", return_value=True), patch("aipass.flow.apps.handlers.plan.close_ops.json_handler"), patch("aipass.flow.apps.handlers.plan.append_closed_plan.append_to_closed_plans", create=True), ): @@ -295,6 +293,67 @@ class TestClosePlanImplSuccess: deps["push_to_plans_central"].assert_called_once() +class TestSpawnBackgroundBehavior: + """#662: spawn_background controls whether vectorization runs inline or in background.""" + + @patch("aipass.flow.apps.handlers.plan.close_ops._spawn_background_runner") + @patch("aipass.flow.apps.handlers.plan.close_ops._resolve_registry_file", return_value=None) + @patch("aipass.flow.apps.handlers.plan.close_ops._find_plan_across_registries", return_value=None) + def test_spawn_background_true_calls_background_runner(self, _mock_find, _mock_resolve, mock_runner, tmp_path): + close_plan_impl = _import_close_plan_impl() + plan_file = tmp_path / "FPLAN-0001_test_2026-03-20.md" + plan_file.write_text("# Real content\nNotes here.", encoding="utf-8") + registry = { + "plans": { + "1": { + "status": "open", + "subject": "Test plan", + "location": str(tmp_path), + "file_path": str(plan_file), + } + } + } + deps = _make_deps() + deps["load_registry"].return_value = registry + deps["validate_plan_exists"].return_value = (True, None) + with ( + patch("aipass.flow.apps.handlers.plan.close_ops.json_handler"), + patch("aipass.flow.apps.handlers.plan.append_closed_plan.append_to_closed_plans", create=True), + ): + result = close_plan_impl(plan_num="1", spawn_background=True, **deps) + assert result["success"] is True + mock_runner.assert_called_once() + assert any("background" in m.get("text", "").lower() for m in result["messages"]) + + @patch("aipass.flow.apps.handlers.plan.close_ops._spawn_background_runner") + @patch("aipass.flow.apps.handlers.plan.close_ops._resolve_registry_file", return_value=None) + @patch("aipass.flow.apps.handlers.plan.close_ops._find_plan_across_registries", return_value=None) + def test_spawn_background_false_skips_background_runner(self, _mock_find, _mock_resolve, mock_runner, tmp_path): + close_plan_impl = _import_close_plan_impl() + plan_file = tmp_path / "FPLAN-0001_test_2026-03-20.md" + plan_file.write_text("# Real content\nNotes here.", encoding="utf-8") + registry = { + "plans": { + "1": { + "status": "open", + "subject": "Test plan", + "location": str(tmp_path), + "file_path": str(plan_file), + } + } + } + deps = _make_deps() + deps["load_registry"].return_value = registry + deps["validate_plan_exists"].return_value = (True, None) + with ( + patch("aipass.flow.apps.handlers.plan.close_ops.json_handler"), + patch("aipass.flow.apps.handlers.plan.append_closed_plan.append_to_closed_plans", create=True), + ): + result = close_plan_impl(plan_num="1", spawn_background=False, **deps) + assert result["success"] is True + mock_runner.assert_not_called() + + class TestClosePlanImplConfirmCancelled: """User cancels when confirm=True.""" @@ -688,7 +747,7 @@ class TestSelfHealCrossPrefixCollision: class TestClosePlanImplSelfHeal: @patch("aipass.flow.apps.handlers.plan.close_ops._resolve_registry_file", return_value="dplan_registry.json") - @patch("aipass.flow.apps.handlers.plan.close_ops.subprocess") + @patch("aipass.flow.apps.handlers.plan.close_helpers.subprocess") def test_triggers_self_heal_when_not_in_registry(self, mock_subprocess, _mock_resolve, tmp_path): close_plan_impl = _import_close_plan_impl() @@ -723,7 +782,6 @@ class TestClosePlanImplSelfHeal: }, ), ) as mock_heal, - patch("aipass.flow.apps.handlers.mbank.process.archive_plan", return_value=True), patch("aipass.flow.apps.handlers.plan.close_ops.json_handler"), patch("aipass.flow.apps.handlers.plan.append_closed_plan.append_to_closed_plans", create=True), ): @@ -752,7 +810,7 @@ class TestClosePlanImplSelfHeal: class TestSelfHealVerifyBlock: @patch("aipass.flow.apps.handlers.plan.close_ops._resolve_registry_file", return_value="fplan_registry.json") - @patch("aipass.flow.apps.handlers.plan.close_ops.subprocess") + @patch("aipass.flow.apps.handlers.plan.close_helpers.subprocess") def test_verify_all_pass(self, mock_subprocess, _mock_resolve, tmp_path): close_plan_impl = _import_close_plan_impl() @@ -781,9 +839,8 @@ class TestSelfHealVerifyBlock: deps["validate_plan_exists"].return_value = (True, None) with ( - patch("aipass.flow.apps.handlers.mbank.process.archive_plan", return_value=True), patch( - "aipass.flow.apps.handlers.mbank.process.PROCESSED_PLANS_DIR", + "aipass.flow.apps.handlers.plan.close_ops.PROCESSED_PLANS_DIR", processed_dir, ), patch("aipass.flow.apps.handlers.plan.close_ops.json_handler"), @@ -803,7 +860,7 @@ class TestSelfHealVerifyBlock: assert len(ok_msgs) >= 2 @patch("aipass.flow.apps.handlers.plan.close_ops._resolve_registry_file", return_value="fplan_registry.json") - @patch("aipass.flow.apps.handlers.plan.close_ops.subprocess") + @patch("aipass.flow.apps.handlers.plan.close_helpers.subprocess") def test_verify_fails_when_file_not_in_processed(self, mock_subprocess, _mock_resolve, tmp_path): close_plan_impl = _import_close_plan_impl() @@ -830,9 +887,8 @@ class TestSelfHealVerifyBlock: deps["validate_plan_exists"].return_value = (True, None) with ( - patch("aipass.flow.apps.handlers.mbank.process.archive_plan", return_value=True), patch( - "aipass.flow.apps.handlers.mbank.process.PROCESSED_PLANS_DIR", + "aipass.flow.apps.handlers.plan.close_ops.PROCESSED_PLANS_DIR", processed_dir, ), patch("aipass.flow.apps.handlers.plan.close_ops.json_handler"), @@ -846,7 +902,7 @@ class TestSelfHealVerifyBlock: assert any("NOT found in processed_plans" in m.get("text", "") for m in fail_msgs) @patch("aipass.flow.apps.handlers.plan.close_ops._resolve_registry_file", return_value="fplan_registry.json") - @patch("aipass.flow.apps.handlers.plan.close_ops.subprocess") + @patch("aipass.flow.apps.handlers.plan.close_helpers.subprocess") def test_verify_fails_when_source_still_exists(self, mock_subprocess, _mock_resolve, tmp_path): close_plan_impl = _import_close_plan_impl() @@ -874,9 +930,8 @@ class TestSelfHealVerifyBlock: deps["validate_plan_exists"].return_value = (True, None) with ( - patch("aipass.flow.apps.handlers.mbank.process.archive_plan", return_value=True), patch( - "aipass.flow.apps.handlers.mbank.process.PROCESSED_PLANS_DIR", + "aipass.flow.apps.handlers.plan.close_ops.PROCESSED_PLANS_DIR", processed_dir, ), patch("aipass.flow.apps.handlers.plan.close_ops.json_handler"), @@ -913,8 +968,7 @@ class TestSelfHealVerifyBlock: with ( patch("aipass.flow.apps.handlers.plan.close_ops._resolve_registry_file", return_value=None), patch("aipass.flow.apps.handlers.plan.close_ops._find_plan_across_registries", return_value=None), - patch("aipass.flow.apps.handlers.plan.close_ops.subprocess"), - patch("aipass.flow.apps.handlers.mbank.process.archive_plan", return_value=True), + patch("aipass.flow.apps.handlers.plan.close_helpers.subprocess"), patch("aipass.flow.apps.handlers.plan.close_ops.json_handler"), patch("aipass.flow.apps.handlers.plan.append_closed_plan.append_to_closed_plans", create=True), ): From f5554158f94c8d97db40990848ea6e78278cce24 Mon Sep 17 00:00:00 2001 From: AIOSAI Date: Thu, 9 Jul 2026 21:34:17 -0700 Subject: [PATCH 15/73] #660 install: aipass install no longer silently repoints global drone/aipass symlinks. setup.sh safe_symlink guard refuses to hijack a symlink pointing at a DIFFERENT install (loud from->to warning, left untouched) unless --force-symlink; --no-symlink opts out entirely. Both flags thread through install.py -> _run_setup. Fresh/same-location installs unchanged. +tests/setup_symlink_guard_test.sh (10 asserts) +3 flag-forwarding tests; touched install output migrated to cli success() (#661). Verified: 635 aipass tests, seedgo 31/31, live dry-run forwarding + guard test all-pass. --- CHANGELOG.md | 13 ++++ setup.sh | 78 ++++++++++++++++++---- src/aipass/aipass/apps/modules/install.py | 40 +++++++---- src/aipass/aipass/tests/test_install.py | 26 ++++++++ tests/setup_symlink_guard_test.sh | 81 +++++++++++++++++++++++ 5 files changed, 213 insertions(+), 25 deletions(-) create mode 100755 tests/setup_symlink_guard_test.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index b75c9340..cdd58219 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,19 @@ PyPI version — not the changelog header. ### Fixed +- **`aipass install` no longer silently repoints your global `drone`/`aipass` + symlinks (issue #660).** `setup.sh` force-overwrote the global CLI symlinks with + `ln -sf` on every run, no check and no opt-out — so `aipass install + --path /tmp/scratch` "to try it" silently hijacked your real global commands to + the scratch tree, which broke them once `/tmp` cleared, disconnected from the + cause. A new `safe_symlink` guard refuses to repoint a symlink that points at a + *different* install: it prints a loud from→to warning and leaves the existing + link untouched unless you pass `--force-symlink`; `--no-symlink` opts out of + symlinking entirely. Both flags thread through `aipass install`. Fresh installs + and same-location reinstalls behave exactly as before. Adds a `safe_symlink` + regression test (`tests/setup_symlink_guard_test.sh`) and 3 flag-forwarding + tests; the touched install output was migrated to `@cli` helpers (#661). + - **`drone @flow close` no longer reports a false "timed out after 30s" on a successful close (issue #662).** A single-plan close committed early (plan marked closed, file archived) and then ran memory vectorization diff --git a/setup.sh b/setup.sh index a91445c4..e6a05c46 100755 --- a/setup.sh +++ b/setup.sh @@ -5,10 +5,12 @@ # On interactive terminals it then chains into `aipass init run` to scaffold a first # project (DPLAN-0234: one command does setup + init). # -# Usage: ./setup.sh [--no-init] [--with-init] [--project ] +# Usage: ./setup.sh [--no-init] [--with-init] [--project ] [--no-symlink] [--force-symlink] # --no-init skip the first-project init chain # --with-init force the init chain even headless (init runs --non-interactive) # --project first-project directory (default: ~/aipass-project) +# --no-symlink do not create/modify global drone/aipass CLI symlinks +# --force-symlink repoint a global symlink even if it points at a different install (#660) # set -euo pipefail @@ -39,6 +41,8 @@ esac # default (auto) chains into init on interactive terminals only — CI/headless skip. RUN_INIT="auto" INIT_PROJECT="" +SKIP_SYMLINK="no" +FORCE_SYMLINK="no" PREV_ARG="" for arg in "$@"; do if [ "$PREV_ARG" = "--project" ]; then @@ -47,10 +51,12 @@ for arg in "$@"; do continue fi case "$arg" in - --no-init) RUN_INIT="no" ;; - --with-init) RUN_INIT="yes" ;; - --project=*) INIT_PROJECT="${arg#--project=}" ;; - --project) PREV_ARG="--project" ;; + --no-init) RUN_INIT="no" ;; + --with-init) RUN_INIT="yes" ;; + --no-symlink) SKIP_SYMLINK="yes" ;; + --force-symlink) FORCE_SYMLINK="yes" ;; + --project=*) INIT_PROJECT="${arg#--project=}" ;; + --project) PREV_ARG="--project" ;; *) echo "WARN: unknown argument '$arg' (ignored)" ;; esac done @@ -964,8 +970,42 @@ else fi # --- Create global symlinks for CLI tools (Linux/macOS only) --- +# #660: never SILENTLY hijack a global 'drone'/'aipass' that points at a +# DIFFERENT install. safe_symlink skips a different-target link (loud warning) +# unless --force-symlink; --no-symlink opts out of symlinking entirely. +SYMLINK_SKIPPED=0 +safe_symlink() { + # safe_symlink [sudo] -> 0 linked · 1 skipped(diff target) · 2 ln failed + local src="$1" dest="$2" use_sudo="${3:-}" existing="" + if [ -L "$dest" ]; then + existing="$(readlink "$dest" 2>/dev/null)" + elif [ -e "$dest" ]; then + existing="$dest (real file, not a symlink)" + fi + if [ -n "$existing" ] && [ "$existing" != "$src" ]; then + if [ "$FORCE_SYMLINK" != "yes" ]; then + echo " SKIP $dest — already points at a different install:" + echo " $existing" + echo " Not repointing (would hijack your existing '$(basename "$dest")'); PATH keeps the above." + echo " Re-run 'aipass install' with --force-symlink to repoint here, or --no-symlink to skip quietly." + SYMLINK_SKIPPED=$((SYMLINK_SKIPPED + 1)) + return 1 + fi + echo " WARNING: repointing $dest" + echo " from $existing" + echo " to $src (--force-symlink)" + fi + if [ -n "$use_sudo" ]; then + $use_sudo ln -sf "$src" "$dest" 2>/dev/null && return 0 || return 2 + fi + ln -sf "$src" "$dest" 2>/dev/null && return 0 || return 2 +} + echo "" -if [ "$IS_WINDOWS" -eq 1 ]; then +if [ "$SKIP_SYMLINK" = "yes" ]; then + echo "Skipping global CLI symlinks (--no-symlink)." + echo " 'drone'/'aipass' resolve from $SCRIPT_DIR/.venv/bin — add it to PATH to use them." +elif [ "$IS_WINDOWS" -eq 1 ]; then echo "Windows: drone available via PATH (set above)" elif [ "$IS_MACOS" -eq 1 ]; then # Mac: symlink into ~/.local/bin (user-writable, no sudo needed). @@ -977,9 +1017,11 @@ elif [ "$IS_MACOS" -eq 1 ]; then for cmd in drone aipass; do if [ -f "$VENV_BIN/$cmd" ]; then - if ln -sf "$VENV_BIN/$cmd" "$LOCAL_BIN/$cmd"; then + safe_symlink "$VENV_BIN/$cmd" "$LOCAL_BIN/$cmd" + rc=$? + if [ "$rc" -eq 0 ]; then echo " $LOCAL_BIN/$cmd -> $VENV_BIN/$cmd" - else + elif [ "$rc" -eq 2 ]; then echo " WARN: Could not create symlink for $cmd" echo " Manual fix: ln -sf $VENV_BIN/$cmd $LOCAL_BIN/$cmd" fi @@ -992,14 +1034,20 @@ else for cmd in drone aipass; do if [ -f "$VENV_BIN/$cmd" ]; then - if sudo ln -sf "$VENV_BIN/$cmd" "/usr/local/bin/$cmd" 2>/dev/null; then + safe_symlink "$VENV_BIN/$cmd" "/usr/local/bin/$cmd" "sudo" + rc=$? + if [ "$rc" -eq 0 ]; then echo " /usr/local/bin/$cmd -> $VENV_BIN/$cmd" LINUX_SYMLINK_DIR="/usr/local/bin" + elif [ "$rc" -eq 1 ]; then + : # skipped a different install — safe_symlink explained; do NOT fall back else - # Fallback: user-local bin (no sudo needed) + # sudo/ln failed (e.g. no sudo) — fall back to user-local bin LOCAL_BIN="$HOME/.local/bin" mkdir -p "$LOCAL_BIN" - if ln -sf "$VENV_BIN/$cmd" "$LOCAL_BIN/$cmd"; then + safe_symlink "$VENV_BIN/$cmd" "$LOCAL_BIN/$cmd" + rc=$? + if [ "$rc" -eq 0 ]; then echo " /usr/local/bin failed (no sudo) — using $LOCAL_BIN/$cmd instead" LINUX_SYMLINK_DIR="$LOCAL_BIN" # Ensure ~/.local/bin is on PATH @@ -1009,7 +1057,7 @@ else echo " ~/.local/bin added to PATH in $PROFILE" fi export PATH="$HOME/.local/bin:$PATH" - else + elif [ "$rc" -eq 2 ]; then echo " WARN: Could not create symlink for $cmd" echo " Manual fix: ln -sf $VENV_BIN/$cmd $LOCAL_BIN/$cmd" fi @@ -1018,6 +1066,12 @@ else done fi +if [ "$SYMLINK_SKIPPED" -gt 0 ]; then + echo "" + echo " NOTE: $SYMLINK_SKIPPED global symlink(s) left untouched (pointed at a different install)." + echo " Your existing 'drone'/'aipass' still work. Use --force-symlink to repoint them here." +fi + # --- Result --- echo "" if [ "$FAIL" -eq 0 ]; then diff --git a/src/aipass/aipass/apps/modules/install.py b/src/aipass/aipass/apps/modules/install.py index 6e225738..0d8a471e 100644 --- a/src/aipass/aipass/apps/modules/install.py +++ b/src/aipass/aipass/apps/modules/install.py @@ -43,7 +43,7 @@ import sys from pathlib import Path from typing import Dict -from aipass.cli.apps.modules import console, warning +from aipass.cli.apps.modules import console, success, warning from aipass.prax import logger from aipass.aipass.apps.handlers.json import json_handler @@ -120,20 +120,26 @@ def _clone_repo(home: Path, dry_run: bool) -> bool: return False -def _run_setup(home: Path, dry_run: bool) -> bool: +def _run_setup(home: Path, dry_run: bool, no_symlink: bool = False, force_symlink: bool = False) -> bool: """Run the repo's setup.sh (venv + editable install + hook wiring + binaries).""" setup = home / "setup.sh" + # --no-init: install owns the init handoff (_handoff_to_init) — without it, + # setup.sh's own init chain (DPLAN-0234) would scaffold the project twice. + # --no-symlink / --force-symlink (#660) pass through to setup.sh's CLI-symlink guard. + setup_args = ["bash", str(setup), "--no-init"] + if no_symlink: + setup_args.append("--no-symlink") + if force_symlink: + setup_args.append("--force-symlink") if dry_run: - console.print(f"[yellow]\\[dry-run][/yellow] would run: bash {setup}") + console.print(f"[yellow]\\[dry-run][/yellow] would run: {' '.join(setup_args)}") return True if not setup.is_file(): warning(f"setup.sh not found at {setup} — cannot build the environment.") return False console.print("[cyan]Building environment[/cyan] [dim](venv, dependencies, hook wiring)…[/dim]") try: - # --no-init: install owns the init handoff (_handoff_to_init) — without it, - # setup.sh's own init chain (DPLAN-0234) would scaffold the project twice. - proc = subprocess.run(["bash", str(setup), "--no-init"], cwd=str(home), timeout=_SETUP_TIMEOUT) + proc = subprocess.run(setup_args, cwd=str(home), timeout=_SETUP_TIMEOUT) if proc.returncode == 0: return True logger.warning("[install] setup.sh exited %s", proc.returncode) @@ -162,11 +168,11 @@ def _verify_binaries(home: Path) -> Dict[str, str | None]: ) aipass = _resolve_aipass_bin(home) if drone: - console.print(f"[green]✓[/green] drone: {drone}") + success(f"drone: {drone}") else: warning("drone not found after setup — check the setup output above.") if aipass: - console.print(f"[green]✓[/green] aipass: {aipass}") + success(f"aipass: {aipass}") else: warning("aipass not found after setup — check the setup output above.") return {"drone": drone, "aipass": aipass} @@ -201,7 +207,7 @@ def _handoff_to_init( headless, init is launched headless too so the whole chain stays non-blocking. """ console.print() - console.print(f"[bold green]✓ AIPass is installed at {home}[/bold green]") + success(f"AIPass is installed at {home}") console.print() console.print(" [cyan]drone systems[/cyan] [dim]# list every agent[/dim]") console.print(" [cyan]aipass doctor[/cyan] [dim]# check system health[/dim]") @@ -258,6 +264,8 @@ def run_install( with_init: bool = False, no_init: bool = False, project: str | None = None, + no_symlink: bool = False, + force_symlink: bool = False, ) -> int: """Run the 4-step one-command install. Returns 0 on success, 1 on failure.""" console.print() @@ -278,20 +286,20 @@ def run_install( console.print(f" Home: [cyan]{home}[/cyan]") if _looks_like_aipass_tree(home): - console.print(f"[green]✓[/green] AIPass already present at {home} — skipping download") + success(f"AIPass already present at {home} — skipping download") elif not _clone_repo(home, dry_run): warning("Could not fetch AIPass — aborting install.") return 1 else: - console.print(f"[green]✓[/green] AIPass downloaded to {home}") + success(f"AIPass downloaded to {home}") # Step 2 — build the environment via setup.sh console.print() console.print(render_step_header(2, TOTAL_STEPS, "Building environment")) - if not _run_setup(home, dry_run): + if not _run_setup(home, dry_run, no_symlink=no_symlink, force_symlink=force_symlink): warning("Environment build failed — aborting install.") return 1 - console.print("[green]✓[/green] Environment ready") + success("Environment ready") # Step 3 — verify the binaries landed console.print() @@ -323,6 +331,8 @@ def print_help() -> None: console.print(" [green]aipass install --here[/green] [dim]# install into current dir[/dim]") console.print(" [green]aipass install --no-init[/green] [dim]# install only, skip init[/dim]") console.print(" [green]aipass install --with-init[/green] [dim]# force init even when headless[/dim]") + console.print(" [green]aipass install --no-symlink[/green] [dim]# skip global CLI symlinks[/dim]") + console.print(" [green]aipass install --force-symlink[/green] [dim]# repoint from another install[/dim]") console.print(" [green]aipass install --project DIR[/green] [dim]# where the first project scaffolds[/dim]") console.print(" [green]aipass install --dry-run[/green] [dim]# walk steps, no side effects[/dim]") console.print() @@ -368,6 +378,8 @@ def handle_command(command: str, args: list[str]) -> bool: here = "--here" in run_args with_init = "--with-init" in run_args no_init = "--no-init" in run_args + no_symlink = "--no-symlink" in run_args + force_symlink = "--force-symlink" in run_args path = _flag_value("--path") project = _flag_value("--project") @@ -379,6 +391,8 @@ def handle_command(command: str, args: list[str]) -> bool: with_init=with_init, no_init=no_init, project=project, + no_symlink=no_symlink, + force_symlink=force_symlink, ) json_handler.log_operation( "install_run", diff --git a/src/aipass/aipass/tests/test_install.py b/src/aipass/aipass/tests/test_install.py index 77da87e9..bb1772b9 100644 --- a/src/aipass/aipass/tests/test_install.py +++ b/src/aipass/aipass/tests/test_install.py @@ -146,6 +146,32 @@ class TestRunSetup: assert _run_setup(tmp_path, dry_run=False) is True run.assert_called_once() + def test_no_symlink_flag_forwarded(self, tmp_path: Path) -> None: + """--no-symlink passes through to setup.sh (#660).""" + (tmp_path / "setup.sh").write_text("#!/usr/bin/env bash\n", encoding="utf-8") + with patch(f"{_MOD}.subprocess.run", return_value=MagicMock(returncode=0)) as run: + assert _run_setup(tmp_path, dry_run=False, no_symlink=True) is True + argv = run.call_args[0][0] + assert "--no-symlink" in argv + assert "--force-symlink" not in argv + + def test_force_symlink_flag_forwarded(self, tmp_path: Path) -> None: + """--force-symlink passes through to setup.sh (#660).""" + (tmp_path / "setup.sh").write_text("#!/usr/bin/env bash\n", encoding="utf-8") + with patch(f"{_MOD}.subprocess.run", return_value=MagicMock(returncode=0)) as run: + assert _run_setup(tmp_path, dry_run=False, force_symlink=True) is True + argv = run.call_args[0][0] + assert "--force-symlink" in argv + + def test_symlink_flags_absent_by_default(self, tmp_path: Path) -> None: + """No symlink flags forwarded unless requested (#660).""" + (tmp_path / "setup.sh").write_text("#!/usr/bin/env bash\n", encoding="utf-8") + with patch(f"{_MOD}.subprocess.run", return_value=MagicMock(returncode=0)) as run: + assert _run_setup(tmp_path, dry_run=False) is True + argv = run.call_args[0][0] + assert "--no-symlink" not in argv + assert "--force-symlink" not in argv + class TestRunInstall: """The four-step orchestrator.""" diff --git a/tests/setup_symlink_guard_test.sh b/tests/setup_symlink_guard_test.sh new file mode 100755 index 00000000..cd8f1343 --- /dev/null +++ b/tests/setup_symlink_guard_test.sh @@ -0,0 +1,81 @@ +#!/usr/bin/env bash +# +# Regression test for setup.sh safe_symlink guard (GitHub #660). +# +# #660: `aipass install` (via setup.sh) must NEVER silently repoint a global +# `drone`/`aipass` symlink that points at a DIFFERENT install. This test sources +# the real safe_symlink function out of setup.sh and asserts its behaviour across +# the meaningful cases. Exits 0 on all-pass, non-zero on any regression. +# +# Run: bash tests/setup_symlink_guard_test.sh +set -u + +REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)" +SETUP="$REPO_ROOT/setup.sh" +TMP="$(mktemp -d)" +FAILURES=0 + +cleanup() { rm -f "$TMP"/binA/* "$TMP"/binB/* "$TMP"/dest/* 2>/dev/null; rmdir "$TMP"/binA "$TMP"/binB "$TMP"/dest "$TMP" 2>/dev/null; } +trap cleanup EXIT + +# Pull the real safe_symlink out of setup.sh (single source of truth — no copy). +FN="$TMP/fn.sh" +sed -n '/^safe_symlink() {/,/^}/p' "$SETUP" > "$FN" +if ! grep -q "safe_symlink()" "$FN"; then + echo "FAIL: could not extract safe_symlink from $SETUP" + exit 1 +fi +# shellcheck disable=SC1090 +source "$FN" + +mkdir -p "$TMP/binA" "$TMP/binB" "$TMP/dest" +echo A > "$TMP/binA/aipass" +echo B > "$TMP/binB/aipass" +echo A > "$TMP/binA/drone" + +assert() { # assert