Implement multi-connection architecture#3218
Draft
kinto0 wants to merge 7 commits intofacebook:mainfrom
Draft
Conversation
Summary: Pull Request resolved: facebook#3204 Add the protocol-level types needed for TSP multi-connection support: ConnectionRequest, ConnectionRequestParams, ConnectionRequestResult, ConnectionTransportKind, and TypeServerMultiConnectionCapability. Bump TSP protocol version to 0.5.0. Also add IPC transport plumbing: Connection::ipc(), Connection::from_transport(), MessageReader::Stream, and a --transport CLI arg for the TSP server. These are transport-layer changes independent of the server architecture. Differential Revision: D102005012
Summary: Add IPC transport plumbing using platform-native mechanisms (Unix domain sockets on Unix, named pipes on Windows) with no external dependencies. Includes Connection::ipc(), Connection::from_transport(), MessageReader::Stream, and a --transport CLI arg for the TSP server. We use IPC rather than TCP (localhost) for several reasons: - **Port forwarding interference:** VS Code Remote (dev containers, WSL, SSH tunneling) automatically forwards TCP ports opened on the remote host to the local client machine. VS Code cannot distinguish between ports meant for external access and ports meant only for local inter-process communication, so it forwards them by default. This would require explicitly configuring VS Code to not auto-forward the TSP port, adding unnecessary complexity. - **Dynamic port allocation:** TCP requires finding an unused port at runtime, adding complexity and potential race conditions. - **Simplicity:** IPC (Unix domain sockets / named pipes) avoids both problems entirely. This is also why the VS Code LSP library (vscode-languageserver-node) defaults to IPC for local language servers. Differential Revision: D102190031
Summary: Relevant in future diff Differential Revision: D102217076
Summary: Move `send_ok`, `send_err`, and `validate_snapshot` from `validation.rs` into `server.rs` on `TspConnection` directly, so response dispatch lives next to request handling. Extract `parse_tsp_request` as a free function to decouple JSON-RPC parsing from dispatch — this enables reuse by multiple connection handlers in a follow-up. Also adds `set_experimental` to `ServerCapabilitiesWithTypeHierarchy` so TSP can advertise protocol extensions without exposing the inner `ServerCapabilities`. Differential Revision: D102217075
Summary: see title Differential Revision: D102225859
Summary: this makes the next commit's diff more readable Differential Revision: D102228861
Summary:
Multi-connection architecture for the TSP (Type Server Protocol)
The commit splits the single monolithic TspConnection into a three-tier structure:
1. TspServer<T> — shared core holding the Arc<T> LSP server, the snapshot counter, and a registry of extra connections. All connections share this via Arc.
2. TspMainConnection<T> — wraps a TspConnection (via Deref) and owns the main stdio event loop. Only this type can:
- Process LSP lifecycle events (delegating to the inner server)
- Handle typeServer/connection requests (open/close extra connections)
- Broadcast snapshotChanged notifications to all connections
3. TspExtraConnection<T> — also wraps TspConnection (via Deref). Spawned on demand over IPC named pipes. Can handle TSP query requests (getComputedType, resolveImport, etc.) but explicitly rejects connection management and LSP methods.
Key mechanics:
- Extra connections are opened via a new ConnectionRequest with type: "open" and an IPC pipe name. Each gets its own reader thread + select loop, terminating on a close signal or pipe disconnect.
- snapshotChanged notifications are broadcast from the main connection to all extra connections, so every client stays in sync.
- The server advertises typeServerMultiConnection in its experimental capabilities.
- All TSP request handlers now call self.inner() (method) instead of self.inner (field), since the inner server is accessed through the shared TspServer.
The result: multiple IDE clients can query the same pyrefly process concurrently over IPC without spawning separate server instances.
Differential Revision: D102228875
Contributor
|
@kinto0 has exported this pull request. If you are a Meta employee, you can view the originating Diff in D102228875. |
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
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
1 participant
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.
Summary:
Multi-connection architecture for the TSP (Type Server Protocol)
The commit splits the single monolithic TspConnection into a three-tier structure:
- Process LSP lifecycle events (delegating to the inner server)
- Handle typeServer/connection requests (open/close extra connections)
- Broadcast snapshotChanged notifications to all connections
Key mechanics:
The result: multiple IDE clients can query the same pyrefly process concurrently over IPC without spawning separate server instances.
Differential Revision: D102228875