smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge PR #8572: fix(lsp): gate rust-analyzer/reloadWorkspace behind rust-analyzer check (@roboomp)

Browse Source
This commit is contained in:
can1357
2026-08-16 02:43:33 +02:00
parent 4f23c19928 40e830f9cf
commit c9da0718ba
6 changed files with 78 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/tools/lsp.md
+1 -1
View File
@@ -211,7 +211,7 @@ Uses the same location normalization and output shape as `definition`, but sends
**Execution**
- Workspace mode first invalidates the per-cwd configuration cache, reloads configuration from disk, and then reloads every newly configured non-custom LSP server.
- Single-file mode keeps the cached configuration and reloads the primary server for that file.
- Both modes clear matching recent initialization failures before starting a server. `reloadServer()` then tries the `rust-analyzer/reloadWorkspace` request, falls back to a `workspace/didChangeConfiguration` notification with `{ settings: {} }`, and finally tears down the client so the next request cold-starts it. For a shared-mux client, teardown first sends the mux restart notification so the shared server—not only this session's link—is replaced.
- Both modes clear matching recent initialization failures before starting a server. For rust-analyzer servers, `reloadServer()` first tries the `rust-analyzer/reloadWorkspace` request (only rust-analyzer implements it; sending it to other servers such as Roslyn can crash them, so it is gated on the server binary/name). Every server then falls back to a `workspace/didChangeConfiguration` notification with `{ settings: {} }`, and finally tears down the client so the next request cold-starts it. For a shared-mux client, teardown first sends the mux restart notification so the shared server—not only this session's link—is replaced.
**Output text**
- One line per server: `Reloaded <server>`, `Restarted <server>`, or `Failed to reload <server>: ...`.
packages/coding-agent/CHANGELOG.md
+3
View File
@@ -75,6 +75,9 @@
### Fixed
- Fixed `always-ask` approval prompts bypassing edit preview readiness when a built-in tool executes under its wire-level alias, such as `edit` running as `apply_patch` ([#8607](https://github.com/can1357/oh-my-pi/issues/8607)).
### Fixed
- Fixed `lsp reload` crashing non-rust-analyzer language servers (e.g. Roslyn/`roslyn-language-server`) by sending the rust-analyzer-specific `rust-analyzer/reloadWorkspace` request to every server; the request is now gated on the server being rust-analyzer, and all other servers reload via `workspace/didChangeConfiguration` directly ([#8571](https://github.com/can1357/oh-my-pi/issues/8571)).
## [17.3.4] - 2026-08-14
packages/coding-agent/src/lsp/client.ts
+7 -1
View File
@@ -746,7 +746,13 @@ function commandBasename(command: string): string {
return separator === -1 ? command : command.slice(separator + 1);
}
function isRustAnalyzerClient(client: LspClient): boolean {
/**
* True when this client speaks the rust-analyzer protocol, detected by the
* command basename (`rust-analyzer[.exe]`) of the configured or resolved
* binary. Callers use it to gate rust-analyzer-only requests such as
* `rust-analyzer/reloadWorkspace` (see {@link reloadServer}).
*/
export function isRustAnalyzerClient(client: LspClient): boolean {
return (
commandBasename(client.config.command) === "rust-analyzer" ||
(client.config.resolvedCommand ? commandBasename(client.config.resolvedCommand) === "rust-analyzer" : false)
packages/coding-agent/src/lsp/servers.ts
+20 -11
View File
@@ -4,6 +4,7 @@ import {
getActiveClients,
getActiveOrPendingClient,
getOrCreateClient,
isRustAnalyzerClient,
type LspServerStatus,
notifySaved,
sendNotification,
@@ -256,17 +257,25 @@ export function isMethodNotFoundError(err: unknown): boolean {
export async function reloadServer(client: LspClient, serverName: string, signal?: AbortSignal): Promise<string> {
throwIfAborted(signal);
// rust-analyzer exposes a real reload request. Every other server rejects it
// with method-not-found — that alone justifies the generic fallback. A caller
// cancel or tool timeout must propagate, never be mistaken for an unsupported
// method and swallowed into a bogus "Restarted" (issue #6369).
try {
await sendRequest(client, "rust-analyzer/reloadWorkspace", null, signal);
return `Reloaded ${serverName}`;
} catch (err) {
throwIfAborted(signal);
if (!isMethodNotFoundError(err)) throw err;
// Method not supported — fall through to the generic reload.
// rust-analyzer exposes a real reload request. Only rust-analyzer implements
// it, so gate the request on the binary (or registered name) rather than
// probing every server: some servers (Roslyn) crash the whole process on an
// unknown method instead of replying with method-not-found — killing the
// server `lsp reload` was meant to refresh (issue #8571, dotnet/roslyn#84890).
// A caller cancel or tool timeout must propagate, never be mistaken for an
// unsupported method and swallowed into a bogus "Restarted" (issue #6369).
if (isRustAnalyzerClient(client) || serverName === "rust-analyzer") {
try {
// Pass an empty object, not null: JSON-RPC structured params must be an
// object or array, and servers that validate this (Roslyn) reject a null
// before returning method-not-found (dotnet/roslyn#84890).
await sendRequest(client, "rust-analyzer/reloadWorkspace", {}, signal);
return `Reloaded ${serverName}`;
} catch (err) {
throwIfAborted(signal);
if (!isMethodNotFoundError(err)) throw err;
// Method not supported — fall through to the generic reload.
}
}
// workspace/didChangeConfiguration is a notification per spec; sending it
// as a request hangs until the tool deadline on servers that route it to
packages/coding-agent/src/lsp/tool.ts
+2 -4
View File
@@ -21,6 +21,7 @@ import {
ensureFileOpen,
getActiveClients,
getOrCreateClient,
isRustAnalyzerClient,
type LspServerStatus,
refreshFile,
sendNotification,
@@ -1075,10 +1076,7 @@ export class LspTool implements AgentTool<typeof lspSchema, LspToolDetails, Them
try {
const client = await getOrCreateClient(serverConfig, this.session.cwd, undefined, signal);
const targetFile = resolvedFile;
const isRustAnalyzerServer =
serverName === "rust-analyzer" ||
path.basename(serverConfig.command) === "rust-analyzer" ||
(serverConfig.resolvedCommand ? path.basename(serverConfig.resolvedCommand) === "rust-analyzer" : false);
const isRustAnalyzerServer = isRustAnalyzerClient(client) || serverName === "rust-analyzer";
const needsProjectIndex =
targetFile !== null && PROJECT_INDEXED_ACTIONS.has(action) && isProjectAwareLspServer(serverConfig);
const rustWorkspaceWait =
packages/coding-agent/test/tools/lsp-regressions.test.ts
+45 -8
View File
@@ -3503,9 +3503,9 @@ describe("lsp regressions", () => {
const loadConfigSpy = vi
.spyOn(lspConfig, "loadConfig")
.mockImplementation(() => configs.shift() ?? configs[0]);
const client = { proc: { kill: vi.fn() } } as unknown as LspClient;
const client = { proc: { kill: vi.fn() }, config: server } as unknown as LspClient;
vi.spyOn(lspClient, "getOrCreateClient").mockResolvedValue(client);
vi.spyOn(lspClient, "sendRequest").mockResolvedValue(null);
vi.spyOn(lspClient, "sendNotification").mockResolvedValue(undefined);
const tool = new LspTool(makeLspSession(tempDir.path()));
const initial = await tool.execute("reload-redetect-status", { action: "status" });
@@ -3640,8 +3640,8 @@ describe("lsp regressions", () => {
describe("reload cancellation and truthful teardown (#6369)", () => {
// A JSON-RPC error response the client maps to isMethodNotFoundError, so a
// non-rust server falls through from `rust-analyzer/reloadWorkspace` to the
// generic `workspace/didChangeConfiguration` reload.
// rust-analyzer server whose reloadWorkspace is unsupported falls through
// to the generic `workspace/didChangeConfiguration` reload.
const methodNotFound = (id: RpcMessage["id"]): RpcMessage => ({
jsonrpc: "2.0",
id,
@@ -3662,7 +3662,7 @@ describe("lsp regressions", () => {
// rust-analyzer/reloadWorkspace is left pending: only the caller
// signal decides its fate.
});
const config: ServerConfig = { command: "fake-reload-cancel-req", fileTypes: [".ts"], rootMarkers: [] };
const config: ServerConfig = { command: "rust-analyzer", fileTypes: [".ts"], rootMarkers: [] };
vi.spyOn(lspConfig, "loadConfig").mockReturnValue({ servers: { fake: config }, idleTimeoutMs: undefined });
const tool = new LspTool(makeLspSession(tempDir.path()));
@@ -3699,7 +3699,7 @@ describe("lsp regressions", () => {
srv.exit(0);
}
});
const config: ServerConfig = { command: "fake-reload-cancel-fb", fileTypes: [".ts"], rootMarkers: [] };
const config: ServerConfig = { command: "rust-analyzer", fileTypes: [".ts"], rootMarkers: [] };
vi.spyOn(lspConfig, "loadConfig").mockReturnValue({ servers: { fake: config }, idleTimeoutMs: undefined });
const tool = new LspTool(makeLspSession(tempDir.path()));
@@ -3727,7 +3727,7 @@ describe("lsp regressions", () => {
srv.exit(0);
}
});
const config: ServerConfig = { command: "fake-reload-fallback", fileTypes: [".ts"], rootMarkers: [] };
const config: ServerConfig = { command: "rust-analyzer", fileTypes: [".ts"], rootMarkers: [] };
vi.spyOn(lspConfig, "loadConfig").mockReturnValue({ servers: { fake: config }, idleTimeoutMs: undefined });
const tool = new LspTool(makeLspSession(tempDir.path()));
@@ -3758,7 +3758,7 @@ describe("lsp regressions", () => {
srv.exit(0);
}
});
const config: ServerConfig = { command: "fake-reload-fallback-code", fileTypes: [".ts"], rootMarkers: [] };
const config: ServerConfig = { command: "rust-analyzer", fileTypes: [".ts"], rootMarkers: [] };
vi.spyOn(lspConfig, "loadConfig").mockReturnValue({ servers: { fake: config }, idleTimeoutMs: undefined });
const tool = new LspTool(makeLspSession(tempDir.path()));
@@ -3773,6 +3773,43 @@ describe("lsp regressions", () => {
}
});
it("does not send rust-analyzer/reloadWorkspace to a non-rust server that crashes on it (#8571)", async () => {
const tempDir = TempDir.createSync("@omp-lsp-reload-non-rust-");
try {
let sawRustReload = false;
const server = installFakeLsp((message, srv) => {
if (message.method === "initialize") {
srv.send({ jsonrpc: "2.0", id: message.id, result: { capabilities: {} } });
} else if (message.method === "rust-analyzer/reloadWorkspace") {
// Roslyn crashes the whole process instead of replying -32601
// (dotnet/roslyn#84890); the gate must never let us send this.
sawRustReload = true;
srv.exit(82);
} else if (message.method === "shutdown") {
srv.send({ jsonrpc: "2.0", id: message.id, result: null });
} else if (message.method === "exit") {
srv.exit(0);
}
});
const config: ServerConfig = { command: "roslyn-language-server", fileTypes: [".cs"], rootMarkers: [] };
vi.spyOn(lspConfig, "loadConfig").mockReturnValue({
servers: { csharp: config },
idleTimeoutMs: undefined,
});
const tool = new LspTool(makeLspSession(tempDir.path()));
const result = await tool.execute("reload-non-rust", { action: "reload", file: "*" });
expect(sawRustReload).toBe(false);
expect(textResult(result)).toContain("Reloaded csharp");
expect(server.killed).toBe(false);
} finally {
vi.restoreAllMocks();
await lspClient.shutdownAll();
tempDir.removeSync();
}
});
it("shutdownClientInstance removes the client by identity and confirms process exit", async () => {
const tempDir = TempDir.createSync("@omp-lsp-teardown-confirm-");
try {