Repository navigation
prod-code lsp gives editors rust-analyzer, gopls and clangd themselves on the node; Zed extension - #337
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c95a930eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let from = |config: GenericLspConfig| ServerCommand { | ||
| program: config.command, | ||
| args: config.args, | ||
| env: config.env.into_iter().collect(), | ||
| }; |
There was a problem hiding this comment.
Preserve the TypeScript server initialization options
When the node selects typescript-language-server and the checkout does not contain its own node_modules/typescript, GenericLspConfig::for_typescript() supplies the global tsserver.path needed by that server, but this conversion discards initialization_options. The dedicated editor process therefore starts successfully but cannot locate TypeScript unless the editor happens to provide a node-valid path itself; carry these defaults into the forwarded initialize request and merge them with the editor's options.
Useful? React with 👍 / 👎.
| match pushed { | ||
| Ok(()) => prod_code_mcp::watch::mark_synced(&cwd, generation), | ||
| Err(err) => { | ||
| tracing::debug!(error = %format!("{err:#}"), "sync before a save failed") | ||
| } | ||
| } |
There was a problem hiding this comment.
Stop forwarding saves when their sync fails
If the separate sync connection transiently fails while the main LSP session remains alive, this branch only logs the error and immediately forwards textDocument/didSave. Rust-analyzer then runs its check-on-save against the previous on-disk contents even though the editor has just saved newer text, producing stale or misleading diagnostics; retry the sync or surface/terminate the session instead of delivering the save before its contents reach the node.
Useful? React with 👍 / 👎.
| let mirror = cache.join(remote.to_string().replace(':', "_")); | ||
| Self { | ||
| remote, | ||
| mirror_uri: format!("file://{}", mirror.display()), | ||
| mirror, |
There was a problem hiding this comment.
Encode the mirror root before matching file URIs
When HOME or XDG_CACHE_HOME contains a space, #, or another URL-escaped character, copy() returns a percent-encoded URI from Url::from_file_path, while mirror_uri and the early raw.contains(mirror) check use the unencoded filesystem string. Messages about an opened mirrored definition then bypass to_node, so the remote language server receives the client's cache path instead of the original node path and requests such as hover or references fail; construct and match the mirror prefix as a proper file URL.
Useful? React with 👍 / 👎.
|
Live output after the deploy (gateway a70c8bd on a Linux build node; checked again on v0.3.9). A scripted editor session drives The home directories and the node's address in the mirror path are replaced by |
Problem
prod-code lspserved an editor from prod-code's own engines, not from the server the editor would run locally (#332). For Rust that was the in-memory engine; for the other languages, a server the gateway had initialised for itself. The editor's settings never reached the server, and neither did the server's requests. Nothing rancargo checkon save, and rust-analyzer's protocol extensions were refused. Run with a scripted editor session on a one-file crate that movesvtwice (rustc's E0382), prod-code 0.3.8 with #320 deployed:Two more gaps stood in the way of an editor:
Change
editor_proxy. A session whose handshake sayspurpose: "editor"gets a language server process of its own, started in the node's copy of the checkout.for_cpp), basedpyright, the TypeScript server or sourcekit-lsp.processIdininitialize, which is dropped. That id names a process on the editor's machine, and basedpyright exited 4 s after it found no such process on the node (measured below; rust-analyzer, gopls and clangd kept running).workspace/didChangeWatchedFiles, registryEditorServers).PROD_CODE_EDITOR_SERVERS=off, the session is served by the shared engines as before.prod-code lsp.--language <lang>asks for one language's server, and places a language other than the root's under a key of its own, e.g. Swift on a macOS node.textDocument/didSaveandworkspace/didChangeWatchedFileswait for a push of the checkout, so that check on save checks what was saved. The push and the background syncs take turns.editor_files::RemoteFilescopies a file the server names that the editor cannot open into a read-only mirror under the user's cache directory, and names the copy. The editor's messages about a copy name the node's path again. Toolchain and registry files are fetched once; others are fetched each time.PROD_CODE_LSP_TRACE=<file>logs every message.editors/zed. A Zed extension (zed_extension_api0.7.0, builds to a 304 KB wasm32-wasip2 component). It has one server per language (prod-code-rust,-go,-cpp,-python,-typescript,-swift), each runningprod-code lsp --language <lang>. The user's settings for the local server a prod-code server stands in for (lsp.rust-analyzer.*and so on) are passed on. Its README also shows how to point Zed's own rust-analyzer atprod-code lsp, which keeps every Rust feature Zed wires to rust-analyzer by name.rustup component add rust-analyzer).Reproduce and verify
The live tests run a real gateway and real servers on a Linux build node:
In the CLI test
lsp_asks_for_its_language_pushes_a_save_first_and_mirrors_node_files:engine=go;initializeand thedidSaveit precedes;/srv/toolchains/std/vec.rsreaches the editor asfile://<cache>/prod-code/remote/<node>/srv/toolchains/std/vec.rs, read-only, with the node's content.Each new test fails without its piece:
processIdpassed through,an_editors_python_server_does_not_watch_the_editors_processfails after 8.75 s: the session ended.didSaveremoved, the CLI test fails. Its log shows no sync betweeninitializeanddidSave.Which servers exit when
processIdnames no process on the node (initializewithprocessId: 2147483000, 20 s):Checks (Linux build node)
cargo fmt --all -- --check: fmt=0 (aftercargo fmt --allon the node)cargo clippy -p prod-code-gateway -p prod-code-client --all-targets -- -D warnings: clippy=0cargo llvm-cov -p prod-code-gateway -p prod-code-client: passed=177 failed=0editor_proxy.rs89.60% (new)editor_files.rs96.54% (new)lib.rs84.48%workspace.rs81.97%main.rs80.91%editors/zed:cargo build --release --target wasm32-wasip2ok;cargo clippy --target wasm32-wasip2 -- -D warningsclean;cargo fmt --checkclean.zed is already running, whatever--user-data-dirsays). The check in Zed follows the deploy, in a project that selects the servers in its.zed/settings.json.Closes #332
Closes #333
Closes #334