Merge pull request #361 from AIOSAI/citizen/ai_mail
feat(ai_mail): fix(daemon): resolve relative registry branch paths to absolute before spawn — fix @ai_mail cwd=repo-root bug (issue #360 finding #3)
This commit is contained in:
@@ -43,6 +43,10 @@ llm = [
|
||||
trinity = [
|
||||
"trinity-pattern>=1.0.0",
|
||||
]
|
||||
memory = [
|
||||
"numpy>=2.0",
|
||||
"chromadb>=1.0",
|
||||
]
|
||||
seedgo = []
|
||||
dev = [
|
||||
"pytest",
|
||||
|
||||
@@ -177,8 +177,8 @@ fi
|
||||
echo "Upgrading pip ..."
|
||||
"$VENV_PYTHON" -m pip install --upgrade pip --quiet
|
||||
|
||||
echo "Installing aipass in editable mode (with dev extras) ..."
|
||||
"$VENV_PYTHON" -m pip install -e ".[dev]" --quiet
|
||||
echo "Installing aipass in editable mode (with dev + memory extras) ..."
|
||||
"$VENV_PYTHON" -m pip install -e ".[dev,memory]" --quiet
|
||||
|
||||
# --- Detect shadowing drone installs (Windows) ---
|
||||
# Issues #317 + #321: system-Python pip or legacy npm aipass-drone can shadow venv drone.exe.
|
||||
|
||||
@@ -635,6 +635,8 @@ def poll_cycle(config: Dict[str, Any], state: Dict[str, Any]) -> int:
|
||||
continue
|
||||
|
||||
branch_path = Path(branch_path_str)
|
||||
if not branch_path.is_absolute():
|
||||
branch_path = _REPO_ROOT / branch_path
|
||||
|
||||
if is_protected_branch(branch_email):
|
||||
continue
|
||||
|
||||
@@ -316,8 +316,11 @@ def main():
|
||||
# even after cd'ing away. Drone reads this as fallback for caller detection.
|
||||
spawn_env["AIPASS_BRANCH_NAME"] = branch_email.lstrip("@")
|
||||
|
||||
# Extract CWD from lock file path (branch_path/.ai_mail.local/.dispatch.lock)
|
||||
lock_path = Path(lock_file)
|
||||
# Extract CWD from lock file path (branch_path/.ai_mail.local/.dispatch.lock).
|
||||
# Resolve to absolute so the cwd passed to claude is never relative — a relative
|
||||
# cwd would be interpreted against dispatch_monitor's own cwd and produce the
|
||||
# wrong directory for branches like @ai_mail whose registry path is relative.
|
||||
lock_path = Path(lock_file).resolve()
|
||||
branch_path = lock_path.parent.parent
|
||||
cwd = str(branch_path)
|
||||
|
||||
|
||||
@@ -652,3 +652,112 @@ def test_scan_and_ack_test_emails_no_inbox(tmp_path):
|
||||
branch_path.mkdir()
|
||||
count = scan_and_ack_test_emails(branch_path, "@testbranch")
|
||||
assert count == 0
|
||||
|
||||
|
||||
# ---- poll_cycle absolute path regression tests ---------------
|
||||
|
||||
|
||||
def test_poll_cycle_resolves_relative_branch_path(tmp_path, monkeypatch):
|
||||
"""poll_cycle resolves relative registry paths to absolute before spawning.
|
||||
|
||||
Regression test for issue #360 finding #3: spawn_agent was called with a
|
||||
relative branch_path when the registry stores paths like 'src/aipass/ai_mail'.
|
||||
A relative cwd cascades through dispatch_monitor, producing the wrong
|
||||
working directory for the spawned claude process.
|
||||
"""
|
||||
# Set up fake repo root and branch inside it
|
||||
repo_root = tmp_path / "repo"
|
||||
branch_dir = repo_root / "src" / "aipass" / "ai_mail"
|
||||
branch_dir.mkdir(parents=True)
|
||||
(branch_dir / ".ai_mail.local").mkdir()
|
||||
inbox = {
|
||||
"messages": [
|
||||
{
|
||||
"id": "d1",
|
||||
"status": "new",
|
||||
"from": "@devpulse",
|
||||
"subject": "probe",
|
||||
"body": "report your pwd",
|
||||
"auto_execute": True,
|
||||
}
|
||||
]
|
||||
}
|
||||
(branch_dir / ".ai_mail.local" / "inbox.json").write_text(json.dumps(inbox))
|
||||
|
||||
# Registry with relative path (real-world format)
|
||||
registry = {"branches": [{"email": "@ai_mail", "path": "src/aipass/ai_mail", "status": "active"}]}
|
||||
(repo_root / "AIPASS_REGISTRY.json").write_text(json.dumps(registry))
|
||||
|
||||
monkeypatch.setattr(daemon_mod, "BRANCH_REGISTRY", repo_root / "AIPASS_REGISTRY.json")
|
||||
monkeypatch.setattr(daemon_mod, "_REPO_ROOT", repo_root)
|
||||
|
||||
spawned_paths = []
|
||||
|
||||
def mock_spawn_agent(branch_path, branch_email, message, config, state):
|
||||
spawned_paths.append(branch_path)
|
||||
return True
|
||||
|
||||
config = {
|
||||
"autonomous_branches": ["@ai_mail"],
|
||||
"max_dispatches_per_branch_per_day": 10,
|
||||
}
|
||||
state = {"daily_counts": {}, "session_cycles": {}}
|
||||
|
||||
with (
|
||||
patch("aipass.ai_mail.apps.handlers.dispatch.daemon.spawn_agent", side_effect=mock_spawn_agent),
|
||||
patch("aipass.ai_mail.apps.handlers.dispatch.daemon._check_lock", return_value=None),
|
||||
patch("aipass.ai_mail.apps.handlers.dispatch.daemon._is_branch_occupied", return_value=False),
|
||||
patch("aipass.ai_mail.apps.handlers.dispatch.daemon.scan_and_ack_test_emails", return_value=0),
|
||||
):
|
||||
daemon_mod.poll_cycle(config, state)
|
||||
|
||||
assert len(spawned_paths) == 1, "Expected one spawn"
|
||||
assert spawned_paths[0].is_absolute(), f"branch_path must be absolute, got: {spawned_paths[0]}"
|
||||
assert spawned_paths[0] == branch_dir
|
||||
|
||||
|
||||
def test_poll_cycle_absolute_path_unchanged(tmp_path, monkeypatch):
|
||||
"""poll_cycle passes already-absolute registry paths through unchanged."""
|
||||
repo_root = tmp_path / "repo"
|
||||
branch_dir = repo_root / "src" / "aipass" / "drone"
|
||||
branch_dir.mkdir(parents=True)
|
||||
(branch_dir / ".ai_mail.local").mkdir()
|
||||
inbox = {
|
||||
"messages": [
|
||||
{
|
||||
"id": "d2",
|
||||
"status": "new",
|
||||
"from": "@devpulse",
|
||||
"subject": "probe",
|
||||
"body": "task",
|
||||
"auto_execute": True,
|
||||
}
|
||||
]
|
||||
}
|
||||
(branch_dir / ".ai_mail.local" / "inbox.json").write_text(json.dumps(inbox))
|
||||
|
||||
registry = {"branches": [{"email": "@drone", "path": str(branch_dir), "status": "active"}]}
|
||||
(repo_root / "AIPASS_REGISTRY.json").write_text(json.dumps(registry))
|
||||
monkeypatch.setattr(daemon_mod, "BRANCH_REGISTRY", repo_root / "AIPASS_REGISTRY.json")
|
||||
monkeypatch.setattr(daemon_mod, "_REPO_ROOT", repo_root)
|
||||
|
||||
spawned_paths = []
|
||||
|
||||
def mock_spawn_agent(branch_path, branch_email, message, config, state):
|
||||
spawned_paths.append(branch_path)
|
||||
return True
|
||||
|
||||
config = {"autonomous_branches": ["@drone"], "max_dispatches_per_branch_per_day": 10}
|
||||
state = {"daily_counts": {}, "session_cycles": {}}
|
||||
|
||||
with (
|
||||
patch("aipass.ai_mail.apps.handlers.dispatch.daemon.spawn_agent", side_effect=mock_spawn_agent),
|
||||
patch("aipass.ai_mail.apps.handlers.dispatch.daemon._check_lock", return_value=None),
|
||||
patch("aipass.ai_mail.apps.handlers.dispatch.daemon._is_branch_occupied", return_value=False),
|
||||
patch("aipass.ai_mail.apps.handlers.dispatch.daemon.scan_and_ack_test_emails", return_value=0),
|
||||
):
|
||||
daemon_mod.poll_cycle(config, state)
|
||||
|
||||
assert len(spawned_paths) == 1
|
||||
assert spawned_paths[0].is_absolute()
|
||||
assert spawned_paths[0] == branch_dir
|
||||
|
||||
@@ -565,6 +565,26 @@
|
||||
"standard": "unused_function",
|
||||
"lines": [427],
|
||||
"reason": "LLM-based extraction function — called conditionally when API key is available."
|
||||
},
|
||||
{
|
||||
"file": "tests/test_vector.py",
|
||||
"standard": "architecture",
|
||||
"reason": "Test file — lives in tests/ by design, not in 3-layer apps/ structure."
|
||||
},
|
||||
{
|
||||
"file": "tests/test_vector.py",
|
||||
"standard": "documentation",
|
||||
"reason": "Test file — test functions don't require docstrings."
|
||||
},
|
||||
{
|
||||
"file": "tests/test_vector.py",
|
||||
"standard": "encapsulation",
|
||||
"reason": "Test file — direct handler imports are correct for unit testing handler internals."
|
||||
},
|
||||
{
|
||||
"file": "tests/test_vector.py",
|
||||
"standard": "meta",
|
||||
"reason": "Test file — META block present at lines 1-7; hook false-positive on test file format."
|
||||
}
|
||||
],
|
||||
"notes": {
|
||||
|
||||
@@ -125,6 +125,41 @@ def _get_rollover_threshold(branch_name: str, file_path: Path | None = None) ->
|
||||
return 600
|
||||
|
||||
|
||||
def _check_vector_deps() -> bool:
|
||||
"""
|
||||
Check whether the memory venv has chromadb and numpy available.
|
||||
|
||||
Runs a quick subprocess check using the memory venv Python.
|
||||
Logs a warning if deps are missing so the self-report is honest.
|
||||
|
||||
Returns:
|
||||
True if both chromadb and numpy are importable, False otherwise
|
||||
"""
|
||||
import subprocess
|
||||
import sys
|
||||
|
||||
venv_python = _MEMORY_ROOT / ".venv" / "bin" / "python3"
|
||||
if not venv_python.exists():
|
||||
venv_python = Path(sys.executable)
|
||||
|
||||
try:
|
||||
result = subprocess.run(
|
||||
[str(venv_python), "-c", "import chromadb; import numpy"],
|
||||
capture_output=True,
|
||||
timeout=10,
|
||||
)
|
||||
if result.returncode != 0:
|
||||
logger.warning(
|
||||
"[memory_watcher] vector storage degraded — chromadb/numpy not available in memory venv. "
|
||||
"Run: pip install -e '.[dev,memory]'"
|
||||
)
|
||||
return False
|
||||
return True
|
||||
except Exception as e:
|
||||
logger.warning(f"[memory_watcher] vector dep check failed: {e}")
|
||||
return False
|
||||
|
||||
|
||||
def check_and_rollover() -> Dict[str, Any]:
|
||||
"""
|
||||
Check all memory files and trigger rollover if any exceed their threshold.
|
||||
@@ -146,12 +181,15 @@ def check_and_rollover() -> Dict[str, Any]:
|
||||
|
||||
_startup_check_done = True
|
||||
|
||||
vector_deps_ok = _check_vector_deps()
|
||||
|
||||
results = {
|
||||
"success": True,
|
||||
"files_checked": 0,
|
||||
"files_over_limit": [],
|
||||
"rollover_triggered": False,
|
||||
"memory_pool": None,
|
||||
"vector_storage": "healthy" if vector_deps_ok else "degraded — chromadb/numpy not installed",
|
||||
}
|
||||
|
||||
# Get all branch paths
|
||||
|
||||
@@ -22,7 +22,10 @@ import sys
|
||||
from typing import Any
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import numpy as np
|
||||
import pytest
|
||||
|
||||
np = pytest.importorskip("numpy")
|
||||
pytest.importorskip("chromadb")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user