Enable Linux/macOS Python CI + 1.9.1 portability fixes - #1
Merged
Merged
Conversation
The Python matrix ran on windows-latest only because workload commands were built with subprocess.list2cmdline (Windows quoting), which the POSIX shell rejects (syntax error near unexpected token). Fixing that and running the suite on Linux surfaced several portability issues, all addressed here. - tests/shellcmd.py: shlex.join on POSIX, list2cmdline on Windows; every test and dev-script command builder routes through it. pytest gains pythonpath=["tests"] so the helper imports from all test dirs. - .github/workflows/ci.yml: python matrix now windows-latest/ubuntu-latest/ macos-latest (macOS limited to one Python version for runner cost); the wheel smoke steps are POSIX-portable (bash arrays instead of GNU find -maxdepth). - job_wait returns the EARLIEST matching signal (terminal event, metric_ge threshold, or return_progress) by event sequence instead of always preferring the terminal event. - codex_pipe: a peer close is reported consistently (closed the connection) on read or write, and close() shuts a socket down before closing so a reader blocked in recv() wakes promptly on POSIX. - _orphaned_mcp_servers: POSIX detection matches only the Vanth MCP entrypoint (the vanth console script or python -m vanth.server), so --reap-orphans can no longer terminate unrelated processes (e.g. pytest inside the checkout). Orphan findings are advisory and no longer flip doctor's exit code. - Test hardening for POSIX: locked direct DB writes in the pid/publication race, stale portable-rename directory expectation, same-size capture mutation, and load-adjusted budgets for the cross-process burst and slow wake adapter. Version 1.9.1. Windows: 783 passed, 6 skipped. Linux (3.12): 787 passed, 2 skipped. go test ./... green.
The first matrix run (ubuntu/macos/windows) passed on Linux and exposed: - Windows: `on_failure` emitted the failure_threshold event before persisting failure_streak, so a waiter could read the policy state without the key. Persist the streak before reacting. - macOS: directory materialization used /dev/fd, which is unreliable for creating nested entries under a directory fd; use the dev/inode-checked plain-path fallback there. - Windows: the stop-after-restart test now waits (bounded) for OS process teardown instead of asserting immediately. - Daemon-start test helpers now surface the daemon's stderr and log tail on failure, to diagnose the remaining macOS daemon-start failures.
- daemon: override server_bind to skip http.server's socket.getfqdn reverse DNS lookup, which blocked daemon startup on the macOS runner (daemon.json never written, /health never served, MCP clients timed out). - server: _claim_launch now clears worker_pid, so stale-claim recovery cannot skip an abandoned launch whose previous runner pid is still visible. - tests: relax the relay-activity watchdog idle threshold from 50ms (below macOS scheduling jitter) and make the blocking-relay test tear down via parent liveness instead of relying on the idle timer.
- process_watch: reset the idle timer when activity occurred within the idle threshold, not within the (tiny) sampling interval. A relay whose notify cadence is coarser than the sampler was mis-reaped on macOS runners. - tests: stop the maintenance loop before asserting the abandoned-claim recovery outcome, so the restart policy cannot re-claim and relaunch the job mid-assertion (a fast-runner race).
- server: extend _emit's lock-contention retry budget (10 attempts, capped backoff) so reader-parsed AGENT_EVENTs are not dropped under a concurrent job burst on Windows. - tests: accept running-or-completed for the status snapshot taken when a job_wait returns (a short job can finish first on a loaded runner).
The quick-job retry delivery test waited 5s for a stop/retry/deliver sequence (retry_delay_seconds=1, 0.2s dispatcher poll). Under the Windows runner's full suite load that occasionally exceeded 5s; use a 20s ceiling (it is a max wait, so passing runs are unaffected).
The on_failure watcher compared only the latest 'failed' event to the stored one and incremented the streak by one. With a fast restart (backoff 0) two failures can land between watcher ticks, undercounting the streak (Windows CI saw a final streak of 2 instead of 3). Count all failed events after the last counted one instead.
The redirect handler replied 302 without reading the request body; closing the socket with unread data makes the OS send an RST, which surfaced as WinError 10053 before the client could read the 302, masking the redirect-refusal path on Windows. Read Content-Length bytes first.
The test inserts a synthetic 'running' row with no live pid; the background reconciler could orphan it between the guarded publish/stop UPDATEs, so a guarded write saw a non-running row and returned False (Windows CI flake). Stop the maintenance loop before the interleavings.
The recovery can be won by a concurrent dispatcher pass and the orphaned event write can land a tick later under load; assert on a bounded poll instead of an immediate read (Windows CI saw the status recovered but the event not yet visible).
A POST /jobs that spawns a runner can exceed 5s on a loaded Windows runner; use 20s (connection-refused is immediate, so the health-poll deadline still holds).
Spawning 10 adapter subprocesses CPU-starves a 2-core Windows runner, so job completion took >4s even without waiting on the adapters. The real regression (completion waiting on the 5s adapters) needs ~15s and trips the 10s waiter, so bound just under it.
Independent review of the POSIX-CI/1.9.1 changes surfaced several issues: - job_wait: metric candidates now honour since_event_id (a satisfied threshold at/before the cursor no longer starves every later signal) and every threshold is considered instead of breaking on dictionary order. - Orphaned-MCP detection: reject false positives (python unrelated.py -m vanth.server, bash -lc ..., vanth CLI subcommands) since the reaper kills each match; Windows uses Get-CimInstance command lines (the WMIC CSV parse misread alphabetic columns and could not establish identity). - on_failure: count failed executions over a bounded seq interval (no under/over-count), and mark the reaction complete only after it succeeds so a crash/error retries it. - macOS materialization: re-verify parent + staging descriptors before publication and fail closed on an ancestor swap. - Idle reaper: measure from last activity so the timeout is idle, not ~2x. - Tests: the blocking-relay test now blocks longer than the idle threshold; the delivery-retry test asserts the retrying state was actually observed.
…, stdin activity Follow-up review of the review fixes surfaced three regressions and two gaps: - on_failure: policy state written by 1.9.0 (last_failure_event_id, no seq watermark) was recounted from scratch on upgrade, double-counting failures. The legacy marker is now resolved to its job-local sequence before counting. - Idle reaper: a one-shot stdin traffic sample was only remembered for the iteration that observed it, so a process could be reaped ~two sampling intervals after traffic instead of one idle timeout later. The stdin timestamp is now retained across iterations. - MCP matcher: refuses ambiguous forms instead of guessing — quoted subcommands (vanth "logs" --follow), attached -c payloads, unknown interpreter options, and long options all reject; a quoted argv0 (Windows "C:\Program Files\...python.exe") is split off so genuine launches with spaces in the interpreter path still match. Missing an exotic launch is the safe failure mode for a reaper that kills what it accepts. - job_wait metric_ge: the threshold value and the returned event are read in one joined query, so a stale satisfying value can no longer be paired with a newer event that no longer satisfies the threshold. - macOS materialization: comment/CHANGELOG now state the residual race (transient swap restored before the check, staging-name replacement under an unchanged parent) instead of claiming full fail-closed coverage. - Matcher tests extended with the lookalike cases; reaction retry documented as at-least-once with the upgrade path noted.
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
Enables the Python test matrix on
ubuntu-latestandmacos-latestalongside Windows, and fixes the portability issues that running the suite on POSIX surfaced. Bumps to 1.9.1.The Python job previously ran on
windows-latestonly because workload command strings were built withsubprocess.list2cmdline(Windows quoting), whichbashrejects (syntax error near unexpected token).Changes
tests/shellcmd.py(shlex.joinon POSIX,list2cmdlineon Windows); all test/dev-script builders route through it.pytestgainspythonpath = ["tests"].windows-latest,ubuntu-latest,macos-latest(macOS on one Python version for cost); POSIX-portable wheel smoke steps (bash arrays instead of GNUfind -maxdepth).job_waitordering — returns the earliest matching signal (terminal event,metric_gethreshold, orreturn_progress) by event sequence instead of always preferring the terminal event.closed the connection) on read or write;close()shuts a socket down before closing so a blockedrecv()wakes promptly on POSIX.vanthconsole script orpython -m vanth.server), sovanth doctor --reap-orphanscan no longer kill unrelated processes (e.g. apytestrun inside the checkout). Orphan findings are advisory and no longer flipdoctor's exit code.Validation
go test ./...greenmacOS is unverified locally and is validated by this PR's CI run.