mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 07:20:43 +00:00
fix(lsp): resolve Windows .cmd shims (#75343)
Resolve Windows npm .cmd shim startup failures for bundled LSP servers by routing LSP process spawning through the shared Windows spawn resolver with a sanitized child environment. The change reuses existing PATH/PATHEXT and .cmd shim handling, keeps non-Windows behavior unchanged, and adds focused regression coverage for resolver wiring, env sanitization, and spawn materialization. Fixes #75352. Tests: - pnpm test src/agents/pi-bundle-lsp-runtime.windows-spawn.test.ts src/agents/pi-bundle-lsp-runtime.test.ts - pnpm check:changed Thanks @ElliotDrel. Co-authored-by: Elliot Drel <156480527+ElliotDrel@users.noreply.github.com> Co-authored-by: Brad Groux <3053586+BradGroux@users.noreply.github.com>
This commit is contained in:
@@ -421,6 +421,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Agents/models: forward model `maxTokens` as the default output-token limit for OpenAI-compatible Responses and Completions transports when no runtime override is provided, preventing provider defaults from silently truncating larger outputs. (#76645) Thanks @joeyfrasier.
|
||||
- macOS CLI/onboarding: honor sensitive wizard text steps in `openclaw-mac wizard` with termios no-echo input, suppressing saved credential previews while preserving long API keys and gateway tokens. Fixes #76698. Thanks @anurag-bg-neu and @sallyom.
|
||||
- Control UI/Skills: fix skill detail modal silently failing to open in all browsers by deferring `showModal()` until the dialog element is connected to the DOM; the Lit `ref` callback fired before connection causing a `DOMException: HTMLDialogElement.showModal: Dialog element is not connected` on every skill click. Thanks @nickmopen.
|
||||
- fix(lsp): resolve Windows .cmd shims in LSP server spawning so npm-installed language servers (e.g. typescript-language-server) start correctly on Windows. Fixes #75352. Thanks @ElliotDrel.
|
||||
- Gateway/update: run `doctor --non-interactive --fix` after Control UI global package updates before reporting success, so legacy config is migrated before the gateway restart. Thanks @stevenchouai.
|
||||
- Gateway/cron: stop a lazy cron startup that loses a hot-reload race, preventing the old cron service from starting after reload has already replaced cron state.
|
||||
- CLI/plugins: warn when npm plugin installs remain shadowed by a failing config-selected source and surface the repair path in `plugins doctor`. Thanks @LindalyX-Lee.
|
||||
|
||||
@@ -2,6 +2,11 @@ import { spawn, type ChildProcess } from "node:child_process";
|
||||
import type { AgentToolResult } from "@mariozechner/pi-agent-core";
|
||||
import type { OpenClawConfig } from "../config/types.openclaw.js";
|
||||
import { logDebug, logWarn } from "../logger.js";
|
||||
import {
|
||||
materializeWindowsSpawnProgram,
|
||||
resolveWindowsSpawnProgram,
|
||||
} from "../plugin-sdk/windows-spawn.js";
|
||||
import { sanitizeHostExecEnv } from "../infra/host-env-security.js";
|
||||
import { setPluginToolMeta } from "../plugins/tools.js";
|
||||
import { killProcessTree } from "../process/kill-tree.js";
|
||||
import { normalizeOptionalLowercaseString } from "../shared/string-coerce.js";
|
||||
@@ -64,13 +69,21 @@ function delay(ms: number): Promise<void> {
|
||||
});
|
||||
}
|
||||
|
||||
function spawnLspServerProcess(config: StdioMcpServerLaunchConfig): ChildProcess {
|
||||
return spawn(config.command, config.args ?? [], {
|
||||
export function spawnLspServerProcess(config: StdioMcpServerLaunchConfig): ChildProcess {
|
||||
const mergedEnv = sanitizeHostExecEnv({ baseEnv: process.env, overrides: config.env ?? null });
|
||||
const program = resolveWindowsSpawnProgram({
|
||||
command: config.command,
|
||||
env: mergedEnv,
|
||||
allowShellFallback: true,
|
||||
});
|
||||
const invocation = materializeWindowsSpawnProgram(program, config.args ?? []);
|
||||
return spawn(invocation.command, invocation.argv, {
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
env: { ...process.env, ...config.env },
|
||||
env: mergedEnv,
|
||||
cwd: config.cwd,
|
||||
detached: process.platform !== "win32",
|
||||
windowsHide: process.platform === "win32",
|
||||
windowsHide: invocation.windowsHide ?? process.platform === "win32",
|
||||
shell: invocation.shell,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
111
src/agents/pi-bundle-lsp-runtime.windows-spawn.test.ts
Normal file
111
src/agents/pi-bundle-lsp-runtime.windows-spawn.test.ts
Normal file
@@ -0,0 +1,111 @@
|
||||
import { describe, expect, it, vi, beforeEach } from "vitest";
|
||||
|
||||
const resolveWindowsSpawnProgramMock = vi.hoisted(() => vi.fn());
|
||||
const materializeWindowsSpawnProgramMock = vi.hoisted(() => vi.fn());
|
||||
const sanitizeHostExecEnvMock = vi.hoisted(() => vi.fn());
|
||||
const spawnMock = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("../plugin-sdk/windows-spawn.js", () => ({
|
||||
resolveWindowsSpawnProgram: resolveWindowsSpawnProgramMock,
|
||||
materializeWindowsSpawnProgram: materializeWindowsSpawnProgramMock,
|
||||
}));
|
||||
|
||||
vi.mock("../infra/host-env-security.js", () => ({
|
||||
sanitizeHostExecEnv: sanitizeHostExecEnvMock,
|
||||
}));
|
||||
|
||||
vi.mock("node:child_process", async () => ({
|
||||
...(await vi.importActual<typeof import("node:child_process")>("node:child_process")),
|
||||
spawn: spawnMock,
|
||||
}));
|
||||
|
||||
vi.mock("../logger.js", () => ({
|
||||
logDebug: vi.fn(),
|
||||
logWarn: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("../process/kill-tree.js", () => ({
|
||||
killProcessTree: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("./embedded-pi-lsp.js", () => ({
|
||||
loadEmbeddedPiLspConfig: vi.fn().mockReturnValue({ lspServers: {}, diagnostics: [] }),
|
||||
}));
|
||||
|
||||
const FAKE_CHILD = {
|
||||
stdout: { setEncoding: vi.fn(), on: vi.fn() },
|
||||
stderr: { setEncoding: vi.fn(), on: vi.fn() },
|
||||
on: vi.fn(),
|
||||
pid: 1234,
|
||||
} as unknown as import("node:child_process").ChildProcess;
|
||||
|
||||
describe("spawnLspServerProcess Windows .cmd shim handling", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
spawnMock.mockReturnValue(FAKE_CHILD);
|
||||
});
|
||||
|
||||
it("calls sanitizeHostExecEnv with baseEnv/overrides, not a flat merged object", async () => {
|
||||
const configEnv = { MY_TOKEN: "secret", TOOL_PATH: "/custom" };
|
||||
const sanitizedEnv = { PATH: "/usr/bin", MY_TOKEN: "secret", TOOL_PATH: "/custom" };
|
||||
|
||||
sanitizeHostExecEnvMock.mockReturnValue(sanitizedEnv);
|
||||
resolveWindowsSpawnProgramMock.mockReturnValue({ resolvedCommand: "tls", isShim: false });
|
||||
materializeWindowsSpawnProgramMock.mockReturnValue({
|
||||
command: "typescript-language-server",
|
||||
argv: ["--stdio"],
|
||||
shell: false,
|
||||
windowsHide: true,
|
||||
});
|
||||
|
||||
const { spawnLspServerProcess } = await import("./pi-bundle-lsp-runtime.js");
|
||||
spawnLspServerProcess({ command: "typescript-language-server", args: ["--stdio"], env: configEnv });
|
||||
|
||||
// Must use structured params so config.env entries are not dropped
|
||||
expect(sanitizeHostExecEnvMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ baseEnv: process.env, overrides: configEnv }),
|
||||
);
|
||||
});
|
||||
|
||||
it("passes sanitized env to resolveWindowsSpawnProgram", async () => {
|
||||
const sanitizedEnv = { PATH: "C:\\Windows;C:\\nodejs", PATHEXT: ".COM;.EXE;.BAT;.CMD" };
|
||||
|
||||
sanitizeHostExecEnvMock.mockReturnValue(sanitizedEnv);
|
||||
resolveWindowsSpawnProgramMock.mockReturnValue({ resolvedCommand: "tls", isShim: false });
|
||||
materializeWindowsSpawnProgramMock.mockReturnValue({
|
||||
command: "typescript-language-server",
|
||||
argv: ["--stdio"],
|
||||
shell: false,
|
||||
windowsHide: true,
|
||||
});
|
||||
|
||||
const { spawnLspServerProcess } = await import("./pi-bundle-lsp-runtime.js");
|
||||
spawnLspServerProcess({ command: "typescript-language-server", args: ["--stdio"] });
|
||||
|
||||
expect(resolveWindowsSpawnProgramMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ env: sanitizedEnv, allowShellFallback: true }),
|
||||
);
|
||||
});
|
||||
|
||||
it("passes materialized invocation to spawn with the sanitized env", async () => {
|
||||
const sanitizedEnv = { PATH: "/usr/bin" };
|
||||
|
||||
sanitizeHostExecEnvMock.mockReturnValue(sanitizedEnv);
|
||||
resolveWindowsSpawnProgramMock.mockReturnValue({ resolvedCommand: "tls", isShim: true });
|
||||
materializeWindowsSpawnProgramMock.mockReturnValue({
|
||||
command: "cmd.exe",
|
||||
argv: ["/c", "typescript-language-server.cmd", "--stdio"],
|
||||
shell: true,
|
||||
windowsHide: true,
|
||||
});
|
||||
|
||||
const { spawnLspServerProcess } = await import("./pi-bundle-lsp-runtime.js");
|
||||
spawnLspServerProcess({ command: "typescript-language-server", args: ["--stdio"] });
|
||||
|
||||
expect(spawnMock).toHaveBeenCalledWith(
|
||||
"cmd.exe",
|
||||
["/c", "typescript-language-server.cmd", "--stdio"],
|
||||
expect.objectContaining({ env: sanitizedEnv, shell: true, windowsHide: true }),
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user