Windows CI cross-platform fixes (14 failures -> windows-setup green, PR659). Unmasked by the #691 collection fix; 6 branches. CAUSE 1 pid-liveness (ai_mail/flow/hooks/skills): production _is_pid_alive already cross-plat (ctypes OpenProcess on win32), but tests mocked os.kill which the win32 path never reaches -> pinned sys.platform=linux (or patched _is_pid_alive) so tests exercise the POSIX contract on every platform. CAUSE 2 path assumptions: prax jsonl str(Path) backslash, hooks rollover repr()/%r, ai_mail darwin lsof fixed posix path, seedgo is_bypassed Path.as_posix normalization (only production change). 10 files (9 test, 1 code). Owners self-fixed; devpulse verified diffs + Linux no-regression 525 changed-test green. CI Windows verifies.

This commit is contained in:
AIOSAI
2026-07-11 12:16:28 -07:00
parent 7c9309cf88
commit ca096295a3
11 changed files with 73 additions and 59 deletions
+16
View File
@@ -34,6 +34,22 @@ PyPI version — not the changelog header.
### Fixed
- **Windows CI cross-platform fixes — `windows-setup` green (PR659).** Fixing the
telegram collection errors unmasked 14 pre-existing Windows-only failures across
six branches. Two root causes. **(1) pid-liveness tests** (ai_mail, flow, hooks,
skills) mocked `os.kill`, but the production `_is_pid_alive` already branches to a
ctypes `OpenProcess` path on Windows and never reaches `os.kill`, so the mocks had
no effect and the real path ran instead — pinned `sys.platform` to `linux` in those
tests (or patched `_is_pid_alive` directly) so they exercise the POSIX contract
deterministically on every platform. **(2) POSIX path assumptions** — prax's jsonl
test hardcoded `/some/path` (backslashes under `str(Path)` on Windows) now asserts
against `str(test_path)`; hooks' rollover test compares `repr()` (matches `%r`
logging); ai_mail's darwin lsof-parser test uses a fixed POSIX path; and seedgo's
`is_bypassed()` now normalizes the rule file via `Path(rule_file).as_posix()` before
matching (the one production fix — Windows backslash rule paths never matched the
forward-slash file path). 10 files (9 test, 1 code); owners self-fixed, devpulse
verified every diff + Linux no-regression (525 changed-test assertions green).
- **Flaky `test_deletes_old_system_log` made deterministic (@prax log-sweep tests).**
The sweep integration test reached `log_watchdog._get_system_logs_dir` through a
`_get_sweep()` wrapper and patched it by string path; a sibling test
+14 -21
View File
@@ -9,10 +9,11 @@
"""Tests for dispatch daemon handler -- config loading, state management, inbox scanning."""
import json
import os
import sys
import pytest
from datetime import datetime, date, timedelta
from unittest.mock import patch
from unittest.mock import MagicMock, mock_open, patch
import aipass.ai_mail.apps.handlers.dispatch.daemon as daemon_mod
from aipass.ai_mail.apps.handlers.dispatch.daemon import (
@@ -25,6 +26,17 @@ from aipass.ai_mail.apps.handlers.dispatch.daemon import (
get_registered_branches,
check_inbox_for_dispatch,
is_protected_branch,
_handle_signal,
_check_lock,
_acquire_lock,
_is_registered_sender,
poll_cycle,
_write_pid_file,
_remove_pid_file,
_read_session_type,
_is_branch_occupied,
spawn_agent,
run_daemon,
)
@@ -764,26 +776,6 @@ def test_poll_cycle_absolute_path_unchanged(tmp_path, monkeypatch):
assert spawned_paths[0] == branch_dir
# ---- Additional imports for new tests --------------------------------
import os
from unittest.mock import MagicMock, mock_open
from aipass.ai_mail.apps.handlers.dispatch.daemon import (
_handle_signal,
_check_lock,
_acquire_lock,
_is_registered_sender,
poll_cycle,
_write_pid_file,
_remove_pid_file,
_read_session_type,
_is_branch_occupied,
spawn_agent,
run_daemon,
)
# ---- _handle_signal tests --------------------------------------
@@ -1003,6 +995,7 @@ def test_write_pid_file_existing_dead_pid(tmp_path, monkeypatch):
def test_write_pid_file_existing_permission_error(tmp_path, monkeypatch):
"""Existing PID file with PermissionError on kill returns False."""
monkeypatch.setattr("sys.platform", "linux")
pid_file = tmp_path / "daemon.pid"
pid_file.write_text("888888", encoding="utf-8")
monkeypatch.setattr(daemon_mod, "DAEMON_PID_FILE", pid_file)
+3 -1
View File
@@ -142,6 +142,7 @@ def test_check_pid_alive_dead(monkeypatch):
def test_check_pid_alive_permission_error(monkeypatch):
"""PermissionError means process exists but cannot signal -- returns True."""
monkeypatch.setattr("sys.platform", "linux")
monkeypatch.setattr(os, "kill", _raise_permission)
assert _check_pid_alive(1) is True
@@ -260,7 +261,7 @@ def test_get_pid_cwd_linux_oserror(monkeypatch):
def test_get_pid_cwd_darwin(monkeypatch, tmp_path):
"""macOS: reads cwd via lsof."""
monkeypatch.setattr("sys.platform", "darwin")
target = str(tmp_path / "project")
target = "/tmp/pytest-project"
class FakeResult:
returncode = 0
@@ -362,6 +363,7 @@ def test_check_lock_stale_old_timestamp(tmp_path, monkeypatch):
def test_check_lock_permission_error_treated_active(tmp_path, monkeypatch):
"""Lock PID that raises PermissionError is treated as active."""
monkeypatch.setattr("sys.platform", "linux")
lock_dir = tmp_path / ".ai_mail.local"
lock_dir.mkdir(parents=True)
lock_file = lock_dir / ".dispatch.lock"
+6 -6
View File
@@ -79,7 +79,7 @@ class TestIsLockStale:
mod = _import_lock_ops()
lock = tmp_path / ".test.lock"
lock.write_text("999999999", encoding="utf-8")
with patch(f"{_MOD}.os.kill", side_effect=ProcessLookupError):
with patch(f"{_MOD}._pid_alive", return_value=False):
result = mod.is_lock_stale(lock)
assert result is True
@@ -100,11 +100,11 @@ class TestIsLockStale:
assert result is True
def test_permission_error_treated_as_alive(self, tmp_path):
"""PermissionError from os.kill means process exists — lock valid."""
"""When _pid_alive says process exists, lock is valid (not stale)."""
mod = _import_lock_ops()
lock = tmp_path / ".test.lock"
lock.write_text("1", encoding="utf-8")
with patch(f"{_MOD}.os.kill", side_effect=PermissionError):
with patch(f"{_MOD}._pid_alive", return_value=True):
result = mod.is_lock_stale(lock)
assert result is False
@@ -138,7 +138,7 @@ class TestAcquireLock:
mod = _import_lock_ops()
lock = tmp_path / ".test.lock"
lock.write_text("999999999", encoding="utf-8")
with patch(f"{_MOD}.os.kill", side_effect=ProcessLookupError):
with patch(f"{_MOD}._pid_alive", return_value=False):
result = mod.acquire_lock(lock)
assert result is True
assert lock.read_text(encoding="utf-8") == str(os.getpid())
@@ -149,7 +149,7 @@ class TestAcquireLock:
lock = tmp_path / ".test.lock"
lock.write_text("999999999", encoding="utf-8")
with (
patch(f"{_MOD}.os.kill", side_effect=ProcessLookupError),
patch(f"{_MOD}._pid_alive", return_value=False),
patch.object(Path, "unlink", side_effect=OSError("permission denied")),
):
result = mod.acquire_lock(lock)
@@ -170,7 +170,7 @@ class TestAcquireLock:
mod = _import_lock_ops()
lock = tmp_path / ".test.lock"
lock.write_text("999999999", encoding="utf-8")
with patch(f"{_MOD}.os.kill", side_effect=ProcessLookupError):
with patch(f"{_MOD}._pid_alive", return_value=False):
mod.acquire_lock(lock)
call_args = mock_json_handler.call_args
assert call_args[0][1]["stale_recovery"] is True
+1 -1
View File
@@ -21,7 +21,7 @@ class TestIsPidAlive:
assert cc_sessions._is_pid_alive(1) is False
def test_permission_error_treated_as_alive(self):
with patch("os.kill", side_effect=PermissionError("denied")):
with patch("sys.platform", "linux"), patch("os.kill", side_effect=PermissionError("denied")):
assert cc_sessions._is_pid_alive(42) is True
def test_oserror_treated_as_dead(self):
+2 -2
View File
@@ -435,7 +435,7 @@ class TestReadAll:
class TestLiveness:
def test_is_pid_alive_true(self):
with patch("os.kill") as mock_kill:
with patch("sys.platform", "linux"), patch("os.kill") as mock_kill:
assert presence._is_pid_alive(1234) is True
mock_kill.assert_called_once_with(1234, 0)
@@ -444,7 +444,7 @@ class TestLiveness:
assert presence._is_pid_alive(1234) is False
def test_is_pid_alive_permission_error(self):
with patch("os.kill", side_effect=PermissionError):
with patch("sys.platform", "linux"), patch("os.kill", side_effect=PermissionError):
assert presence._is_pid_alive(1234) is True
def test_cwd_matches_linux(self):
+1 -1
View File
@@ -166,4 +166,4 @@ class TestFindRepoRootFailLoud:
assert result is None
assert "_find_repo_root failed" in caplog.text
assert bad_home in caplog.text
assert repr(bad_home) in caplog.text
+3 -2
View File
@@ -69,11 +69,12 @@ class TestAppendJsonl:
"""Verify non-serializable types fall back to str()."""
append_jsonl = _get_append_jsonl()
target = tmp_path / "test.jsonl"
test_path = Path("/some/path")
append_jsonl(target, {"path": Path("/some/path")})
append_jsonl(target, {"path": test_path})
line = json.loads(target.read_text().strip())
assert line["path"] == "/some/path"
assert line["path"] == str(test_path)
class TestRotation:
@@ -40,7 +40,7 @@ def is_bypassed(
if rule.get("standard") and rule.get("standard") != standard:
continue
rule_file = rule.get("file", "")
if rule_file and rule_file not in file_path_posix:
if rule_file and Path(rule_file).as_posix() not in file_path_posix:
continue
functions = rule.get("functions")
if functions and name is not None:
@@ -923,28 +923,28 @@ class TestCreateCommand:
mock_validate.return_value = {"name": "dev_central", "path": "/home/aipass/dev_central"}
self.bot._handle_create_command(self.chat_id, "chat dev_central")
assert self.chat_id in self.bot._create_state
self.bot.send_message.assert_called_once()
msg = self.bot.send_message.call_args[0][1]
self.bot.send_message.assert_called_once() # type: ignore[union-attr]
msg = self.bot.send_message.call_args[0][1] # type: ignore[union-attr]
assert "dev_central" in msg
assert "token" in msg.lower()
def test_create_missing_args(self):
self.bot._handle_create_command(self.chat_id, "")
self.bot.send_message.assert_called_once()
msg = self.bot.send_message.call_args[0][1]
self.bot.send_message.assert_called_once() # type: ignore[union-attr]
msg = self.bot.send_message.call_args[0][1] # type: ignore[union-attr]
assert "Usage" in msg
def test_create_invalid_format(self):
self.bot._handle_create_command(self.chat_id, "foo bar")
self.bot.send_message.assert_called_once()
msg = self.bot.send_message.call_args[0][1]
self.bot.send_message.assert_called_once() # type: ignore[union-attr]
msg = self.bot.send_message.call_args[0][1] # type: ignore[union-attr]
assert "Usage" in msg
@patch("aipass.skills.lib.telegram.apps.handlers.base_bot.validate_branch", return_value=None)
def test_create_branch_not_found(self, mock_validate):
self.bot._handle_create_command(self.chat_id, "chat nonexistent")
self.bot.send_message.assert_called_once()
msg = self.bot.send_message.call_args[0][1]
self.bot.send_message.assert_called_once() # type: ignore[union-attr]
msg = self.bot.send_message.call_args[0][1] # type: ignore[union-attr]
assert "not found" in msg
@patch("aipass.skills.lib.telegram.apps.handlers.base_bot.get_bot_by_branch")
@@ -953,8 +953,8 @@ class TestCreateCommand:
mock_validate.return_value = {"name": "dev_central", "path": "/tmp"}
mock_get_bot.return_value = {"bot_id": "dev_central", "username": "dc_bot"}
self.bot._handle_create_command(self.chat_id, "chat dev_central")
self.bot.send_message.assert_called_once()
msg = self.bot.send_message.call_args[0][1]
self.bot.send_message.assert_called_once() # type: ignore[union-attr]
msg = self.bot.send_message.call_args[0][1] # type: ignore[union-attr]
assert "already has a bot" in msg
@patch("aipass.skills.lib.telegram.apps.handlers.base_bot.get_bot_by_branch", return_value=None)
@@ -970,8 +970,8 @@ class TestCreateCommand:
def test_create_single_arg_no_branch(self):
"""Calling /create with only 'chat' and no branch name shows usage."""
self.bot._handle_create_command(self.chat_id, "chat")
self.bot.send_message.assert_called_once()
self.bot._handle_create_command(self.chat_id, "chat") # type: ignore[union-attr]
self.bot.send_message.assert_called_once() # type: ignore[union-attr]
msg = self.bot.send_message.call_args[0][1]
assert "Usage" in msg
@@ -1022,13 +1022,13 @@ class TestCreateToken:
self.bot._handle_create_token(self.chat_id, "123456789:ABCdefGHIjklMNOpqr")
# Should have been called
mock_create_bot.assert_called_once()
# Last send_message should contain success info
# Last send_message should contain success info # type: ignore[union-attr]
last_msg = self.bot.send_message.call_args[0][1]
assert "my_new_bot" in last_msg
def test_invalid_token_format_no_colon(self):
self._set_create_state()
self.bot._handle_create_token(self.chat_id, "shorttoken")
self.bot._handle_create_token(self.chat_id, "shorttoken") # type: ignore[union-attr]
msg = self.bot.send_message.call_args[0][1]
assert "doesn't look like a valid" in msg
# State should still be present (user can retry)
@@ -1036,7 +1036,7 @@ class TestCreateToken:
def test_invalid_token_format_too_short(self):
self._set_create_state()
self.bot._handle_create_token(self.chat_id, "1:A")
self.bot._handle_create_token(self.chat_id, "1:A") # type: ignore[union-attr]
msg = self.bot.send_message.call_args[0][1]
assert "doesn't look like a valid" in msg
@@ -1044,7 +1044,7 @@ class TestCreateToken:
def test_token_validation_fails(self, mock_validate_token):
self._set_create_state()
self.bot._handle_create_token(self.chat_id, "123456789:ABCdefGHIjklMNOpqr")
mock_validate_token.assert_called_once()
mock_validate_token.assert_called_once() # type: ignore[union-attr]
msg = self.bot.send_message.call_args[0][1]
assert "validation failed" in msg.lower()
@@ -1053,7 +1053,7 @@ class TestCreateToken:
def test_bot_creation_fails(self, mock_validate_token, mock_create_bot):
self._set_create_state()
mock_validate_token.return_value = {"username": "test_bot"}
self.bot._handle_create_token(self.chat_id, "123456789:ABCdefGHIjklMNOpqr")
self.bot._handle_create_token(self.chat_id, "123456789:ABCdefGHIjklMNOpqr") # type: ignore[union-attr]
msg = self.bot.send_message.call_args[0][1]
assert "failed" in msg.lower()
@@ -1061,7 +1061,7 @@ class TestCreateToken:
"""State older than _create_state_ttl should be rejected."""
old_time = time.time() - 600 # 10 minutes ago, TTL is 300s
self._set_create_state(started_at=old_time)
self.bot._handle_create_token(self.chat_id, "123456789:ABCdefGHIjklMNOpqr")
self.bot._handle_create_token(self.chat_id, "123456789:ABCdefGHIjklMNOpqr") # type: ignore[union-attr]
msg = self.bot.send_message.call_args[0][1]
assert "expired" in msg.lower()
assert self.chat_id not in self.bot._create_state
@@ -1125,7 +1125,7 @@ class TestCancelCommand:
}
with patch("aipass.skills.lib.telegram.apps.handlers.base_bot.parse_command", return_value=("cancel", "")):
self.bot.process_update(update)
assert self.chat_id not in self.bot._create_state
assert self.chat_id not in self.bot._create_state # type: ignore[union-attr]
msg = self.bot.send_message.call_args[0][1]
assert "cancelled" in msg.lower()
@@ -1140,7 +1140,7 @@ class TestCancelCommand:
},
}
with patch("aipass.skills.lib.telegram.apps.handlers.base_bot.parse_command", return_value=("cancel", "")):
self.bot.process_update(update)
self.bot.process_update(update) # type: ignore[union-attr]
msg = self.bot.send_message.call_args[0][1]
assert "Nothing to cancel" in msg
@@ -1711,6 +1711,7 @@ class TestLockPidReuse:
assert self.bot._check_lock() is False
assert not self.bot._lock_file.exists()
@patch("sys.platform", "linux")
@patch("aipass.skills.lib.telegram.apps.handlers.base_bot.os.kill")
def test_alive_pid_same_bot_returns_true(self, mock_kill):
"""Live PID running this bot returns True (lock held)."""
@@ -1726,6 +1727,7 @@ class TestLockPidReuse:
assert self.bot._check_lock() is True
assert self.bot._lock_file.exists() # Lock preserved
@patch("sys.platform", "linux")
@patch("aipass.skills.lib.telegram.apps.handlers.base_bot.os.kill")
def test_alive_pid_different_bot_cleans_lock(self, mock_kill):
"""Live PID running a DIFFERENT bot cleans stale lock (PID reuse)."""
@@ -221,12 +221,12 @@ class TestIsPidAlive:
def test_permission_error_treated_as_alive(self):
"""Treats PermissionError from os.kill as evidence the PID is alive."""
with patch("os.kill", side_effect=PermissionError("denied")):
with patch("sys.platform", "linux"), patch("os.kill", side_effect=PermissionError("denied")):
assert BaseBot._is_pid_alive(42) is True
def test_os_error_treated_as_dead(self):
"""Treats a generic OSError from os.kill as evidence the PID is dead."""
with patch("os.kill", side_effect=OSError("some error")):
with patch("sys.platform", "linux"), patch("os.kill", side_effect=OSError("some error")):
assert BaseBot._is_pid_alive(42) is False
@@ -546,7 +546,7 @@ class TestHandleMessageNoSession:
patch("subprocess.run", return_value=MagicMock(returncode=1)),
):
bot.handle_message(42, "hello", {"message_id": 1})
msg = bot.send_message.call_args[0][1]
msg = bot.send_message.call_args[0][1] # type: ignore[union-attr]
assert "No live Claude session" in msg
assert "api" in msg
@@ -559,5 +559,5 @@ class TestHandleMessageNoSession:
patch("subprocess.run", return_value=MagicMock(returncode=1)),
):
bot.handle_message(42, "hello", {"message_id": 1})
msg = bot.send_message.call_args[0][1]
msg = bot.send_message.call_args[0][1] # type: ignore[union-attr]
assert "No live Claude session" in msg