fix(daemon): don't touch the env-derived socket dir when binding to explicit paths - #16
Merged
Merged
Conversation
…xplicit paths bind_to() unconditionally called paths::ensure_socket_dir(), which resolves the global socket directory from PILOTTY_SOCKET_DIR, XDG_RUNTIME_DIR, or HOME. The paths tests mutate those env vars under a mutex that only serializes the paths tests against each other — env vars are process-global, so a daemon test calling bind_to in parallel could read a poisoned value and fail with 'Read-only file system' while creating the directory. Seen intermittently in CI on both platforms (test_client_connects_to_running_daemon). bind_to receives explicit paths and already creates the socket's parent directory; preparing the env-derived default directory belongs in bind(), the production entry point. Moving it removes bind_to's dependency on global environment state entirely. Co-Authored-By: Claude Fable 5 <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
Fixes the intermittent CI failure in
test_client_connects_to_running_daemon(seen on ubuntu in the #14 run and on macos on the v0.0.8 bump commit, run 29156160576):Root cause
bind_to()unconditionally calledpaths::ensure_socket_dir(), which resolves the global socket directory fromPILOTTY_SOCKET_DIR→XDG_RUNTIME_DIR→HOME→ tempdir. The paths tests mutate those exact env vars, serialized by a mutex — but only against each other. Env vars are process-global, so daemon/client tests running in parallel could observe a mid-test poisoned value and try to create a directory on an unwritable path.Fix
bind_to()receives explicit paths and already creates the socket's parent directory two lines later — the env-derived global dir was never its business. Preparing that directory (with 0700) moves tobind(), the production entry point that derives paths from the environment.bind_tonow has zero dependency on global env state, which removes the race structurally rather than tuning test ordering.No production behavior change:
bind()still prepares the default directory before delegating.Verification
Five consecutive full-suite runs green locally; clippy clean under
-D warnings; fmt clean.🤖 Generated with Claude Code