fix(hooks): gate coordinator IPC behind cfg(unix) so Windows builds#426
Merged
fix(hooks): gate coordinator IPC behind cfg(unix) so Windows builds#426
Conversation
The background hook coordinator added in #411 used Unix-only APIs (`std::os::unix::net::{UnixStream, UnixListener}`, `libc::kill`, `libc::SIGTERM`, `libc::pid_t`) without conditional compilation. The v1.10.0 release workflow's `build-local-artifacts (x86_64-pc-windows-msvc)` job consequently failed with 16 errors in `src/coordinator/{client,process}.rs`. Gate all Unix-only socket and `libc` code in `client.rs` and `process.rs` behind `#[cfg(unix)]` (including their test modules), and provide a `#[cfg(not(unix))]` `CoordinatorClient` stub whose `connect()` always returns `Ok(None)`. Existing call sites in `commands/hooks/jobs.rs` and `core/worktree/prune.rs` already handle the `None` case as "no coordinator running", and the only invokers of `fork_coordinator` are already gated, so the public API stays compatible. To prevent this class of regression from slipping past PR review again, add a `windows-check` job to `test.yml` that runs `cargo check --all-targets` on `windows-latest`. This catches Windows compile breaks on every PR rather than only at release tag time. Fixes #424
The new `windows-check` PR job (cargo check --all-targets) caught an un-gated `use std::os::unix::fs::PermissionsExt;` at the top of `src/hooks/executor.rs`'s test module. Its only consumer is already inside a `#[cfg(unix)]` block, so the import just needs the same gate. Pre-existing: this didn't break the release workflow because dist's build doesn't compile the lib-test target — only the `--all-targets` check exercises the test modules on Windows.
This was referenced Apr 28, 2026
Merged
Merged
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
libccode insrc/coordinator/{client,process}.rs(and theirmod tests) behind#[cfg(unix)], with a#[cfg(not(unix))]CoordinatorClientstub whoseconnect()returnsOk(None).windows-checkjob to.github/workflows/test.ymlthat runscargo check --all-targetsonwindows-latest, so this class of break is caught on every PR instead of only at release tag time.Fixes #424.
Why
The v1.10.0 release (run #25074742437) failed the
build-local-artifacts (x86_64-pc-windows-msvc)job with 16 errors in the new background-hook coordinator (#411): unconditional uses ofstd::os::unix::net::{UnixStream, UnixListener},libc::kill,libc::SIGTERM,libc::pid_t. Thelibccrate is already a[target.'cfg(unix)'.dependencies], so any reference outside a#[cfg(unix)]scope fails on Windows.The Unix-only
fork_coordinatorwas already#[cfg(unix)]and both invokers (hooks/yaml_executor,commands/hooks/jobs) already branch on#[cfg(unix)]. The remaining IPC helpers (start_socket_listener,handle_client_connection,cancel_single_job,send_response,build_job_list,write_pid_file) are only reached from those gated entry points, so gating them is safe and keeps the public surface unchanged.CoordinatorClientcallers (commands/hooks/jobs.rs,core/worktree/prune.rs) already treatOk(None)as "no coordinator running", which is the truthful state on Windows.Why this slipped past CI
PR CI (
test.yml) ran only on Ubuntu and macOS; the Windows target was built only by the release workflow on tag push. The newwindows-checkjob closes that gap with a fastcargo check(no full build, no tests) so any future un-gatedstd::os::unix::*orlibc::*reference fails the PR rather than the release.Verification
mise run fmt:check✓mise run clippy✓ (zero warnings)mise run test:unit -- coordinator::— all 80 coordinator tests pass on macOSstd::os::unix::/libc::hit insrc/coordinator/is now inside a#[cfg(unix)](or#[cfg(all(test, unix))]) scope.x86_64-pc-windows-msvclocally (aws-lc-sysneedswindows.h); the newwindows-checkCI job is the authoritative validation.Test plan
windows-checkjob onwindows-latest.build-local-artifacts (x86_64-pc-windows-msvc)succeeds end-to-end.🤖 Generated with Claude Code