Skip to content

fix: remove stdout stream handling for share #167

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 5, 2025

Conversation

JoJoJoJoJoJoJo
Copy link
Contributor

remove fcntl and select for windows compatible

@JoJoJoJoJoJoJo JoJoJoJoJoJoJo force-pushed the jonathan/fix-select-read-stderr-error branch from cc18d8d to 5c4e6ce Compare June 5, 2025 06:50
@JoJoJoJoJoJoJo JoJoJoJoJoJoJo marked this pull request as ready for review June 5, 2025 06:57
Copy link
Contributor

@GabrielDrapor GabrielDrapor left a comment

Choose a reason for hiding this comment

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

LGTM

@JoJoJoJoJoJoJo JoJoJoJoJoJoJo requested a review from Copilot June 5, 2025 07:42
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes POSIX-specific non-blocking stream handling (fcntl/select) to improve Windows compatibility in the share command.

  • Deleted make_non_blocking and related tests.
  • Simplified loops to read only stderr and removed select calls.
  • Adjusted error message dispatch in the router to send raw errors.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
tests/test_share.py Removed make_non_blocking test and its import
src/mcpm/router/transport.py Changed await writer.send(SessionMessage(message=err)) to raw send
src/mcpm/commands/share.py Dropped fcntl/select usage, removed non-blocking setup, and simplified I/O loops
Comments suppressed due to low confidence (1)

src/mcpm/router/transport.py:223

  • The router protocol likely expects a SessionMessage object. Sending the raw err may break the consumer; restore or replace the wrapper so the message format matches the API contract.
await writer.send(err)

@JoJoJoJoJoJoJo JoJoJoJoJoJoJo enabled auto-merge (squash) June 5, 2025 07:48
@JoJoJoJoJoJoJo JoJoJoJoJoJoJo merged commit 11fddcc into main Jun 5, 2025
6 checks passed
@JoJoJoJoJoJoJo JoJoJoJoJoJoJo deleted the jonathan/fix-select-read-stderr-error branch June 5, 2025 07:48
mcpm-semantic-release bot pushed a commit that referenced this pull request Jun 5, 2025
## [1.13.3](v1.13.2...v1.13.3) (2025-06-05)

### Bug Fixes

* remove stdout stream handling for share ([#167](#167)) ([11fddcc](11fddcc))
@mcpm-semantic-release
Copy link

🎉 This PR is included in version 1.13.3 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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.

3 participants