Skip to content

fix: make the generated CLI client's session walk work on Windows - #232

Merged
timesler merged 1 commit into
atlassian-labs:mainfrom
fenxcustom:feat/windows-process-tree-walk
Jun 12, 2026
Merged

fix: make the generated CLI client's session walk work on Windows#232
timesler merged 1 commit into
atlassian-labs:mainfrom
fenxcustom:feat/windows-process-tree-walk

Conversation

@mikeasick-fennec

Copy link
Copy Markdown
Contributor

What was broken

The generated CLI client resolves the calling session's bridge by walking the process
tree. That walk worked on Linux, macOS, and other Unix -- it read each parent PID from
/proc, then ps. Neither exists on Windows, so the walk saw only the immediate parent
(cmd.exe, never in BRIDGES) and fell back to any live bridge. Two sessions sharing a
CLI name could misroute on Windows.

What this changes

  • windows_parent_pid(): reads the parent PID from a CreateToolhelp32Snapshot via
    ctypes -- standard library, no new dependency.
  • ancestors() now goes through a shared parent_pid() dispatcher that selects by
    os.name between two per-OS leaves -- unix_parent_pid (the existing /proc then
    ps lookup) and the new windows_parent_pid. Dispatching by os.name also keeps a
    stray ps on PATH (e.g. Git for Windows) from short-circuiting the native lookup.
    The walk itself stays platform-neutral.
  • A cross-platform e2e test: a live decoy bridge keyed first under a non-ancestor PID,
    the real bridge under the test process's PID; the client must pick the real one or it
    falls back to the decoy and fails. Runs the .cmd on Windows and the sh script on
    Unix.

Checked

  • New e2e test passes locally on Windows, exercising the real .cmd -> python ->
    ctypes walk; CI exercises the sh -> /proc/ps path.
  • rustfmt and clippy clean on the change.

Follows #231 (the shared-program Windows .cmd client) -- the process-tree walk was the
remaining place multi-session bridge selection was still Unix-only.

🤖 Generated with Claude Code

The generated CLI client selects the calling session's bridge by walking up
the process tree (find_bridge over ancestors). The walk read each parent PID
from /proc then ps -- neither exists on Windows, so it saw only the immediate
parent (cmd.exe, never in BRIDGES) and fell back to "any live bridge." Two
sessions sharing a CLI name could route to the wrong one.

Keep the walk (ancestors) platform-neutral and resolve each parent PID through
one shared dispatcher, parent_pid(), which by os.name calls a per-OS leaf:
unix_parent_pid() (the existing /proc then ps lookup) or the new
windows_parent_pid(), which reads th32ParentProcessID from a
CreateToolhelp32Snapshot via ctypes (standard library, no new dependency).
Dispatching by os.name keeps a stray ps on PATH (e.g. Git for Windows) from
short-circuiting the native lookup.

Add a cross-platform e2e test: a live decoy bridge keyed first under a
non-ancestor PID, the real bridge under the test process's PID; the client
must select the real one. A regressed walk falls back to the decoy and fails.
Runs the .cmd on Windows (exercising the ctypes walk) and the sh script on Unix.

Testing strategy review: integration test at the correct layer for process-tree
routing; the two-live-bridge design makes the assertion a true regression catch
(a broken walk returns wrong-session, not correct-session). The take(8)
connection cap and the missing TestBridge Drop are pre-existing harness
patterns, not introduced by this test.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@atlassian-cla-bot

Copy link
Copy Markdown

Thank you for your submission! Like many open source projects, we ask that you sign our CLA (Contributor License Agreement) before we can accept your contribution.
If your email is listed below, please ensure that you sign the CLA with the same email address.

The following users still need to sign our CLA:
❌mikeasick-fennec

Already signed the CLA? To re-check, try refreshing the page.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 fixes multi-session bridge disambiguation for the generated CLI client on Windows by making the process-tree walk work without relying on Unix-only facilities (/proc, ps). It introduces a Windows-specific parent-PID lookup and adds an e2e test to ensure the client selects the correct session bridge even when a live decoy exists.

Changes:

  • Add a Windows ctypes implementation to retrieve a process’s parent PID via Toolhelp snapshots.
  • Refactor the Python client’s ancestor walk to use a shared parent_pid() dispatcher (Windows vs Unix leaf implementations).
  • Add an end-to-end test that fails if the client falls back to a live decoy bridge instead of selecting the ancestor-session bridge.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
crates/mcp-compressor-core/src/client_gen/cli.rs Adds Windows parent-PID lookup and refactors ancestor walk to be platform-neutral with OS-specific leaf functions.
crates/mcp-compressor-core/tests/generated_clients.rs Adds an e2e regression test to verify correct bridge selection in multi-session scenarios (including Windows .cmd execution path).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread crates/mcp-compressor-core/src/client_gen/cli.rs
@timesler
timesler merged commit a46dc69 into atlassian-labs:main Jun 12, 2026
12 of 13 checks 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.

3 participants