Conversation
ibetitsmike
left a comment
ibetitsmike
left a comment
There was a problem hiding this comment.
Mux working on behalf of Mike.
I found two blocking concurrency issues in the new LSP manager and transport paths. I left both inline below.
src/node/services/lsp/lspManager.ts
Outdated
| const client = await this.clientFactory({ | ||
| descriptor, | ||
| runtime, | ||
| rootPath, | ||
| rootUri, | ||
| }); | ||
| workspaceEntry.clients.set(clientKey, client); |
There was a problem hiding this comment.
Mux working on behalf of Mike.
If two lsp_query calls hit the same workspace and root concurrently, both callers can observe no existing client here, both await clientFactory(), and the later one overwrites the earlier client in workspaceEntry.clients. That leaks an orphaned LSP process outside manager tracking. Please dedupe in-flight creation per (workspaceId, clientKey) or otherwise serialize this path, then add a concurrent query test that asserts the factory only runs once.
| const body = this.encoder.encode(JSON.stringify(message)); | ||
| const header = this.encoder.encode(`Content-Length: ${body.byteLength}\r\n\r\n`); | ||
| await this.stdinWriter.write(header); | ||
| await this.stdinWriter.write(body); |
There was a problem hiding this comment.
Mux working on behalf of Mike.
This method writes one LSP frame as two separate awaited writes, header then body. If two requests or notifications share this transport concurrently, the byte stream can interleave as headerA, headerB, bodyA, bodyB, which breaks Content-Length framing. Please serialize writes, or write each framed message in one critical section, and add a concurrent send test for it.
|
hey @terion-name chatted with the team and I have a good/bad news type of situation. in the meantime - you can definitely finish this implementation in your fork and play around with it. |
2 participants
This is far from final, but I opened WIP PR for feedback and discussion.
Your codebase is complicated and needs deep understanding to make everything properly so I would not be surprised if you say that this approach is incorrect and should be discarded, and yes, this mostly is done by codex.
But if direction seems correct and you are ok with idea — I can continue and move this further.
In short: deep LSP integration gives coding agents superpowers. From diagnosing problems to code path tracing and exploring. I thought it would be great to add this to MUX. Basic design is inspired buy opencode, but adapted to your architecture.