From ca096295a39c94f3fca31ffe9f2e9bf022763e39 Mon Sep 17 00:00:00 2001 From: AIOSAI Date: Sat, 11 Jul 2026 12:16:28 -0700 Subject: [PATCH] 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. --- CHANGELOG.md | 16 +++++++ src/aipass/ai_mail/tests/test_daemon.py | 35 +++++++--------- src/aipass/ai_mail/tests/test_wake.py | 4 +- src/aipass/flow/tests/test_lock_ops.py | 12 +++--- src/aipass/hooks/tests/test_cc_sessions.py | 2 +- src/aipass/hooks/tests/test_presence.py | 4 +- src/aipass/hooks/tests/test_rollover.py | 2 +- src/aipass/prax/tests/test_jsonl_writer.py | 5 ++- .../seedgo/apps/handlers/bypass/utils.py | 2 +- .../lib/telegram/tests/test_multi_bot.py | 42 ++++++++++--------- .../telegram/tests/test_presence_pointer.py | 8 ++-- 11 files changed, 73 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d025c0e1..f8c67069 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/aipass/ai_mail/tests/test_daemon.py b/src/aipass/ai_mail/tests/test_daemon.py index fe153bcd..96497fb1 100644 --- a/src/aipass/ai_mail/tests/test_daemon.py +++ b/src/aipass/ai_mail/tests/test_daemon.py @@ -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) diff --git a/src/aipass/ai_mail/tests/test_wake.py b/src/aipass/ai_mail/tests/test_wake.py index fc6ccf5a..a36c1a01 100644 --- a/src/aipass/ai_mail/tests/test_wake.py +++ b/src/aipass/ai_mail/tests/test_wake.py @@ -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" diff --git a/src/aipass/flow/tests/test_lock_ops.py b/src/aipass/flow/tests/test_lock_ops.py index 36f6fa40..5259411a 100644 --- a/src/aipass/flow/tests/test_lock_ops.py +++ b/src/aipass/flow/tests/test_lock_ops.py @@ -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 diff --git a/src/aipass/hooks/tests/test_cc_sessions.py b/src/aipass/hooks/tests/test_cc_sessions.py index 2911c687..513249fe 100644 --- a/src/aipass/hooks/tests/test_cc_sessions.py +++ b/src/aipass/hooks/tests/test_cc_sessions.py @@ -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): diff --git a/src/aipass/hooks/tests/test_presence.py b/src/aipass/hooks/tests/test_presence.py index 3d551f30..628d9a2c 100644 --- a/src/aipass/hooks/tests/test_presence.py +++ b/src/aipass/hooks/tests/test_presence.py @@ -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): diff --git a/src/aipass/hooks/tests/test_rollover.py b/src/aipass/hooks/tests/test_rollover.py index 679df460..54809df3 100644 --- a/src/aipass/hooks/tests/test_rollover.py +++ b/src/aipass/hooks/tests/test_rollover.py @@ -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 diff --git a/src/aipass/prax/tests/test_jsonl_writer.py b/src/aipass/prax/tests/test_jsonl_writer.py index 8a48e811..2b873e07 100644 --- a/src/aipass/prax/tests/test_jsonl_writer.py +++ b/src/aipass/prax/tests/test_jsonl_writer.py @@ -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: diff --git a/src/aipass/seedgo/apps/handlers/bypass/utils.py b/src/aipass/seedgo/apps/handlers/bypass/utils.py index a1c7633f..adeb477e 100644 --- a/src/aipass/seedgo/apps/handlers/bypass/utils.py +++ b/src/aipass/seedgo/apps/handlers/bypass/utils.py @@ -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: diff --git a/src/aipass/skills/lib/telegram/tests/test_multi_bot.py b/src/aipass/skills/lib/telegram/tests/test_multi_bot.py index 8b8352b3..cc08d78f 100644 --- a/src/aipass/skills/lib/telegram/tests/test_multi_bot.py +++ b/src/aipass/skills/lib/telegram/tests/test_multi_bot.py @@ -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).""" diff --git a/src/aipass/skills/lib/telegram/tests/test_presence_pointer.py b/src/aipass/skills/lib/telegram/tests/test_presence_pointer.py index a70dc07c..b1fcecbf 100644 --- a/src/aipass/skills/lib/telegram/tests/test_presence_pointer.py +++ b/src/aipass/skills/lib/telegram/tests/test_presence_pointer.py @@ -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