fix: Windows breakage after #16 — sys.path shadow and Microsoft OpenSSH multiplex#19
Merged
Conversation
The Microsoft OpenSSH port that ships with Windows 10/11 has flaky
ControlMaster support: the master commonly drops mid-session with
"Connection reset by peer" and the stale socket file then blocks
subsequent multiplex attempts. Reported user output:
mux_client_request_session: read from master failed:
Connection reset by peer
ControlSocket C:\Users\jorma/.ssh/cm\<hash> already exists,
disabling multiplexing
Skip the ControlMaster/ControlPath/ControlPersist `-o` flags entirely
when running on Windows. Keepalives are still added (those work fine),
parallel_load + the fetch semaphore continue to bound concurrency, and
each fetch falls back to its own simple connection — auth happens N
times instead of once, but correctness wins over the perf optimisation.
Windows users who still want multiplexing can configure it in
~/.ssh/config explicitly (e.g. via WSL or a Cygwin ssh); we'll detect
their config via `ssh -G` and respect it.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per review feedback on #18 — mama.utils.system.System already provides static platform checks set from sys.platform at import time, so use System.windows instead of a local _is_windows(). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Microsoft's OpenSSH port for Windows is the actually-buggy client — Cygwin, Git-Bash/MSYS2, and WSL ssh on Windows all have working ControlMaster. Probe `ssh -V` once at startup: only the Microsoft port prints "OpenSSH_for_Windows_<ver>", so we can skip multiplex narrowly for that one case and let everyone else use it. Also addresses review nits: * Trim the inline comment block (it now points at the helper instead of inlining the explanation). * Fix the options_to_add docstring — multiplex is known-broken on the Microsoft client, not "unsupported on this platform". * Add a test for "Windows + user has multiplex configured in ssh config" → we still respect the user's config and don't override it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Drop double-checked locking — racing initialisers both write the same bool, the lock was paranoia. * Drop `or ''` defensive null checks — subprocess.run(text=True) returns strings, not None. * Compress the docstring to 4 lines (matching this file's terse-helper style for sibling helpers like is_multiplex_configured). * Drop the inline call-site comment — the function name says it. 13 lines lighter, same behaviour, all 41 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When git invokes the wrapper as `python /path/to/mama_ssh.py`,
__package__ is empty and the script took a fallback path that did:
sys.path.insert(0, os.path.dirname(_THIS_DIR)) # = .../mama
from utils import ssh_multiplex
That puts the `mama/` package dir at the front of sys.path. Inside
`mama/` lives a `types/` subpackage (mama.types — git/asset/dep_source
etc.), which then shadows Python's stdlib `types`. The next thing
that does `from types import MethodType, GenericAlias` (e.g.
`contextlib`, transitively imported by ssh_multiplex.py) explodes:
ImportError: cannot import name 'MethodType' from 'types'
(...site-packages/mama/types/__init__.py)
This bit Windows users on uv-installed Pythons where `contextlib`
hadn't been pre-cached before our path manipulation ran.
Fix: try the qualified `from mama.utils import ssh_multiplex` first
(works for any pip-installed mama, no path manipulation needed). Only
fall back to sys.path manipulation if that fails, and add the
GRANDPARENT of mama_ssh.py — i.e. the dir that contains `mama/` — so
`import mama.<x>` resolves but `mama/types/` never appears as a
top-level import root.
Adds a regression test that runs the wrapper in a fresh subprocess and
asserts the `mama/` dir does not appear on sys.path.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drops the `_buggy_ssh_cached` global, the `global` declaration, and the inner `is None` branch. Same behaviour, idiomatic stdlib, less code. Test fixture switched from setattr-ing the (now removed) global to calling cache_clear(). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
execute_piped only captured stdout, but `ssh -V` writes its banner to
stderr on most builds. Add an additive merge_stderr=False parameter
that redirects stderr into the captured stdout when set.
Switch multiplex_known_broken() over to execute_piped(merge_stderr=True,
throw=False) instead of building its own subprocess.run + try/except
around (TimeoutExpired, FileNotFoundError, OSError) — execute_piped
returns None on failure, which is what we want for the safe-default
("can't tell, skip mux") branch anyway.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
RedFox20
approved these changes
May 5, 2026
2 tasks
RedFox20
pushed a commit
that referenced
this pull request
May 7, 2026
Banner-based detection in v0.11.29 still leaves multiplex enabled for the very Microsoft OpenSSH it tried to skip — Jorma reports ongoing `mux_client_request_session: read from master failed: Connection reset by peer` and `ControlSocket ... already exists, disabling multiplexing` mid-fetch. Per Jorma's instruction, drop detection and disable multiplex on native Windows entirely. WSL, Cygwin and Git-Bash run as Linux from Python's POV (System.windows is False) so they keep multiplex. Also drops merge_stderr from execute_piped — added in #19 only for the now-removed `ssh -V` banner probe, no other callers. Co-authored-by: Mark Kuckian <mark.cowan@krattworks.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
2 participants
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.
Re-opens #18 (auto-closed when its source fork was deleted; same 5 commits, same code at
18dabd0).Two issues that hit Windows users after #16:
1.
mama_ssh.pyshadows stdlibtypesWhen git invokes the wrapper as
python mama_ssh.py …,__package__is empty and the script took a fallback path:That puts
<...>/mama/at the front ofsys.path. Insidemama/lives atypes/subpackage (mama.types— git/asset/dep_source helpers), which then shadows Python's stdlibtypesfor any subsequent first-time import. The nextfrom types import MethodType, GenericAlias(e.g. insidecontextlib, transitively imported byssh_multiplex.py) explodes:It mostly bit uv-tool-installed Pythons, where
contextlibhadn't been pre-cached before our path manipulation ran. Fix: try the qualifiedfrom mama.utils import ssh_multiplexfirst (works for any pip-installed mama with no path manipulation needed); fall back to inserting the package's parent — nevermama/itself.2. Microsoft OpenSSH for Windows ControlMaster is flaky
Masters drop with "Connection reset by peer" mid-session and the stale socket then blocks reattach:
Detect the buggy client narrowly by
ssh -Vbanner — only the Microsoft port printsOpenSSH_for_Windows_<ver>. When that's the active ssh, skip multiplex; keepalives, parallel fetches, and the concurrency cap stay enabled. Cygwin, Git-Bash/MSYS2, and WSL ssh on Windows report the standardOpenSSH_<ver>p1banner and keep full multiplex. Result is cached per process.User config (
~/.ssh/config) is never overridden in either case.