fix(daemon): launch LSP servers with caller env
This commit is contained in:
@@ -21,14 +21,14 @@ The extension is **stateless** — it opens a fresh socket connection per reques
|
|||||||
|
|
||||||
Communication is **newline-delimited JSON (NDJSON)** over a Unix socket at `$XDG_RUNTIME_DIR/pi-lsp-$UID.sock`. Each line is one independent request/response pair with an `id` field for matching. See `src/daemonProtocol.ts` for the type definitions (`DaemonRequest`, `DaemonResponse`).
|
Communication is **newline-delimited JSON (NDJSON)** over a Unix socket at `$XDG_RUNTIME_DIR/pi-lsp-$UID.sock`. Each line is one independent request/response pair with an `id` field for matching. See `src/daemonProtocol.ts` for the type definitions (`DaemonRequest`, `DaemonResponse`).
|
||||||
|
|
||||||
Current ops: `request`, `diagnostics`, `status`, `shutdown`.
|
Current ops: `request`, `diagnostics`, `status`, `shutdown`, `destroy_server`. `request` and `diagnostics` include a `launch` context with the caller env. The env is used only when spawning a new server for `(server.id, rootDir)`; existing running servers keep their original process env until idle eviction or manual destroy/restart.
|
||||||
|
|
||||||
### Server Lifecycle
|
### Server Lifecycle
|
||||||
|
|
||||||
1. First LSP tool call for a file triggers `getOrCreateEntry()` in the daemon
|
1. First LSP tool call for a file triggers `getOrCreateEntry()` in the daemon
|
||||||
2. `pickServer()` matches the file extension against `server.ts` registry
|
2. `pickServer()` matches the file extension against `server.ts` registry
|
||||||
3. `findRoot()` walks upward looking for root markers (e.g., `go.mod`, `tsconfig.json`)
|
3. `findRoot()` walks upward looking for root markers (e.g., `go.mod`, `tsconfig.json`)
|
||||||
4. A new `LspClient` is spawned, initialized via LSP `initialize`/`initialized`, and waited on (`waitForReady()`)
|
4. A new `LspClient` is spawned with the caller/session environment from the daemon request, initialized via LSP `initialize`/`initialized`, and waited on (`waitForReady()`)
|
||||||
5. The file is synced via `didOpen` or `didChange` (based on mtime comparison)
|
5. The file is synced via `didOpen` or `didChange` (based on mtime comparison)
|
||||||
6. On idle timeout (default 5 min), the entry is evicted and the server process killed
|
6. On idle timeout (default 5 min), the entry is evicted and the server process killed
|
||||||
|
|
||||||
|
|||||||
@@ -16,8 +16,8 @@ import { ServerNotFoundError } from "./types.ts";
|
|||||||
import { findRoot, pathToUri, uriToPath } from "./root.ts";
|
import { findRoot, pathToUri, uriToPath } from "./root.ts";
|
||||||
|
|
||||||
// Is On PATH - Returns true if `cmd` resolves to an executable via the
|
// Is On PATH - Returns true if `cmd` resolves to an executable via the
|
||||||
// current PATH. Absolute/relative paths are checked directly.
|
// supplied PATH. Absolute/relative paths are checked directly.
|
||||||
function isOnPath(cmd: string): boolean {
|
function isOnPath(cmd: string, env: NodeJS.ProcessEnv): boolean {
|
||||||
const isExec = (p: string) => {
|
const isExec = (p: string) => {
|
||||||
try {
|
try {
|
||||||
fs.accessSync(p, fs.constants.X_OK);
|
fs.accessSync(p, fs.constants.X_OK);
|
||||||
@@ -29,9 +29,9 @@ function isOnPath(cmd: string): boolean {
|
|||||||
if (cmd.includes(path.sep)) return isExec(cmd);
|
if (cmd.includes(path.sep)) return isExec(cmd);
|
||||||
const exts =
|
const exts =
|
||||||
process.platform === "win32"
|
process.platform === "win32"
|
||||||
? (process.env.PATHEXT ?? ".EXE;.CMD;.BAT").split(";")
|
? (env.PATHEXT ?? ".EXE;.CMD;.BAT").split(";")
|
||||||
: [""];
|
: [""];
|
||||||
for (const dir of (process.env.PATH ?? "").split(path.delimiter)) {
|
for (const dir of (env.PATH ?? "").split(path.delimiter)) {
|
||||||
if (!dir) continue;
|
if (!dir) continue;
|
||||||
for (const ext of exts) {
|
for (const ext of exts) {
|
||||||
if (isExec(path.join(dir, cmd + ext))) return true;
|
if (isExec(path.join(dir, cmd + ext))) return true;
|
||||||
@@ -69,16 +69,17 @@ export class LspClient {
|
|||||||
constructor(private readonly server: ServerConfig) {}
|
constructor(private readonly server: ServerConfig) {}
|
||||||
|
|
||||||
// Start - Spawns the server process and wires up JSON-RPC.
|
// Start - Spawns the server process and wires up JSON-RPC.
|
||||||
async start(rootDir: string): Promise<void> {
|
async start(rootDir: string, env: NodeJS.ProcessEnv): Promise<void> {
|
||||||
// Verify Binary On PATH - Fail fast with a clear message instead of
|
// Verify Binary On PATH - Fail fast with a clear message instead of
|
||||||
// letting spawn ENOENT surface as a generic error. It's the user's
|
// letting spawn ENOENT surface as a generic error. Resolution uses the
|
||||||
// responsibility to have the server installed & on PATH.
|
// caller/session env, not the daemon's launch-time env.
|
||||||
if (!isOnPath(this.server.command)) {
|
if (!isOnPath(this.server.command, env)) {
|
||||||
throw new ServerNotFoundError(this.server.command);
|
throw new ServerNotFoundError(this.server.command);
|
||||||
}
|
}
|
||||||
this.proc = spawn(this.server.command, this.server.args, {
|
this.proc = spawn(this.server.command, this.server.args, {
|
||||||
stdio: ["pipe", "pipe", "pipe"],
|
stdio: ["pipe", "pipe", "pipe"],
|
||||||
cwd: rootDir,
|
cwd: rootDir,
|
||||||
|
env,
|
||||||
});
|
});
|
||||||
this.proc.on("error", (err) => {
|
this.proc.on("error", (err) => {
|
||||||
process.stderr.write(
|
process.stderr.write(
|
||||||
@@ -277,10 +278,11 @@ export class LspClient {
|
|||||||
export async function startClientForFile(
|
export async function startClientForFile(
|
||||||
server: ServerConfig,
|
server: ServerConfig,
|
||||||
filePath: string,
|
filePath: string,
|
||||||
|
env: NodeJS.ProcessEnv = process.env,
|
||||||
): Promise<{ client: LspClient; uri: string; rootDir: string }> {
|
): Promise<{ client: LspClient; uri: string; rootDir: string }> {
|
||||||
const rootDir = findRoot(filePath, server.rootMarkers);
|
const rootDir = findRoot(filePath, server.rootMarkers);
|
||||||
const client = new LspClient(server);
|
const client = new LspClient(server);
|
||||||
await client.start(rootDir);
|
await client.start(rootDir, env);
|
||||||
const uri = client.openDocument(filePath);
|
const uri = client.openDocument(filePath);
|
||||||
// Wait For Workspace Load - gopls & friends reject requests with errors
|
// Wait For Workspace Load - gopls & friends reject requests with errors
|
||||||
// like "no views" until their initial load completes.
|
// like "no views" until their initial load completes.
|
||||||
|
|||||||
@@ -14,6 +14,7 @@ import {
|
|||||||
tryConnect,
|
tryConnect,
|
||||||
type DaemonRequest,
|
type DaemonRequest,
|
||||||
type DaemonResponse,
|
type DaemonResponse,
|
||||||
|
type LaunchContext,
|
||||||
} from "./daemonProtocol.ts";
|
} from "./daemonProtocol.ts";
|
||||||
|
|
||||||
// Default Idle TTL - 5 minutes. Per-server overrides via ServerConfig.idleTtlMs.
|
// Default Idle TTL - 5 minutes. Per-server overrides via ServerConfig.idleTtlMs.
|
||||||
@@ -52,7 +53,10 @@ function log(...args: unknown[]) {
|
|||||||
// Get Or Create Entry - Looks up the cached client for a file, spawning a
|
// Get Or Create Entry - Looks up the cached client for a file, spawning a
|
||||||
// fresh LspClient if needed. The returned entry is guaranteed to have its
|
// fresh LspClient if needed. The returned entry is guaranteed to have its
|
||||||
// `ready` promise resolved before the caller uses it.
|
// `ready` promise resolved before the caller uses it.
|
||||||
async function getOrCreateEntry(filePath: string): Promise<ClientEntry> {
|
async function getOrCreateEntry(
|
||||||
|
filePath: string,
|
||||||
|
launch: LaunchContext,
|
||||||
|
): Promise<ClientEntry> {
|
||||||
const server = pickServer(filePath);
|
const server = pickServer(filePath);
|
||||||
const rootDir = findRoot(filePath, server.rootMarkers);
|
const rootDir = findRoot(filePath, server.rootMarkers);
|
||||||
const key = `${server.id}::${rootDir}`;
|
const key = `${server.id}::${rootDir}`;
|
||||||
@@ -72,7 +76,7 @@ async function getOrCreateEntry(filePath: string): Promise<ClientEntry> {
|
|||||||
client,
|
client,
|
||||||
ready: (async () => {
|
ready: (async () => {
|
||||||
log(`spawn`, server.id, rootDir);
|
log(`spawn`, server.id, rootDir);
|
||||||
await client.start(rootDir);
|
await client.start(rootDir, launch.env);
|
||||||
await client.waitForReady();
|
await client.waitForReady();
|
||||||
log(`ready`, server.id);
|
log(`ready`, server.id);
|
||||||
})(),
|
})(),
|
||||||
@@ -157,7 +161,7 @@ async function handle(req: DaemonRequest): Promise<DaemonResponse> {
|
|||||||
switch (req.op) {
|
switch (req.op) {
|
||||||
case "request": {
|
case "request": {
|
||||||
const filePath = path.resolve(req.file);
|
const filePath = path.resolve(req.file);
|
||||||
const entry = await getOrCreateEntry(filePath);
|
const entry = await getOrCreateEntry(filePath, req.launch);
|
||||||
const { uri } = await syncFile(entry, filePath);
|
const { uri } = await syncFile(entry, filePath);
|
||||||
bumpIdle(entry);
|
bumpIdle(entry);
|
||||||
const result = await entry.client.sendRequest(
|
const result = await entry.client.sendRequest(
|
||||||
@@ -168,7 +172,7 @@ async function handle(req: DaemonRequest): Promise<DaemonResponse> {
|
|||||||
}
|
}
|
||||||
case "diagnostics": {
|
case "diagnostics": {
|
||||||
const filePath = path.resolve(req.file);
|
const filePath = path.resolve(req.file);
|
||||||
const entry = await getOrCreateEntry(filePath);
|
const entry = await getOrCreateEntry(filePath, req.launch);
|
||||||
const { uri, changed } = await syncFile(entry, filePath);
|
const { uri, changed } = await syncFile(entry, filePath);
|
||||||
bumpIdle(entry);
|
bumpIdle(entry);
|
||||||
if (changed) entry.client.clearDiagnostics(uri);
|
if (changed) entry.client.clearDiagnostics(uri);
|
||||||
|
|||||||
@@ -5,7 +5,11 @@
|
|||||||
// Why Not One Persistent Socket - For now we open a fresh connection per
|
// Why Not One Persistent Socket - For now we open a fresh connection per
|
||||||
// request. The cost is negligible (Unix socket, same machine) compared to
|
// request. The cost is negligible (Unix socket, same machine) compared to
|
||||||
// the LSP request itself, and it keeps client code stateless.
|
// the LSP request itself, and it keeps client code stateless.
|
||||||
import { sendOnce, type DaemonResponse } from "./daemonProtocol.ts";
|
import {
|
||||||
|
buildLaunchContext,
|
||||||
|
sendOnce,
|
||||||
|
type DaemonResponse,
|
||||||
|
} from "./daemonProtocol.ts";
|
||||||
|
|
||||||
// Unwrap - Throws on { ok: false }, returns result on { ok: true }. All
|
// Unwrap - Throws on { ok: false }, returns result on { ok: true }. All
|
||||||
// callers want the result-or-throw shape, so we centralize it.
|
// callers want the result-or-throw shape, so we centralize it.
|
||||||
@@ -21,7 +25,15 @@ export async function daemonRequest(
|
|||||||
method: string,
|
method: string,
|
||||||
params: Record<string, unknown>,
|
params: Record<string, unknown>,
|
||||||
): Promise<unknown> {
|
): Promise<unknown> {
|
||||||
return unwrap(await sendOnce({ op: "request", file, method, params }));
|
return unwrap(
|
||||||
|
await sendOnce({
|
||||||
|
op: "request",
|
||||||
|
file,
|
||||||
|
method,
|
||||||
|
params,
|
||||||
|
launch: buildLaunchContext(),
|
||||||
|
}),
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Wait For Diagnostics - Diagnostics arrive as a notification, not a
|
// Wait For Diagnostics - Diagnostics arrive as a notification, not a
|
||||||
@@ -30,7 +42,14 @@ export async function daemonDiagnostics(
|
|||||||
file: string,
|
file: string,
|
||||||
timeoutMs = 1500,
|
timeoutMs = 1500,
|
||||||
): Promise<unknown> {
|
): Promise<unknown> {
|
||||||
return unwrap(await sendOnce({ op: "diagnostics", file, timeoutMs }));
|
return unwrap(
|
||||||
|
await sendOnce({
|
||||||
|
op: "diagnostics",
|
||||||
|
file,
|
||||||
|
timeoutMs,
|
||||||
|
launch: buildLaunchContext(),
|
||||||
|
}),
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Status - Lists currently-cached LSP servers (id, root, opened files,
|
// Status - Lists currently-cached LSP servers (id, root, opened files,
|
||||||
|
|||||||
@@ -10,6 +10,28 @@ import * as os from "node:os";
|
|||||||
import * as path from "node:path";
|
import * as path from "node:path";
|
||||||
import { spawn } from "node:child_process";
|
import { spawn } from "node:child_process";
|
||||||
|
|
||||||
|
// Launch Context - Captures the caller/session environment used when the
|
||||||
|
// daemon spawns a new language server. Running servers keep their original
|
||||||
|
// process env; later requests for the same root reuse the existing server.
|
||||||
|
export interface LaunchContext {
|
||||||
|
env: Record<string, string>;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Build Launch Context - Convert Node's optional-valued process.env into the
|
||||||
|
// concrete string map accepted by child_process.spawn(). Env contents are
|
||||||
|
// sensitive: keep them internal to requests and never log or expose them.
|
||||||
|
export function buildLaunchContext(
|
||||||
|
env: NodeJS.ProcessEnv = process.env,
|
||||||
|
): LaunchContext {
|
||||||
|
return {
|
||||||
|
env: Object.fromEntries(
|
||||||
|
Object.entries(env).filter((entry): entry is [string, string] => {
|
||||||
|
return typeof entry[1] === "string";
|
||||||
|
}),
|
||||||
|
),
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
// Request Shapes - Sent client -> daemon.
|
// Request Shapes - Sent client -> daemon.
|
||||||
export type DaemonRequest =
|
export type DaemonRequest =
|
||||||
| {
|
| {
|
||||||
@@ -18,8 +40,15 @@ export type DaemonRequest =
|
|||||||
file: string;
|
file: string;
|
||||||
method: string;
|
method: string;
|
||||||
params: Record<string, unknown>;
|
params: Record<string, unknown>;
|
||||||
|
launch: LaunchContext;
|
||||||
|
}
|
||||||
|
| {
|
||||||
|
id: number;
|
||||||
|
op: "diagnostics";
|
||||||
|
file: string;
|
||||||
|
timeoutMs?: number;
|
||||||
|
launch: LaunchContext;
|
||||||
}
|
}
|
||||||
| { id: number; op: "diagnostics"; file: string; timeoutMs?: number }
|
|
||||||
| { id: number; op: "status" }
|
| { id: number; op: "status" }
|
||||||
| { id: number; op: "shutdown" }
|
| { id: number; op: "shutdown" }
|
||||||
| { id: number; op: "destroy_server"; serverId?: string };
|
| { id: number; op: "destroy_server"; serverId?: string };
|
||||||
@@ -30,8 +59,14 @@ export type DaemonRequestWithoutId =
|
|||||||
file: string;
|
file: string;
|
||||||
method: string;
|
method: string;
|
||||||
params: Record<string, unknown>;
|
params: Record<string, unknown>;
|
||||||
|
launch: LaunchContext;
|
||||||
|
}
|
||||||
|
| {
|
||||||
|
op: "diagnostics";
|
||||||
|
file: string;
|
||||||
|
timeoutMs?: number;
|
||||||
|
launch: LaunchContext;
|
||||||
}
|
}
|
||||||
| { op: "diagnostics"; file: string; timeoutMs?: number }
|
|
||||||
| { op: "status" }
|
| { op: "status" }
|
||||||
| { op: "shutdown" }
|
| { op: "shutdown" }
|
||||||
| { op: "destroy_server"; serverId?: string };
|
| { op: "destroy_server"; serverId?: string };
|
||||||
|
|||||||
Reference in New Issue
Block a user