Repository navigation
Make Windows a supported platform - #11
Merged
Merged
Conversation
A signal cannot cross processes on every platform this harness supports. Windows has no process-to-process SIGTERM or SIGINT; Node implements `process.kill` there as TerminateProcess, so the target dies where it stands, its `finally` never runs, and the eval claim is stranded. Polling a file behaves identically everywhere. This sentinel is deliberately sticky. An unattended agent loop starts a fresh eval seconds after the last one exits, so a marker consumed by the eval it aborts would be gone before the loop's next iteration read it. Only clearStop removes it. Nothing reads the marker yet. Later tasks make eval poll it and make `stop --force` write it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FV5Usy3xLiz4ngEp6zPC5L
The previous commit added stop.force but nothing read it. eval already has an AbortController fed by SIGINT/SIGTERM; that controller is now also fed by a 500ms poll of forceRequested, installed at the same spot as the signal handlers and for the same reason — a marker already pending when a run starts must abort it at its first checkpoint, not after it has claimed the run and begun measuring. The poller is unref'd so it never keeps the process alive on its own, and its interval is torn down on every exit path alongside the signal handlers, including the early return when claimEval itself throws. This makes the abort path identical on every platform: Windows never sees SIGINT or SIGTERM cross processes, but a file it can always see. `stop --force` itself still signals rather than writing the marker — that switch is a later task. Because the marker is sticky, an abort that finds it still pending means every later eval on this tag will abort too until someone clears it. The ABORTED verdict now says so and names the cure, rather than reporting the same generic "interrupted" message regardless of cause — folded into the existing `message` field so --json's stdout still carries nothing but the verdict object. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FV5Usy3xLiz4ngEp6zPC5L
The previous two commits built the marker and the poller that reads it; this one finally stops sending a signal. `stop --force` now calls requestForceStop unconditionally, before it even looks at the eval lock, so the marker stands regardless of whether a pid was ever readable. A running eval picks it up at its next 500ms checkpoint and aborts itself through the AbortController Task 2 wired up; nothing is signalled between processes, so the path is identical on Windows and POSIX, and it can no longer reach an unrelated process the OS has recycled a stale pid onto. evalRunning is demoted from mechanism to reporting: its only job now is to say, for the human's benefit, whether an eval happens to be running right now and to name its pid in the message. A corrupt pid file used to be swallowed with "assuming there is nothing to signal" because it once gated the SIGTERM; now that the marker already stands before evalRunning is even called, a corrupt lock costs the report one line and nothing else. The "what this leaves you with" report below is untouched — it still only reports, and dropping an abandoned commit remains the human's call. status gains a way to see this from the outside: `forced` is checked alongside `stopped`, and a pending force renders as its own FORCED line rather than being folded into "requested", because the two markers mean different things to someone reading the terminal — one lets the current experiment finish, the other means it already didn't. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FV5Usy3xLiz4ngEp6zPC5L
Windows CI run 34195066520 failed five tests: two were product defects (now fixed in Tasks 1-3 by wiring up marker-based abortion instead of signals), and three were test defects assuming POSIX conventions that Windows does not share. The path separator `/` is POSIX-only; Windows uses `\`. `stateDir` returns a filesystem path that ends correctly on either platform, but the test asserted a hardcoded `/sep4` in test/state/state.test.js:59. Now it uses the sep export from node:path, just as the code under test does. Line endings vary by platform and repository configuration. The Windows runner checks templates/program.md out with CRLF, turning every `\n` into `\r\n`. The regex in test/cli/init.test.js:117 required a bare `\n` at a paragraph boundary. The new `.gitattributes` pins program.md to LF on all platforms so a checkout never surprises the test, and the regex now accepts both line endings so it stays honest against a CRLF copy that might arrive some other way. On Windows, `chmod` is a no-op — there is no permission model to enforce. The test in test/pipeline.test.js:184 tried to make a directory unlistable while keeping it traversable, a distinction that exists only on POSIX. The test is now wrapped with `it.skipIf(!posix)` and a comment explaining why it cannot run on Windows, matching this repo's established idiom for platform-specific tests (see test/state/state.test.js:191-223). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FV5Usy3xLiz4ngEp6zPC5L
Tasks 1-4 removed the actual reason Windows was unsupported: stop --force now writes a polled marker instead of relying on POSIX signals, and three test assumptions have been fixed. The checkWindows function's warning about interrupted evals not reaching the ABORTED path is no longer honest — measurement works on Windows now that the stop mechanism is reliable. Where a platform lacks a tool or feature, doctor degrades gracefully: one check reports NA rather than the whole report failing. This is the intended design, stated in the check() header comment. checkWindows violated that by warning about the platform itself; now it simply contributes no platform line on Windows, exactly as every platform other than darwin and linux already does. os.loadavg() is not implemented on Windows — it returns a hardcoded [0, 0, 0] to avoid breaking portable code. Reporting that would tell a user under heavy load that their machine is perfectly idle, which is worse than saying nothing in a tool whose whole job is predicting whether measurement will be reliable. checkLoad now returns NA on Windows with clear guidance to check the machine manually, matching the tool's principle of informing, never blocking. The two new tests guard against regression: on Windows (Task 7), they verify both changes work; on macOS/Linux, both pass trivially because the second does not produce a platform finding at all, and the first enters the else branch unchanged. They earn their keep on Windows CI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FV5Usy3xLiz4ngEp6zPC5L
The doc comment above killGroup now explains the platform-specific mechanism that makes grandchildren die even on Windows despite the absence of process groups. On Windows, libuv assigns each non-detached child to a job object with JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, so terminating the direct child closes its job handle and kills everything it spawned. This holds only because every subprocess started by this harness is process.execPath — see the updated comment for the two changes that would silently break it, leaving vitest's workers running and corrupting later measurements. The runner.test.js assertions confirm this on every platform, including Windows CI, by recording each grandchild's pid and asserting it cannot be signaled. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FV5Usy3xLiz4ngEp6zPC5L
Tasks 1-6 removed the only reason Windows was excluded: stop --force now writes a sticky marker that a running eval polls into the same AbortController Ctrl+C uses, instead of sending a signal Windows cannot deliver across processes. Nothing in the abort path is platform-specific anymore, so the matrix should stop pretending otherwise. Adding windows-latest alongside ubuntu-latest and macos-latest is what actually turns "Windows works now" from a claim into something CI checks on every push, which is the whole point of doing this last. The README's Platform support section is rewritten to match: the table reads "supported" on all three rows, and the prose explains the marker mechanism and the handful of genuinely cosmetic differences that remain (doctor's missing load average, the skipped ownership check, one Windows-inapplicable chmod test) rather than warning readers away from the platform doctor no longer warns about itself. The "Three ways to end a run" item for stop --force still described the old SIGTERM behavior, which Task 3 made false — it no longer signals anything. Rewritten to say what actually happens: the marker is written, the running eval polls for it and cancels itself, and that cancellation is what tears down the process tree. The stop --clear sentence below it undersold what the command does now that there are two sentinel files to clear; it read as cancelling one request when clearStop has cleared both since the forced-stop marker was introduced, so it now says so explicitly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FV5Usy3xLiz4ngEp6zPC5L
…r minor ones evalOnce had no abort checkpoint before the gate phase: the scope diff, config hash and freeze.restore all ran unconditionally, and the first read of o.signal.aborted was after runGates returned. Runner.run only ever registers an 'abort' listener, and an AbortSignal that is already aborted by the time a listener is added never dispatches to it — so a gate whose Runner.run() happened to be invoked after the sticky-marker poll had already fired would run to completion (or its own per-child timeout) before the harness ever noticed. cmd-eval.js's own comment claimed a pending marker aborts a run "at its first checkpoint, not after it has begun measuring", which was false because no such checkpoint existed. Added one as the literal first statement of evalOnce, matching the existing post-gate check's idiom. Investigating this closely turned up that runGates()'s own post-gate abort check, combined with typecheck's built-in per-file abort awareness, already protects a plain-JS repo with no ESLint config from the worst of this — the newly added CLI-level test (marker written before eval is spawned) passes on both the fixed and unfixed tree for that reason. A second, deterministic unit test against evalOnce() directly (baseline.commit pointed at a nonexistent commit, so an unchecked signal reaches the real scope diff and fails with a git error instead of the abort message) gives a genuine red/green cycle without depending on timing. state/lock.js's guard against a pid file holding 1 or -1 justified itself by "the exact form stop --force uses for its own valid pids" and its error text promised the pid "is not a process this command will signal". Neither is true after 57139fc: nothing in src/cli/ hands a pid to process.kill any more. The guard itself still matters for a completely different reason — alive() uses process.kill(pid, 0) as a liveness probe, and pid 1 or -1 would make that probe always succeed, permanently stranding the lock via isStale() — so the comment and error text now say that instead, with no change to the guard's logic. The test asserting on the old error string, and its own comment repeating the same stale "signals the pid's process group" claim, are both updated to match. Also: the poll's forceRequested().then() in cmd-eval.js gets a .catch(), matching its sibling call; four leftover comments across the eval and stop tests that still described the pre-Windows signal-based mechanism now describe the marker instead, with each comment's actual point kept intact; the README's stop --force item now says plainly that the marker is sticky and will abort the next eval if nothing was running when it was issued; and a plain `stop` now checks whether a forced stop from an earlier, uncleared `stop --force` is still pending before promising the current experiment will be measured, since that promise is false in exactly that case. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FV5Usy3xLiz4ngEp6zPC5L
Windows is a supported platform as of this release. `stop --force` no longer signals the running eval: it writes a sticky marker that eval polls into the AbortController it already had, so the abort path — ABORTED, exit 2, no results.tsv row, claim released — is identical on Linux, macOS and Windows. Nothing is signalled between processes on any platform, which also closes a race on POSIX where a pid read from a file could have been recycled onto an unrelated process. CI now runs the full suite on windows-latest alongside the other two, on Node 20 and 22. That is what makes the README's support claim a fact the next release has to keep true, rather than a sentence someone wrote once. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FV5Usy3xLiz4ngEp6zPC5L
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.
e4e359edeclared Windows unsupported from evidence, not from reading the code, and named that evidence: CI run 34195066520 on the since-deletedci/windowsbranch. That run is still on GitHub. Its log shows five failures, and two of them are one defect.Both ABORTED tests called
child.kill('SIGINT')— one process asking another to abort. Windows has no such thing; Node implementsprocess.killthere asTerminateProcess, soevaldied where it stood, itsfinallynever ran,claim.release()was never called, and it exitednullinstead of2leaving a stranded claim. The broken feature was thereforestop --force, not Ctrl+C: a console SIGINT on Windows is real and already worked.What changed
stop --forcestops signalling and starts writing a sticky marker file. A runningevalpolls it every 500ms into theAbortControllerit already had, so nothing is signalled between processes on any platform and the stop path is identical everywhere. This also closes a race that existed on POSIX, wherestop --forcesignalled a pid read from a file that the OS may have recycled.The other three failures were defects in the tests, not the product: a hardcoded
/separator, a regex requiring a bare\nagainst a file the Windows runner checks out with CRLF, and achmodfixture Windows ignores entirely.doctorno longer warns about the platform, and no longer reports the fabricated[0, 0, 0]thatos.loadavg()returns on Windows.What did not change
src/runner.js. An earlier draft of the design claimed teardown left vitest's workers alive on Windows — that was wrong, and the same CI run disproves it: all three grandchild tests pass onwindows-latest. libuv assigns every non-detached child to a global job object withKILL_ON_JOB_CLOSE, and every subprocess this harness starts isprocess.execPath, so every intermediate carries one. The only change there is a comment recording why, and what would break it.templates/program.mdis untouched. Its description ofstop --forcewas already platform-neutral and stays true.Why this is a draft
Opened to trigger the Windows CI this PR exists to prove.
e4e359edeliberately kept Windows out of the matrix because "a permanently red run teaches nobody anything" — that reasoning still holds, so a red Windows job means this work is not done, not that the matrix row comes back out.Suite is 477/477 on macOS. Design and plan:
docs/superpowers/specs/2026-09-14-windows-support-design.md.🤖 Generated with Claude Code
https://claude.ai/code/session_01FV5Usy3xLiz4ngEp6zPC5L