Skip to content

fix(copilot): add Windows platform guard to _fix_pipe_blocking_mode()#26

Merged
jrob5756 merged 2 commits intomicrosoft:mainfrom
Shazwazza:fix/windows-fcntl-platform-guard
Mar 9, 2026
Merged

fix(copilot): add Windows platform guard to _fix_pipe_blocking_mode()#26
jrob5756 merged 2 commits intomicrosoft:mainfrom
Shazwazza:fix/windows-fcntl-platform-guard

Conversation

@Shazwazza
Copy link
Copy Markdown
Contributor

Summary

_fix_pipe_blocking_mode() (introduced in #23) unconditionally imports cntl, a Unix-only module, causing ModuleNotFoundError: No module named 'fcntl' on Windows. O_NONBLOCK does not exist on Windows and pipes are always blocking, so the entire method is a no-op on that platform.

Fix

Add sys.platform == "win32" early return before the cntl import.

Validation

Cross-validated with GPT-5.4 (independent model review):

  • sys.platform == "win32" is the right check — better targeted than os.name == "nt"; Cygwin/MSYS2/WSL correctly take the Unix path
  • No-op on Windows is safe — subprocess.PIPE is blocking by default on Windows, so skipping won't reintroduce the BlockingIOError this method was designed to prevent
  • Future improvement — os.set_blocking(fd, True) (available cross-platform in Python 3.12+) could replace the cntl approach entirely without platform branching, but that's a larger refactor beyond the scope of this bug fix

Tests

  • est_skips_on_windows — verifies no-op on win32
  • est_runs_on_unix — verifies the method proceeds past the guard on non-Windows

All 35 copilot provider tests pass.

_fix_pipe_blocking_mode() unconditionally imports fcntl, a Unix-only
module, causing ModuleNotFoundError on Windows. O_NONBLOCK does not
exist on Windows and pipes are always blocking, so the method is a
no-op on that platform.

Add sys.platform == 'win32' early return before the fcntl import.

Adds 2 tests:
- test_skips_on_windows: verifies no-op on win32
- test_runs_on_unix: verifies the method proceeds past the guard

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jrob5756 jrob5756 self-requested a review March 7, 2026 13:37
Copy link
Copy Markdown
Collaborator

@jrob5756 jrob5756 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was about to merge then saw the lint issues. Can you take a look at those and we can merge this.

@jrob5756 jrob5756 merged commit 99b09ba into microsoft:main Mar 9, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants