Skip to content

Commit 452a8b0

Browse files
committed
cross-platform: replace os.kill/fcntl with psutil/filelock; fix Windows encoding
Source changes: - restart_daemon.py: replace fcntl.flock with filelock.FileLock (cross-platform), replace os.kill(pid, 15/9) with psutil.Process.terminate/kill - ai_monitor.py: replace os.kill(pid, 0) with psutil.pid_exists, replace os.kill(pid, SIGTERM) with psutil.Process.terminate - client.py: replace os.kill(pid, 0) with psutil.pid_exists - install.py: replace os.kill(pid, 0) with psutil.pid_exists Test changes: - test_daemon_restart_safety.py: replace placeholder tests with real filelock RAII, thread safety, multiprocess exclusion, and psutil lifecycle tests (11 tests, 10 run in CI) - All 17 test files: add encoding="utf-8" to .read_text() calls to fix Windows UnicodeDecodeError (cp1252 default codec)
1 parent 5b87adb commit 452a8b0

22 files changed

Lines changed: 391 additions & 275 deletions

‎plugins/autorun/src/autorun/ai_monitor.py‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
import signal
2222
import logging
2323
from pathlib import Path
24+
25+
import psutil
2426
from contextlib import contextmanager
2527

2628
# Import centralized tmux utilities for DRY compliance
@@ -108,10 +110,12 @@ def start_monitor(session_id, prompt="Continue working", stop_marker=None, max_c
108110
pf = Path(f"/tmp/ai-monitor-{session_id}.pid")
109111
if pf.exists():
110112
try:
111-
os.kill(int(pf.read_text()), 0)
112-
return int(pf.read_text()) # Already running
113-
except (OSError, ValueError):
113+
pid = int(pf.read_text())
114+
if psutil.pid_exists(pid):
115+
return pid # Already running
114116
pf.unlink()
117+
except (OSError, ValueError):
118+
pf.unlink(missing_ok=True)
115119

116120
# Spawn monitor as subprocess
117121
script = Path(__file__)
@@ -128,21 +132,23 @@ def stop_monitor(session_id):
128132
pf = Path(f"/tmp/ai-monitor-{session_id}.pid")
129133
if pf.exists():
130134
try:
131-
os.kill(int(pf.read_text()), signal.SIGTERM)
135+
proc = psutil.Process(int(pf.read_text()))
136+
proc.terminate()
132137
pf.unlink()
133-
except (OSError, ValueError):
134-
pass
138+
except (psutil.NoSuchProcess, psutil.AccessDenied, OSError, ValueError):
139+
pf.unlink(missing_ok=True)
135140

136141
def check_monitor(session_id):
137142
"""Check if monitor is running, return PID or None"""
138143
pf = Path(f"/tmp/ai-monitor-{session_id}.pid")
139144
if pf.exists():
140145
try:
141146
pid = int(pf.read_text())
142-
os.kill(pid, 0)
143-
return pid
144-
except (OSError, ValueError):
147+
if psutil.pid_exists(pid):
148+
return pid
145149
pf.unlink()
150+
except (OSError, ValueError):
151+
pf.unlink(missing_ok=True)
146152
return None
147153

148154
# Core monitor loop

‎plugins/autorun/src/autorun/client.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,8 +268,8 @@ async def forward(depth: int = 0):
268268
if lock_path.exists():
269269
try:
270270
pid = int(lock_path.read_text().strip())
271-
os.kill(pid, 0) # Check if process is alive (signal 0)
272-
daemon_alive = True
271+
import psutil
272+
daemon_alive = psutil.pid_exists(pid)
273273
except (ValueError, ProcessLookupError, PermissionError):
274274
# PID invalid or process dead — safe to spawn new daemon
275275
lock_path.unlink(missing_ok=True)

‎plugins/autorun/src/autorun/install.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,10 @@ def _restart_daemon_if_running() -> None:
162162
if not lock_path.exists():
163163
return
164164
try:
165+
import psutil
165166
pid = int(lock_path.read_text().strip())
166-
os.kill(pid, 0)
167+
if not psutil.pid_exists(pid):
168+
return
167169
except (ValueError, OSError):
168170
return
169171

‎plugins/autorun/src/autorun/restart_daemon.py‎

Lines changed: 25 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,11 @@
2323
import time
2424
import socket
2525
import subprocess
26-
import fcntl
27-
import errno
2826
from contextlib import contextmanager
2927
from pathlib import Path
3028

3129
import psutil
30+
from filelock import FileLock, Timeout
3231

3332
HOME_DIR = Path.home() / ".autorun"
3433
SOCKET_PATH = HOME_DIR / "daemon.sock"
@@ -43,7 +42,8 @@ def get_daemon_pid() -> int | None:
4342
try:
4443
pid = int(LOCK_PATH.read_text().strip())
4544
# Verify process actually exists (not just a stale lock file)
46-
os.kill(pid, 0)
45+
if not psutil.pid_exists(pid):
46+
return None
4747
return pid
4848
except (ValueError, OSError):
4949
return None
@@ -111,56 +111,33 @@ def verify_bashlex() -> bool:
111111
return False
112112

113113

114-
def acquire_restart_lock():
115-
"""Acquire exclusive restart lock to prevent concurrent restarts.
116-
117-
Returns:
118-
file descriptor if lock acquired, None otherwise
119-
120-
Prefer restart_lock() context manager for RAII-safe usage.
121-
"""
122-
try:
123-
lock_fd = open(RESTART_LOCK_PATH, 'w')
124-
fcntl.flock(lock_fd.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB)
125-
lock_fd.write(f"{os.getpid()}\n")
126-
lock_fd.flush()
127-
return lock_fd
128-
except (IOError, OSError) as e:
129-
if hasattr(e, 'errno') and e.errno == errno.EAGAIN:
130-
return None # Another restart in progress
131-
raise
132-
133-
134-
def release_restart_lock(lock_fd) -> None:
135-
"""Release restart lock and cleanup lock file."""
136-
if lock_fd:
137-
try:
138-
fcntl.flock(lock_fd.fileno(), fcntl.LOCK_UN)
139-
lock_fd.close()
140-
except (IOError, OSError):
141-
pass
142-
try:
143-
RESTART_LOCK_PATH.unlink()
144-
except OSError:
145-
pass
146-
147-
148114
@contextmanager
149115
def restart_lock():
150116
"""Context manager for the restart lock. Yields True if acquired, False otherwise.
151117
118+
Uses filelock for cross-platform file locking (works on Unix and Windows).
119+
RAII: lock is only released if it was successfully acquired.
120+
152121
Usage:
153122
with restart_lock() as acquired:
154123
if not acquired:
155124
print("Another restart in progress")
156125
return
157126
# ... do restart work ...
158127
"""
159-
lock_fd = acquire_restart_lock()
128+
lock = FileLock(str(RESTART_LOCK_PATH), timeout=0)
129+
acquired = False
160130
try:
161-
yield lock_fd is not None
131+
lock.acquire()
132+
acquired = True
133+
except Timeout:
134+
pass
135+
136+
try:
137+
yield acquired
162138
finally:
163-
release_restart_lock(lock_fd)
139+
if acquired:
140+
lock.release()
164141

165142

166143
def _resolve_src_dir() -> Path | None:
@@ -325,7 +302,11 @@ def _stop_daemon(pid: int) -> None:
325302
4. If clean but stale files remain: cleanup
326303
"""
327304
print(f" Sending SIGTERM to PID {pid}")
328-
os.kill(pid, 15) # SIGTERM
305+
try:
306+
proc = psutil.Process(pid)
307+
proc.terminate() # SIGTERM on Unix, TerminateProcess on Windows
308+
except (psutil.NoSuchProcess, psutil.AccessDenied):
309+
pass
329310

330311
# Wait for FULL shutdown (PID gone AND socket closed)
331312
shutdown_clean = wait_for_shutdown(max_wait=5.0)
@@ -334,9 +315,10 @@ def _stop_daemon(pid: int) -> None:
334315
# Daemon didn't shut down cleanly - force it
335316
print(" ⚠️ Timeout, forcing shutdown")
336317
try:
337-
os.kill(pid, 9) # SIGKILL
318+
proc = psutil.Process(pid)
319+
proc.kill() # SIGKILL on Unix, TerminateProcess on Windows
338320
time.sleep(0.5)
339-
except OSError:
321+
except (psutil.NoSuchProcess, psutil.AccessDenied):
340322
pass
341323
# Only cleanup stale files if daemon failed to clean up
342324
cleanup_stale_files()

‎plugins/autorun/tests/test_bootstrap_config.py‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ def test_disable_bootstrap_adds_flag(self, temp_hooks_dir):
169169

170170
# Verify hooks.json was modified
171171
hooks_file = Path(temp_hooks_dir) / "hooks" / "claude-hooks.json"
172-
content = hooks_file.read_text()
172+
content = hooks_file.read_text(encoding="utf-8")
173173

174174
# All commands should now have --no-bootstrap
175175
assert "--no-bootstrap" in content
@@ -186,7 +186,7 @@ def test_enable_bootstrap_removes_flag(self, temp_hooks_dir_disabled):
186186

187187
# Verify hooks.json was modified
188188
hooks_file = Path(temp_hooks_dir_disabled) / "hooks" / "claude-hooks.json"
189-
content = hooks_file.read_text()
189+
content = hooks_file.read_text(encoding="utf-8")
190190

191191
# No commands should have --no-bootstrap
192192
assert "--no-bootstrap" not in content
@@ -232,13 +232,13 @@ def test_disable_preserves_json_structure(self, temp_hooks_dir):
232232
from autorun.__main__ import set_bootstrap_config
233233

234234
hooks_file = Path(temp_hooks_dir) / "hooks" / "claude-hooks.json"
235-
original_data = json.loads(hooks_file.read_text())
235+
original_data = json.loads(hooks_file.read_text(encoding="utf-8"))
236236

237237
with mock.patch.dict(os.environ, {"CLAUDE_PLUGIN_ROOT": temp_hooks_dir}):
238238
set_bootstrap_config(enabled=False)
239239

240240
# Verify JSON is still valid
241-
modified_content = hooks_file.read_text()
241+
modified_content = hooks_file.read_text(encoding="utf-8")
242242
modified_data = json.loads(modified_content)
243243

244244
# Structure should be preserved
@@ -415,7 +415,7 @@ def test_no_bootstrap_calls_set_bootstrap_config_false(self, temp_hooks_dir):
415415

416416
# Verify flag was added
417417
hooks_file = Path(temp_hooks_dir) / "hooks" / "claude-hooks.json"
418-
assert "--no-bootstrap" in hooks_file.read_text()
418+
assert "--no-bootstrap" in hooks_file.read_text(encoding="utf-8")
419419

420420
def test_enable_bootstrap_calls_set_bootstrap_config_true(
421421
self, temp_hooks_dir_disabled
@@ -432,7 +432,7 @@ def test_enable_bootstrap_calls_set_bootstrap_config_true(
432432

433433
# Verify flag was removed
434434
hooks_file = Path(temp_hooks_dir_disabled) / "hooks" / "claude-hooks.json"
435-
assert "--no-bootstrap" not in hooks_file.read_text()
435+
assert "--no-bootstrap" not in hooks_file.read_text(encoding="utf-8")
436436

437437
def test_version_returns_zero(self, capsys):
438438
"""Test that --version returns exit code 0."""

‎plugins/autorun/tests/test_claude_e2e_real_money.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -734,7 +734,7 @@ def test_plan_export_posttooluse_exitplanmode_writes_to_notes(
734734
)
735735

736736
# Verify exported content matches our plan
737-
exported_content = exported[0].read_text()
737+
exported_content = exported[0].read_text(encoding="utf-8")
738738
assert unique_marker in exported_content or "E2E Test Plan" in exported_content, (
739739
f"Exported file does not contain expected plan content.\n"
740740
f"Exported file: {exported[0]}\n"

0 commit comments

Comments
 (0)