Windows: restore daemon boot shims + Task Scheduler lifecycle (v2.3.0 still can't start on Windows)#74
Open
danielhertz1999-bit wants to merge 3 commits into
Conversation
main dropped the Windows compatibility layer in 1a333da (the v2.0.0 rewrite merge-base) -- the daemon crashed on launch on this platform with several unconditional POSIX-only calls that the windows-port-v2 branch had already fixed against the older v2.0.0 base but that never made it back into main: - daemon/__init__.py: unconditional `import resource` (POSIX-only) and an eagerly-evaluated `signal.SIGHUP` reference, both gated behind win32 checks - socket_server.py: `inspect.signature(asyncio.start_unix_server)` evaluated before the existing IS_WINDOWS branch that already avoids calling it -- moved below so the attribute is never touched on Windows - Restored src/iai_mcp/_filelock.py (deleted in 1a333da) and repointed capture_queue.py, lifecycle.py, lifecycle_event_log.py, lillibrain/io.py, and doctor/_lifecycle_checks.py at it instead of the stdlib fcntl module Verified: daemon boots clean, reports version 2.2.2, `iai-mcp doctor` passes except the pre-existing/known .daemon.sock non-issue (this daemon uses the TCP+auth-token transport on Windows, not a Unix socket).
…nly calls A follow-up sweep after the boot fix found four more Windows-crash sites that the v2.0.0 rewrite (1a333da) dropped and the v2 port never re-added — the Task Scheduler install/start/uninstall path (present in the v1 port, commit 2fc0296) plus three stray unguarded POSIX-only calls: - cli/_daemon.py + cli/__init__.py: add _is_windows(), SCHTASKS_TASK_NAME, _find_pythonw(), _render_schtasks_xml(); give cmd_daemon_install/uninstall/ start real Windows branches (schtasks /Create+/Run, /End+/Delete, /Run) — previously install/uninstall fell into Linux-only systemctl logic and start printed "Unsupported OS". Also guard the macOS/Linux os.getuid() calls in all three with `hasattr(os, "getuid")` (they crashed before reaching any branch when the mocked-macOS tests run on a Windows host). daemon stop already had its taskkill branch; the schtasks task name matches the live-registered task. - cli/_crypto.py cmd_crypto_status: gate os.geteuid() behind `os.name != "nt"` (mirrors crypto.py's read-path guard); report mode_secure/uid_matches_process as null on Windows, where st_mode/st_uid are synthetic and ACLs govern access. - capture.py, migrate/_to_lilli_verify.py: guard os.fchmod with `hasattr(os, "fchmod")`, matching the existing crypto.py / memory_bank.py sites. Verified on Windows: `crypto status`, `daemon install --dry-run`, and `daemon start` all run cleanly (previously crashed). Targeted CLI test suite goes 26 -> 15 failures (fixed 11, zero regressions); the 15 remaining are pre-existing POSIX-specific test assertions (mode 0o600 vs Windows 0o666, mocked launchctl paths) — the separate Windows test-suite port, out of scope. A whole-tree re-sweep of six POSIX-only bug classes (uid family, fchmod/chown, fcntl/resource imports, POSIX signals + os.kill, AF_UNIX sockets, fork/setsid/ rlimit) found no further unguarded sites.
Follow-up to the previous two commits, from a multi-agent adversarial review (all four fixes confirmed crash-fixed; a whole-tree sweep of six POSIX-only bug classes found no further unguarded sites). Three quality refinements, none a crash: - cli/_crypto.py cmd_crypto_status: restore the original JSON key insertion order (mode, mode_secure, uid, uid_matches_process) so macOS/Linux `crypto status` output is byte-for-byte unchanged (the prior edit reordered uid ahead of mode_secure; json.dumps has no sort_keys, so stdout order had shifted). - cli/_daemon.py _render_schtasks_xml: XML-escape the interpolated USERNAME and interpreter path (xml.sax.saxutils.escape), so a legal '&' in an account name or path can't produce malformed XML that schtasks /Create rejects. - cli/_daemon.py cmd_daemon_install: move _ensure_crypto_key_present() back to its original position on the macOS/Linux path (after the plist/unit is written) and call it inside the Windows branch instead, so the POSIX control flow is byte-for-byte identical while Windows still ensures the key before registering the task. Verified: crypto status renders correct key order; _render_schtasks_xml stays well-formed with '&' in the username; no new test failures.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On Windows, the daemon cannot start at all on current
main(v2.3.0):python -m iai_mcp.daemondies during import/boot on a chain of POSIX-only calls, andiai-mcp daemon install/starthave no Windows path. This PR restores the cross-platform shims (which existed in the v1 Windows port but were dropped in the v2.0.0 rewrite and never re-added) and guards the remaining POSIX-only sites, so the daemon boots and is manageable on Windows again.All changes are no-ops on macOS/Linux — they are guarded by
sys.platform/os.name/hasattr/getattrchecks or route through a shim that delegates to the original stdlib call on POSIX.What was broken on Windows
Starting the daemon crashed in sequence on:
daemon/__init__.py— top-levelimport resource(POSIX-only →ModuleNotFoundError)daemon/__init__.py—signal.SIGHUPreferenced in an eagerly-evaluated tuple (AttributeErrorbefore the surroundingtry/exceptruns)socket_server.py—inspect.signature(asyncio.start_unix_server)evaluated before the existingIS_WINDOWSbranch that already avoids it (AttributeError)capture_queue.py,lifecycle.py,lifecycle_event_log.py,lillibrain/io.py— unconditionalimport fcntl(the_filelock.pyshim they used was deleted in the v2.0.0 rewrite1a333da)doctor/_lifecycle_checks.py::check_c_lock_healthy— localimport fcntlcli/_daemon.py—daemon install/uninstall/startcallos.getuid()before any platform branch and have no Windows branch (install/uninstallfall into Linux-onlysystemctllogic;startprints "Unsupported OS")cli/_crypto.py::cmd_crypto_status— unguardedos.geteuid()(AttributeError)capture.py,migrate/_to_lilli_verify.py—os.fchmodnot guarded (AttributeError; sibling sites incrypto.py/memory_bank.pyalready guard it)Changes
Restore the file-locking shim (commit 1)
src/iai_mcp/_filelock.py— a cross-platformflock-compatible shim (LockFileEx/UnlockFileExon Windows, delegates to stdlibfcntlon POSIX), imported asfrom iai_mcp import _filelock as fcntlso call sites are unchanged.capture_queue.py,lifecycle.py,lifecycle_event_log.py,lillibrain/io.py,doctor/_lifecycle_checks.pyat the shim.daemon/__init__.py: makeimport resourcelocal + gated onsys.platform == "win32"inside_raise_fd_limit()(fd-limit raising is a POSIX-only concept — early-returns on Windows); guardsignal.SIGHUPwithgetattr(signal, "SIGHUP", None).socket_server.py: move theinspect.signature(asyncio.start_unix_server)call below the existingIS_WINDOWSbranch.Task Scheduler daemon management + remaining guards (commit 2)
cli/__init__.py: add_is_windows()andSCHTASKS_TASK_NAME.cli/_daemon.py: add_find_pythonw()+_render_schtasks_xml(), and real Windows branches tocmd_daemon_install/uninstall/start(schtasks /Create+/Run,/End+/Delete,/Run). Guard the macOS/Linuxos.getuid()calls withhasattr(os, "getuid"). (daemon stopalready had itstaskkill /F /Tbranch.)cli/_crypto.py: gateos.geteuid()behindos.name != "nt"(mirrors the read-path guard incrypto.py); reportmode_secure/uid_matches_processasnullon Windows, wherest_mode/st_uidare synthetic and access is governed by ACLs.capture.py,migrate/_to_lilli_verify.py: guardos.fchmodwithhasattr(os, "fchmod"), matching the existingcrypto.py/memory_bank.pysites.Verification (Windows 11, Python 3.12)
python -m iai_mcp.daemonboots toWAKEand reportsversion: 2.2.2(verified on the v2.2.2 base these commits were authored against; rebased cleanly onto v2.3.0 here — all 13 files byte-compile on the new base).iai-mcp daemon status→ healthy;iai-mcp doctorpasses except the pre-existing.daemon.sock-absent check (Windows uses the TCP+auth-token transport, not a Unix socket).iai-mcp crypto status,iai-mcp daemon install --dry-run,iai-mcp daemon startall run cleanly (each previously crashed).test_cli_daemon.py+test_cli_crypto.py) goes 26 → 15 failures on a Windows host (fixed 11, zero regressions vs. the pre-patch baseline). The 15 remaining are pre-existing POSIX-specific test assertions running on Windows (e.g. asserting mode0o600where Windows reports0o666, mockedlaunchctlpaths) — a separate test-suite-port concern, not addressed here.fchmod/chown,fcntl/resourceimports, POSIX signals +os.kill,AF_UNIXsockets,fork/setsid/rlimit) found no further unguarded sites after these changes.Notes
🤖 Generated with Claude Code