Deploy/windows fixes integrated#64
Open
danielhertz1999-bit wants to merge 19 commits into
Open
Conversation
Rebased onto upstream/main (which carries its own Windows port); this commit re-applies only the deltas upstream still lacks. Product fix: - crypto._try_file_set: os.open defaults to text mode on Windows, which translated 0x0A bytes in the raw AES key to 0x0D0A and silently corrupted it. Add os.O_BINARY (no-op on POSIX). Still unfixed upstream. - bench/contradiction_longitudinal: write CSV/JSON/MD as UTF-8 (cp1252 default crashed on the delta glyph). Test portability: - conftest: make Path.home() honor $HOME on Windows and mirror HOME->USERPROFILE so the home-redirecting fixtures (and their subprocesses) stay hermetic. - UTF-8 reads in source-scanning guards (constitutional_guards, bench_worktree, contradiction route columns). - Skip POSIX-only checks on Windows (archive 0o700, crypto geteuid/uid, launchd getuid); the product already guards these behind hasattr(os,geteuid). - cpu_features: match /proc/cpuinfo via as_posix() (Windows renders backslashes). - capture_hooks_install: assert .ps1/powershell hooks on Windows, .sh/bash on POSIX. - daemon_kill_mid_consolidation: SIGKILL->SIGTERM (TerminateProcess) fallback plus a bounded reopen-retry for Windows async byte-range lock release. - bridge_socket_first: skip on Windows (npm/mcp-wrapper build path is POSIX-only), matching test_bridge_no_spawn_path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
os.kill(pid, 0) is the POSIX "is this PID alive?" idiom, but on Windows
os.kill rejects signal 0 with OSError [WinError 87] (invalid parameter)
even for a live process. Three call sites treated that exception as
"process is dead", which made a healthy, actively-ticking daemon report
as down:
- doctor check (a) "daemon process alive" -> live daemon reported dead
- doctor check (m) heartbeat scanner -> scan aborted with WinError 87
- maintenance pre-flight daemon guard -> always concluded "no daemon
running" on Windows, which could let maintenance run against a store the
live daemon still holds
Each now uses psutil.pid_exists (correct and cross-platform; psutil is
already a hard dep), falling back to os.kill only on POSIX -- matching the
existing pattern in capture.py and lifecycle_lock._is_pid_alive. Verified:
doctor 4/25 -> 2/25 FAIL, checks (a) and (m) now pass and correctly
identify the live daemon PID. 149 relevant tests pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
uuid4() was unseeded, so near-tied scores at the top-K fidelity cutoff could flip the str(id) tie-break between runs, making test_centrality_for_runtime_approx_above_cutoff flaky independent of the approximation algorithm. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
On Windows the daemon authenticates clients with a token it generates at startup, writes to ~/.iai-mcp/.daemon.token, and holds in memory. If that file later disappears out-of-band (external cleanup, AV quarantine, a stale-file sweep) while the daemon keeps running, the TCP port stays up but EVERY new client fails the auth handshake — existing long-lived connections survive, so the daemon looks alive while new sessions and `daemon status` all report "daemon not running". The only recovery was a manual restart. The daemon still holds the token in memory, so it can restore the file cheaply. Add _ipc.reassert_token_if_missing() (idempotent; no-op on POSIX, in client processes, or when the file is present) and call it as the first step of the daemon tick. _generate_token now stashes the token in a module global (_CURRENT_TOKEN); _remove_token_file clears it so a deliberate shutdown/restart can't be resurrected. Observed in production on Windows 11: .daemon.token gone while PID was listening and serving old clients; new connect raised "Daemon auth token not found". Confirmed nothing in the daemon's own code removes the token without also removing the port file (which was intact), so the deletion was external — exactly the single-point-of-failure this heals. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
check (b) "socket file fresh" reported a missing-token connectivity failure
as "<...>.daemon.sock present but unreachable: FileNotFoundError" — doubly
misleading on Windows, where there is no .daemon.sock (the daemon listens on
TCP loopback) and the FileNotFoundError actually meant the auth token file
was gone, not the socket.
Report the real endpoint (127.0.0.1:<port>) instead of the nonexistent sock
path, and when the token file is missing while the port is listening, say so
explicitly with the remediation ("restart the daemon to regenerate it").
_socket_connect_probe now surfaces the underlying FileNotFoundError message
so a missing port vs a missing token are distinguishable.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Node MCP wrapper's PythonCoreBridge never sent the Windows TCP auth token before writing RPC requests, so the daemon's _make_authenticated_handler (added same-day in 21fb055) silently closed every connection. Add authenticateConnection() to ipc.ts and call it at all three of bridge.ts's connection sites. Also add episodes-recent/curiosity-pending/events-query/schema-list CLI subcommands with the same daemon-first/direct-store-fallback shape as the existing bank-recall command, reusing the daemon's own dispatch functions (moved into core/_query_dispatch.py) so the two paths can't drift apart. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
LifecycleLock.acquire() did read -> check-for-live-holder -> write, with no atomicity. Two daemons launched in the same instant both read "no live lock", both wrote, and both passed acquire() without raising LifecycleLockConflict. The singleton guard in daemon.main() that is meant to make the loser exit therefore never fired, so one daemon kept running as a store-contending orphan. This reproduces reliably on Windows, where two independent launchers fire together: the `iai-mcp-daemon` scheduled task (repo .venv interpreter) and the wrapper/doctor respawn path (doctor._respawn_daemon uses sys.executable, resolving to a global Python). Because the Windows daemon binds an ephemeral port (start_server on 127.0.0.1:0), there is no bind collision to catch the duplicate either -- both bind, and .locked/.daemon.port/.daemon.token point at whichever wrote last. A bare O_CREAT|O_EXCL create is not sufficient: reclaiming a *stale* dead-pid lock still needs check-then-remove, and there is no atomic "remove this file only if it is dead" primitive -- under contention a racer can evict a lock a peer just created and briefly reopen the window (an earlier O_EXCL-plus-eviction attempt failed a 12-thread stress test ~10% of runs). Serialise the whole read-check-write behind an OS advisory lock (fcntl.flock on POSIX, msvcrt LockFile on Windows) on a sidecar `.guard` file. Only one acquirer across all processes runs the critical section at a time, which makes the simple read-check-write correct; the guard is held by the open handle and released automatically if the holder dies, so it cannot wedge. Waiting on the guard is bounded (_GUARD_TIMEOUT_SEC) so a stuck peer surfaces as a TimeoutError rather than an indefinite hang. All existing lock tests preserved (stale recovery, corrupt overwrite, cross-host, mode-0600, live-holder conflict). Adds a 12-thread concurrent acquire() stress test asserting exactly one winner (20/20 runs green; the pre-fix path fails it), plus a guard release/re-acquire test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
downgrade_to_shared() called a *blocking* flock(base_fd, LOCK_SH) -- the only lock site in _db.py without LOCK_NB. On Windows _filelock.py services LOCK_SH as an exclusive msvcrt byte-range lock with no atomic EX->SH conversion (documented in its own module docstring), so a blocking LOCK_SH on a fd that already holds LOCK_EX polls LK_NBLCK forever against the daemon's own range. That loop runs under _PROCESS_LOCKS_GUARD, stalling every thread until the liveness watchdog force-kills the daemon -- the recurring pre_kill_forensic_dump at _db.py downgrade_to_shared. The daemon downgrades on its first WAKE tick, so this wedged the daemon whenever it genuinely held EXCLUSIVE then (intermittent: no-ops out early when the _PROCESS_LOCKS entry is already absent). Fix, per the "conversion-free protocol" the shim docstring prescribes: - downgrade_to_shared: use LOCK_SH | LOCK_NB and treat EAGAIN/EWOULDBLOCK as success. On Windows SH==EX so the fd already owns the lock (relabel only); on POSIX an EX->SH downgrade never blocks, so behavior is unchanged. - escalate_to_exclusive: same self-conflict (SH->EX on an already-held fd) was bounded by the 4s deadline so it degraded to HippoLockHeldError rather than wedging, silently running sleep consolidation without the EXCLUSIVE label on Windows. Expose SHARED_IS_EXCLUSIVE from _filelock (True on Windows, False on POSIX) and short-circuit to relabel when this fd already holds the lock. POSIX shared locks stay genuinely contended. Verified with a timed before/after: old blocking path still wedged after 3s; patched full cycle (EXCLUSIVE -> downgrade -> escalate -> downgrade) completes in 0ms with correct access-mode transitions. Existing lock/concurrency/consolidation tests unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Guards the fix in 1a2b528. Runs the exact daemon sequence (EXCLUSIVE -> downgrade -> escalate -> downgrade) with every lock call -- including close() teardown -- bounded by a watchdog-thread join timeout, so a reintroduced blocking flock FAILS the assertion within the timeout instead of hanging the suite (a wedged thread holds _PROCESS_LOCKS_GUARD, which would otherwise deadlock an un-bounded close()). Crucially these tests do NOT skip on Windows -- unlike test_hippo_concurrency.py, which is skipif(Windows) and therefore never exercised the platform where this wedge lived. That skip is exactly why the bug went uncaught. Verified: passes on the fix; on the pre-fix blocking-flock code the downgrade test fails with "the blocking-flock wedge is back" (thread never returns). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The liveness watchdog runs in a background daemon thread (name="iai-liveness-watchdog"). When it decides to kill a wedged daemon, _self_kill() branches on hasattr(signal, "SIGKILL"): POSIX sends SIGKILL to its own pid (kills the whole process), but the Windows fallback called sys.exit(1) -- which only raises SystemExit in the calling thread. From a background thread that terminates the thread, not the process, so the wedged daemon kept running. Task Scheduler's IgnoreNew instance policy then treated the task as still "Running" and silently refused to start a replacement, so the daemon stayed down until a manual taskkill. Use os._exit(1) on the no-SIGKILL path: it terminates the whole process immediately from any thread and bypasses interpreter shutdown, which could otherwise hang on the same wedge the watchdog is trying to escape. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The existing _self_kill tests are all gated on @_REQUIRES_SIGKILL, so they skip on Windows -- which is exactly how the no-op sys.exit path shipped uncaught. The new test simulates the no-SIGKILL branch (so it runs on every platform) and asserts _self_kill calls os._exit, never sys.exit. Verified it fails against the pre-fix source. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`iai_mcp daemon stop` on Windows could leave a child interpreter alive and still holding the hippo lock. The stop path sent os.kill(pid, SIGINT) -- which on Windows is not a signal but a TerminateProcess against the parent process only. That orphans any children the daemon spawned, and because the parent then exits immediately the subsequent poll loop returned before the `taskkill /F /PID` escalation (itself childless) ever ran. Replace the SIGINT-then-escalate dance with a single `taskkill /F /T /PID`, which terminates the daemon together with its child processes as one tree while that tree is still intact. schtasks /End still runs first. POSIX (macOS/Linux) stop paths are unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Windows stop path had no coverage (existing stop tests are macOS/Linux or @skipif-SIGKILL). The new test drives cmd_daemon_stop with platform=Windows and asserts it issues taskkill /F /T /PID (tree-kill) after schtasks /End and never falls back to os.kill, which orphans the child tree. Verified it fails against the pre-fix source. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lation consume_wake_signal() did an unlink-to-consume inside a bare `except OSError: return False`. On Windows, unlink() raises PermissionError (sharing violation, WinError 5/32) when a concurrent writer or reader momentarily holds the signal file open -- Python's open() there does not request FILE_SHARE_DELETE. The bare handler swallowed that as "no signal", dropping a real wake and stranding the daemon in HIBERNATION with an unserved socket. Retry the unlink briefly on a Windows PermissionError (mirrors daemon_state._atomic_replace); on POSIX a PermissionError is a genuine permission fault, so it still returns False on the first failure. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Deterministic regression tests for the retry: a simulated Windows transient PermissionError is retried and consumed; a POSIX PermissionError is not retried. Both run on every platform via monkeypatch, so the fix is gated tightly rather than through the racy concurrency test. - De-flake test_consume_atomic_no_race: writers now produce continuously and the consumer polls until it consumes (bounded by a deadline) instead of a fixed iteration count. The old fixed-count consumer could finish before any writer's first replace landed (adverse scheduling on a loaded runner), reading consumed=0 -- the macOS CI failure `assert 0 >= 1`. - The writer harness retries os.replace on a Windows PermissionError, mirroring production so the writer thread can't die early and starve the consumer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…pipeline curiosity_pending was structurally always empty. v1.0.0 moved curiosity off the recall hot-path to a deferred model (pipeline writes deferred_curiosity_input events) but never added the sleep-side consumer, so fire_curiosity — the only writer of curiosity_question events that curiosity_pending reads — had no production caller. deferred_curiosity_input events were written and never read. Add a CURIOSITY_DRAIN sleep step that flushes the event buffer, reads unprocessed deferred_curiosity_input events since a watermark, recomputes recall entropy, and calls fire_curiosity — watermarked for idempotency (never re-fires, advances to max ts seen so mid-run writes aren't lost). Producer now also persists scores + turn in the deferred event, which the drain needs for compute_entropy and per-session cooldown. Placed immediately before RECALL_INDEX_REBUILD so the curiosity_bridge edges it writes are captured by the same cycle's rebuild, preserving the tested invariant that RECALL_INDEX_REBUILD is the final step. Tests: new test_curiosity_drain.py proves producer->drain->curiosity_pending end-to-end plus watermark idempotency; updated sleep-pipeline/overhaul/CLI tests for the 14th step. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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
Type of change
Affected areas
Testing
pytestpasses locallyruff check src/ tests/cleanBenchmarks
If this PR touches retrieval, capture, or consolidation, include before/after numbers from the relevant
bench.*harness.python -m bench.___Notes for reviewers