Skip to content

createBrokerEndpoint: honor injected platform in the Unix branch - #595

Closed
zebbern wants to merge 1 commit into
openai:mainfrom
zebbern:fix-broker-endpoint-posix-path
Closed

createBrokerEndpoint: honor injected platform in the Unix branch#595
zebbern wants to merge 1 commit into
openai:mainfrom
zebbern:fix-broker-endpoint-posix-path

Conversation

@zebbern

@zebbern zebbern commented Aug 5, 2026

Copy link
Copy Markdown

The win32 branch already uses path.win32 explicitly, but the Unix branch
built the socket path with the host path module, so calling
createBrokerEndpoint(dir, "linux") on a Windows host produced
"unix:\tmp\...\broker.sock". Use path.posix.join to match the injected
platform. No behavior change on POSIX hosts, where path.join and
path.posix.join are identical.

Flips "createBrokerEndpoint uses Unix sockets on non-Windows platforms"
from failing to passing on Windows hosts (CI only runs ubuntu-latest, so
this was never visible there).

Copilot AI left a comment

Copy link
Copy Markdown

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 createBrokerEndpoint() so that, in the Unix-socket branch, it respects the injected platform argument rather than inheriting path separator behavior from the host OS, improving correctness when platform is overridden (e.g., in tests or cross-platform tooling).

Changes:

  • Use path.posix.join(...) for Unix socket endpoints to ensure POSIX separators regardless of host platform.
  • Add an inline comment documenting why path.posix is required (mirroring the existing explicit path.win32 usage).

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

@zebbern

zebbern commented Aug 5, 2026

Copy link
Copy Markdown
Author

Withdrawing this PR.

@zebbern zebbern closed this Aug 5, 2026
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