diff --git a/.changeset/share-server-instances.md b/.changeset/share-server-instances.md new file mode 100644 index 00000000..22a70102 --- /dev/null +++ b/.changeset/share-server-instances.md @@ -0,0 +1,5 @@ +--- +"@ian-pascoe/pi-lsp": minor +--- + +Share Server Instances among the Pi sessions in one Pi process, so a main session and its in-process subagents or Advisor sessions no longer each start and load their own copy of every language server. Sessions whose Server Definition resolves to the same command, arguments, environment, `initializationOptions`, `settings`, `lsp.timeouts`, and `maxConcurrentDiagnostics` for the same workspace root lease one process; any difference, such as a project override only one session trusts, still gets its own. A session takes its lease on first use and releases it at shutdown or when it disables the Server Definition, and the process stops when the last lease is released. A crash makes the instance unavailable in every session using it, and `/lsp stop` or `lsp_restart` from any of them stops it for all, each starting it again on next use. `lsp_status` and `/lsp` add `shared with N other sessions` to a running instance other sessions also lease, with a matching `shared_with_sessions` structured field. A server-initiated edit becomes a preview only when one session uses the instance; otherwise it is declined because it cannot be attributed to a session. Server stderr files now live in a private temporary directory per Server Instance, removed when it stops, instead of in the session directory. diff --git a/packages/pi-lsp/GLOSSARY.md b/packages/pi-lsp/GLOSSARY.md index 6f868c23..b25d81c8 100644 --- a/packages/pi-lsp/GLOSSARY.md +++ b/packages/pi-lsp/GLOSSARY.md @@ -16,7 +16,8 @@ - **Result Spill** — the complete LSP operation output referenced when the model-visible result reaches Pi's standard output limit (it holds the complete text), or when a structured result exceeds its 1 MiB cap and is bounded (it holds the complete structured data as JSON). - **Structured Result** — the JSON `structuredContent` of an LSP tool result, which programmatic callers such as codemode scripts receive. It is kept out of model context, adds flat one-based `path`, `line`, and `character` to locations, symbols, and hierarchy items and a `kind_name` to symbols so a script can pass them straight to a position tool, is capped at 1 MiB like Pi's built-in `bash` tool, and is bounded deterministically (longest strings and array tails cut, identifying fields kept, `structured_truncated: true`) when larger. `truncated` also covers a cut of only the model-visible text. - **Server Definition** — a configured language-server command, language mapping, workspace-root policy (the nearest root marker by default, or the nearest workspace root marker such as `pnpm-workspace.yaml` so one Server Instance covers a monorepo), Activation Gate, and protocol settings identified by a stable server ID. A project Server Definition replaces a global definition with the same ID; an invalid project replacement shadows the global definition and is quarantined. -- **Server Instance** — one running language-server process for a Server Definition and a detected workspace root. +- **Server Instance** — one running language-server process for a Server Definition's effective process configuration and a detected workspace root, shared by every Pi session in the process that resolves the same pair. +- **Server Instance Lease** — a Pi session's claim on a shared Server Instance, taken on first use and released at session shutdown, when the session disables the Server Definition, or when the instance is stopped; the instance runs while it has at least one. - **Stop** — ending a Server Instance without preventing a later lazy start for its Server Definition. - **Disabled Server Definition** — a Server Definition that is ineligible to start Server Instances until it is enabled. - **Server Enablement Override** — an explicit enabled or disabled choice for a Server Definition at session, project, or global scope. A session-scoped choice belongs to the selected session history and does not apply when navigating before it. @@ -33,7 +34,7 @@ A tool call discards the Workspace Edit Previews it created when its returned re Language-server documents are valid UTF-8 text. A result position is converted only against the text of the file it lies in; positions in a file without readable text are approximated and named in a warning, never converted against another file's text. A position that disagrees with a readable file's current text never fails the server's result: a character past the line end is clamped to it without a warning, as the LSP specification says; a line past the document end keeps the position, a character inside a Unicode character snaps to its start, and a warning names the file, because it changed since the server read it or the server sent an invalid position. Content edits follow existing symlinks and identify the canonical target in the Mutation Manifest; resource operations act on the named directory entry. Conflicting or non-file workspace edits are rejected before they become applicable previews. A code action whose edit fails this validation is listed as not applicable, with the reason, while the server's other actions remain. -Server Definitions come only from the `lsp` key in Pi's global and trusted project settings. Pi's standard reload lifecycle reloads configuration. Server Instances start lazily, are reused within the Pi session, and retain failure state until explicit recovery. Read operations and code-action listing may query several matching Server Instances; a rename or formatting preview must identify one when several match. References and rename name the root of the Server Instance they searched and warn when other roots of the same Server Definition exist, because files under those roots may be missing from the result. A directory counts as another root only when its files route to a different Server Instance; packages inside a searched workspace root do not. Because a language server searches only packages it has loaded, references and rename in a workspace root also name its packages (directories holding a root marker) where the Server Instance has no synchronized document; before a references or rename request, the extension opens one representative file in each unloaded package, capped by count and time, and names the packages still unloaded (ADR-0004). Workspace root selection never uses the home directory or anything above it unless the working directory is there. +Server Definitions come only from the `lsp` key in Pi's global and trusted project settings. Pi's standard reload lifecycle reloads configuration. Server Instances start lazily, are shared by the Pi sessions in one Pi process that resolve the same Server Definition configuration and root, and retain failure state until a Stop or restart from any of them. Read operations and code-action listing may query several matching Server Instances; a rename or formatting preview must identify one when several match. References and rename name the root of the Server Instance they searched and warn when other roots of the same Server Definition exist, because files under those roots may be missing from the result. A directory counts as another root only when its files route to a different Server Instance; packages inside a searched workspace root do not. Because a language server searches only packages it has loaded, references and rename in a workspace root also name its packages (directories holding a root marker) where the Server Instance has no synchronized document; before a references or rename request, the extension opens one representative file in each unloaded package, capped by count and time, and names the packages still unloaded (ADR-0004). Workspace root selection never uses the home directory or anything above it unless the working directory is there. An Activation Gate is evaluated independently for every candidate file. A Server Definition that does not pass its gate is excluded from automatic routing without warning. Changes to root markers diff --git a/packages/pi-lsp/README.md b/packages/pi-lsp/README.md index de31a774..520c9e40 100644 --- a/packages/pi-lsp/README.md +++ b/packages/pi-lsp/README.md @@ -123,9 +123,11 @@ Invalid server definitions, timeout fields, and `includeHintDiagnostics` are qua individually and remain visible through `status`; unrelated valid settings continue to work. An invalid project server replacement still shadows the global definition. Untrusted project settings are ignored. -Pi's `/reload` reloads configuration. Servers start lazily on first use and live for one Pi session. -After a process or protocol failure, recovery requires stopping the affected Instance, disabling -then enabling its Definition, using the existing `restart` tool operation, or `/reload`. +Pi's `/reload` reloads configuration. Servers start lazily on first use and are shared by the Pi +sessions in one Pi process, such as a main session and its in-process subagents, that resolve the +same Server Definition configuration and root (see [Limits and lifecycle](#limits-and-lifecycle)). +After a process or protocol failure, recovery requires stopping the affected Instance, using the +existing `restart` tool operation, or ending every session that uses it. ## `/lsp` command @@ -141,13 +143,18 @@ roots and the effective enablement scope; opening it never starts a server. Text /lsp enable typescript --global ``` -- **Stop** ends one known Server Instance and clears its failure state. The next matching request - can start it again. Omit the root when only one is known; otherwise choose a root in the picker - or supply its path, relative to Pi's working directory or absolute. -- **Disable** prevents automatic and explicit startup of the entire Server Definition, stops all - its Instances in this session, and clears their failed runtime state. +- **Stop** ends one known Server Instance and clears its failure state, for every session sharing + it. The next matching request in any of them can start it again. Omit the root when only one is + known; otherwise choose a root in the picker or supply its path, relative to Pi's working + directory or absolute. +- **Disable** prevents automatic and explicit startup of the entire Server Definition in this + session and releases this session's leases on its Instances. An Instance stops only when no + other session still uses it; other sessions keep using it. - **Enable** permits lazy startup without launching a process. +The status and picker rows of a running Instance that other sessions also use end with +`shared with N other sessions`. + Unflagged enable/disable choices are custom session entries. They survive `/reload` and saved-session resume, but follow the selected branch: navigating before a choice rolls it back, and forks inherit choices on their selected ancestry. A newly created session starts without session overrides. Pi @@ -305,6 +312,9 @@ Diagnostics, hover, status, code actions, and apply results are compact text too documentation; servers that only answer plaintext render as sent. - `lsp_status` lists one `server_id state [root] language(extensions,...) [error: ...]` line per Server Instance (running, starting, unavailable, or stopped) and per Disabled Server Definition. + A running Instance that other Pi sessions in the process also lease ends its line with + `shared with N other sessions`, and its structured entry has a matching `shared_with_sessions`. + An Instance this session holds no lease on reads `stopped`, even while other sessions use it. One `+N configured, not started (pass all: true to list)` line counts the other Server Definitions, followed by settings warnings. Pass `all: true` to list every configured Server Definition. Its structured result applies the same filter, reports the omitted count as @@ -653,14 +663,42 @@ excluded from model context; the model sees diagnostics only in the original mut ## Limits and lifecycle Every LSP tool uses Pi's 2,000-line/50-KB output limit for model-visible text. Complete truncated text is saved as a -Result Spill and the result names its path. Each server's latest 1 MB of stderr is saved in the -session temp directory and failure messages name that path. With `--no-session`, Pi provides no -session directory, so these files use a private directory under the OS temporary directory instead. -Normal session teardown removes it; forced termination may leave temporary files behind. - -Documents must be valid UTF-8. Each server keeps at most 100 synchronized documents and closes -least-recently-used documents. Session shutdown requests a graceful LSP shutdown, then terminates -the process within the configured timeout. +Result Spill and the result names its path. Result Spills are saved in the session temp directory. +With `--no-session`, Pi provides no session directory, so they use a private directory under the +OS temporary directory instead. Normal session teardown removes it; forced termination may leave +temporary files behind. Each server's latest 1 MB of stderr is saved in a private directory of its +own under the OS temporary directory, and failure messages name that path. It belongs to the +Server Instance, not to a session, so it outlives the session that started the server and is +removed when the Instance stops. + +Server Instances are shared by every Pi session in one Pi process (main sessions, Minimal Subagents +children, Advisor sessions) whose Server Definition resolves to the same command, arguments, +environment, `initializationOptions`, `settings`, `lsp.timeouts`, and `maxConcurrentDiagnostics` +for the same workspace root, with the same pi-lsp version. Sessions that differ in any of these, +for example because only one trusts the project's settings, get their own Instances. There is no +opt-out, and Pi processes never share Instances. + +- A session takes a lease on an Instance the first time it routes a request to it, never at + startup, so a session that never touches code starts nothing. Sessions that need an Instance + that is still starting share its one startup, which continues while any of them waits for it. +- A session releases its leases when it shuts down or disables the Server Definition, and every + lease on an Instance ends when the Instance stops. The Instance stops as soon as its last lease + is released. +- A failure makes the Instance unavailable to every session using it. `/lsp stop` or `lsp_restart` + from any of them stops the process for all of them and clears the failure; each starts it again + lazily on its next use. +- Each Instance has one set of synchronized documents, diagnostics caches, capabilities, and + `maxConcurrentDiagnostics` limit, shared by its sessions. Workspace Edit Previews, Result + Spills, enablement overrides, Post-edit Diagnostics, and the roots that references and rename + warn about stay per session. +- A server-initiated edit (`workspace/applyEdit`) becomes a held Workspace Edit Preview in the + session using the Instance when only one session does. When two or more do, the edit cannot be + attributed to one of them, so the server is told it was not applied and no preview is created. + +Documents must be valid UTF-8. Each Server Instance keeps at most 100 synchronized documents, for +all the sessions sharing it, and closes least-recently-used documents, so one session's activity +can close another's documents, which only costs a re-open. Stopping an Instance requests a +graceful LSP shutdown, then terminates the process within the configured timeout. ## Security diff --git a/packages/pi-lsp/docs/adr/0005-share-server-instances-across-sessions.md b/packages/pi-lsp/docs/adr/0005-share-server-instances-across-sessions.md new file mode 100644 index 00000000..0c28bd9a --- /dev/null +++ b/packages/pi-lsp/docs/adr/0005-share-server-instances-across-sessions.md @@ -0,0 +1,45 @@ +# Share Server Instances across the sessions of one Pi process + +Every Pi session in one Pi process that loads pi-lsp (main sessions, Minimal Subagents children, Advisor sessions) now shares Server Instances through a pi-lsp-private, reference-counted pool. Before, each session's `session_start` built its own `LspServerManager`, which started its own processes and stopped them at `session_shutdown`. A child runs in its parent's process with the parent's working directory and project-trust snapshot, so every child that touched code cold-started its own copy of every language server for the same roots (#465). In this repository one `tsc --lsp` instance costs about 8–10 CPU-seconds to load the workspace and 650–900 MB of memory, and oxlint's server about 200 MB; a parent with three active children ran four of each. + +## The pool + +The pool lives on `globalThis` under the versioned slot `Symbol.for("@ian-pascoe/pi-lsp/server-instance-pool.v1")`, following the registry pattern of the repository's ADR-0008 and ADR-0009, because Pi evaluates extension modules again on `/reload` and for every in-process session, so module-level state would be duplicated. The slot holds only a `Map` of entries; the code that reads them is each pi-lsp copy's own. Only pi-lsp reads or writes the slot. No other package imports or depends on pi-lsp or its pool, and every package stays separately installable: sessions share Server Instances only because each of them loads pi-lsp. Sharing covers every session in the process, has no opt-out setting, and never crosses Pi processes. + +An entry's key is the identity of the process and its protocol behavior: + +- pi-lsp's package version, so copies of different released versions never share client code or entry shapes; +- the server ID and the workspace root; +- the resolved `command`, `args`, and `environment`; +- `initializationOptions` and `settings`; +- the resolved `lsp.timeouts`; +- `maxConcurrentDiagnostics`, so #464's limit is enforced once per shared process. + +Routing-only fields (`languages`, `rootMarkers`, `workspaceRootMarkers`, `requireRootMarker`), enablement, and `includeHintDiagnostics` are left out: they decide which files reach an instance or how results are filtered per session, not what the process is. Sessions whose keys differ, for example because only one trusts a project override, get separate instances without any warning. The environment and timeouts are serialized sorted by name; `initializationOptions` and `settings` keep the key order settings resolution produced, so a difference in their key order alone could only cause an extra instance, never wrong sharing. + +The version separates released versions, not two copies of the same version with different code, such as a linked development checkout loaded beside an installed release. Such copies share instances, and whichever started an instance supplies its client code. This is accepted: it affects only development setups, where a stale instance ends with a Stop, a restart, or the last session using it. + +## Server Instance Leases + +A session takes a Server Instance Lease the first time it routes a request to an instance, never at `session_start`, so a child that never touches code takes none. It releases the lease when it shuts down or disables the Server Definition, and every lease on an instance ends when the instance is stopped. The process stops as soon as its last lease is released. Sessions that route to an instance that is still starting all await its one startup; startup is aborted only when no lease remains, so it continues when the session that triggered it shuts down while another still holds a lease. + +Failure state belongs to the shared instance: a crash makes it unavailable to every leaseholder, and `/lsp stop` or `lsp_restart` from any session stops the process for all of them and clears the failure. Each leaseholder starts it again lazily on its next use, so Stop keeps its glossary meaning. Enablement stays a per-session routing filter. Project and global enablement writes are still not pushed to other running sessions. + +Per session stay the Server Enablement Overrides, the Workspace Edit Preview store, Result Spills and other session files, Pre-edit Baselines and Post-edit Diagnostics, and the routes the session has used; the other-roots warnings of references and rename come only from the asking session's routes. Per shared instance are the synchronized documents, their version counter and 100-document limit, the pull `resultId` cache and push-diagnostics state, the push-only silence memory, capabilities and dynamic registrations, failure state, and the diagnostics limit. One session's activity can evict another's documents, which only costs a re-open. Status reports, for each running instance, how many other sessions lease it; an instance a session holds no lease on reads `stopped` in that session. + +## Callbacks that were bound to one session + +The client's stderr file lived in the starting session's temp directory, which is deleted at its shutdown. The pool now owns it, in a private directory of its own under the OS temporary directory, and deletes it when the instance stops; failure messages cite that path. The unavailable callback and the startup abort signal belong to the pool entry. The function that starts a process captures no session state, because a startup can outlive the session that triggered it. + +A server-initiated `workspace/applyEdit` always arrives unsolicited, because pi-lsp never sends `workspace/executeCommand`. On a shared instance it becomes a held Workspace Edit Preview in the only leaseholder's session when there is exactly one. With two or more leaseholders the client replies `applied: false`, with a `failureReason` saying the shared Server Instance cannot attribute the edit to a session, and creates no preview anywhere: guessing would put an edit in the wrong session's store, where its agent would see a change it never asked for. The client passes its negotiated position encoding with the edit, so the preview no longer depends on the session's handle to the client. + +## Considered Options + +- **A module-level pool**: rejected because Pi evaluates the module again on `/reload` and for every in-process session, so each evaluation would have its own pool and nothing would be shared. +- **A home in pi-utils**: rejected because the pool holds pi-lsp clients and is pi-lsp policy. Putting it in a shared package would couple that package to pi-lsp's internals, while the `globalThis` slot already shares it between pi-lsp copies without any other package knowing it exists. +- **A grace period or idle timeout before stopping an unleased instance**: rejected to keep the lifecycle simple and resource use predictable; a later session pays one cold start. +- **Stop releasing only the caller's lease**: rejected because Stop is how a user recovers a failed instance. If it only released the caller's lease, a crashed instance would stay unavailable in every other session, and the process would keep running wherever it still had a lease. + +## Consequences + +A parent and its children that resolve the same configuration run one process per Server Definition and root instead of one per session. A Stop, a restart, or a crash in one session is felt in all of them, and in a busy parent with active children, documents opened by one session can be closed to make room for another's. Server-initiated edits are lost while two or more sessions share an instance; servers that rely on them, through commands pi-lsp never sends, were already outside its scope. diff --git a/packages/pi-lsp/skills/pi-lsp/SKILL.md b/packages/pi-lsp/skills/pi-lsp/SKILL.md index 52f4e07a..d4b95949 100644 --- a/packages/pi-lsp/skills/pi-lsp/SKILL.md +++ b/packages/pi-lsp/skills/pi-lsp/SKILL.md @@ -16,4 +16,4 @@ disable-model-invocation: true `lsp_status` is declared to the model. `lsp_capabilities` and `lsp_restart` are not declared by default. Call them from a `codemode` script (`await tools.lsp_capabilities({...})`) or load them with `tool_search`. If neither tool is active, ask the user to add the tools to `defaultTools` (for example `"+lsp_capabilities"`); the user can also run `/lsp` for status or `/lsp stop ` to recover a server. -`configured` has not routed a file yet; `stopped` permits lazy startup; `disabled` blocks startup. `unavailable` is sticky and retains stderr. Keep diagnosis read-only until a lifecycle change is authorized; stop before applying a Workspace Edit Preview. +`configured` has not routed a file yet; `stopped` permits lazy startup; `disabled` blocks startup. `unavailable` is sticky and retains stderr. A Server Instance can be shared with other Pi sessions in the process (`shared with N other sessions`); a Stop or restart ends it for all of them, and each starts it again on its next use. Keep diagnosis read-only until a lifecycle change is authorized; stop before applying a Workspace Edit Preview. diff --git a/packages/pi-lsp/src/lsp-command.ts b/packages/pi-lsp/src/lsp-command.ts index f9b75d15..1db732ab 100644 --- a/packages/pi-lsp/src/lsp-command.ts +++ b/packages/pi-lsp/src/lsp-command.ts @@ -38,9 +38,14 @@ export function notifyLspCommand( else process.stderr.write(`${text}\n`); } +/** How a status line counts the other Pi sessions leasing a running Server Instance. */ +export function lspSharedWithSessionsText(otherSessions: number): string { + return `shared with ${otherSessions} other session${otherSessions === 1 ? "" : "s"}`; +} + function statusLabel(server: LspServerStatusEntry, manager: CommandStatusManager): string { const enablement = manager.getEnablement(server.serverId); - return `${server.serverId} — ${server.state}${server.rootPath === undefined ? "" : ` — ${server.rootPath}`} — ${enablement.enabled ? "enabled" : "disabled"} (${enablement.scope})${server.error === undefined ? "" : ` — ${server.error}`}`; + return `${server.serverId} — ${server.state}${server.rootPath === undefined ? "" : ` — ${server.rootPath}`} — ${enablement.enabled ? "enabled" : "disabled"} (${enablement.scope})${server.error === undefined ? "" : ` — ${server.error}`}${server.sharedWithSessions === undefined ? "" : ` — ${lspSharedWithSessionsText(server.sharedWithSessions)}`}`; } /** Render command status without triggering lazy startup. */ diff --git a/packages/pi-lsp/src/lsp-server-client.ts b/packages/pi-lsp/src/lsp-server-client.ts index 44f1eb2e..5fa9d14e 100644 --- a/packages/pi-lsp/src/lsp-server-client.ts +++ b/packages/pi-lsp/src/lsp-server-client.ts @@ -87,6 +87,16 @@ const DOCUMENTATION_FORMATS = [MarkupKind.Markdown, MarkupKind.PlainText]; /** Time budgets, in milliseconds, for one language-server process. */ export type LspServerClientTimeouts = LspTimeouts; +/** + * Converts a server-initiated Workspace Edit, with the position encoding the server negotiated, + * into a held Workspace Edit Preview and returns its ID; a rejection becomes the reply's failure + * reason. + */ +export type LspServerWorkspaceEditHandler = ( + workspaceEdit: WorkspaceEdit, + positionEncoding: LspPositionEncoding, +) => Promise; + /** Launch and protocol configuration for one language-server process. */ export interface LspServerClientOptions { /** Stable configured server ID. */ @@ -110,10 +120,10 @@ export interface LspServerClientOptions { * wait in a first-in, first-out queue. Push-diagnostics waits are not requests and never queue. */ readonly maxConcurrentDiagnostics: number; - /** Mode-safe session file that retains the latest 1 MB of server stderr. */ + /** Mode-safe file that retains the latest 1 MB of server stderr; its directory is created if missing. */ readonly stderrPath: string; /** Convert a server-initiated edit into a Workspace Edit Preview. */ - readonly onWorkspaceEdit?: (workspaceEdit: WorkspaceEdit) => Promise; + readonly onWorkspaceEdit?: LspServerWorkspaceEditHandler; /** Mark the owning Server Instance unavailable after its first terminal process/protocol failure. */ readonly onUnavailable?: (error: LspServerClientError) => void; /** Cancel startup before initialization completes; running clients use shutdown instead. */ @@ -1331,7 +1341,10 @@ export class LspServerClient { return { applied: false, failureReason: "Pi LSP: workspace edit requires a preview" }; } try { - const previewId = await this.options.onWorkspaceEdit(parameters.edit); + const previewId = await this.options.onWorkspaceEdit( + parameters.edit, + this.positionEncodingValue, + ); return { applied: false, failureReason: `Pi LSP: workspace edit captured as preview ${previewId}`, diff --git a/packages/pi-lsp/src/lsp-server-instance-pool.ts b/packages/pi-lsp/src/lsp-server-instance-pool.ts new file mode 100644 index 00000000..99518bf6 --- /dev/null +++ b/packages/pi-lsp/src/lsp-server-instance-pool.ts @@ -0,0 +1,344 @@ +/** + * The process-wide pool of Server Instances that every Pi session running pi-lsp in one Pi process + * shares (ADR-0005). Its entries live on `globalThis` under a registered symbol, because Pi + * evaluates extension modules again on `/reload` and for every in-process Minimal Subagents child + * or Advisor session, so module-level state would not be shared. Only pi-lsp reads or writes the slot; no + * other package depends on it. + */ +import { readFileSync } from "node:fs"; +import { randomUUID } from "node:crypto"; +import { rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { Type } from "typebox"; +import { Value } from "typebox/value"; +import type { LspServerClient, LspServerWorkspaceEditHandler } from "./lsp-server-client.js"; +import type { LspServerDefinition, LspTimeouts } from "./pi-lsp-settings.js"; + +/** The versioned `globalThis` slot holding the shared pool's entries by Server Instance key. */ +export const LSP_SERVER_INSTANCE_POOL_SLOT = Symbol.for( + "@ian-pascoe/pi-lsp/server-instance-pool.v1", +); + +/** The lifecycle a pooled client must offer. */ +export interface LspPooledServerClient { + /** Gracefully stop the language-server process and release protocol resources. */ + shutdown(): Promise; +} + +/** What the pool provides to start one shared language-server process. */ +export interface LspPooledStartInput { + /** Aborted once no lease remains, cancelling a startup still in progress. */ + readonly signal: AbortSignal; + /** + * Pool-owned stderr file outside every session directory, in a private directory of its own + * that the client creates and the pool removes when the instance stops. + */ + readonly stderrPath: string; + /** Marks the shared instance unavailable to every leaseholder after a process or protocol failure. */ + readonly onUnavailable: (cause: unknown) => void; + /** + * Hands a server-initiated edit to the only leaseholder's session, or rejects it when two or + * more sessions hold leases, because the edit cannot be attributed to one of them. + */ + readonly onWorkspaceEdit: LspServerWorkspaceEditHandler; +} + +/** One session's side of a Server Instance Lease. */ +export interface LspServerInstanceLeaseHolder { + /** Create a held Workspace Edit Preview in this session for a server-initiated edit. */ + readonly onWorkspaceEdit: LspServerWorkspaceEditHandler; +} + +/** What a leaseholder gets when the shared startup has ended. */ +export type LspServerInstanceOutcome = + | { readonly kind: "running"; readonly client: TClient } + | { readonly kind: "unavailable"; readonly error: string } + | { readonly kind: "stopped" }; + +/** A leased instance's state as one session sees it. */ +export type LspServerInstanceLeaseState = "running" | "starting" | "stopped" | "unavailable"; + +interface LspPoolEntry { + readonly leases: Set>; + readonly lifetime: AbortController; + /** Settles, never rejecting, when startup has ended. */ + started: Promise; + settled: boolean; + client: TClient | undefined; + /** Failure that makes the instance unavailable to every leaseholder until it is stopped. */ + error: string | undefined; + /** + * Why the leases that waited for a previous instance's failed Stop ended; they fail with it + * instead of starting. A startup aborted by a Stop or the last release records nothing. + */ + stopError: string | undefined; + stderrDirectory: string | undefined; + /** Present once the instance is stopping; a later lease starts a new instance after it. */ + stopping: Promise | undefined; +} + +function describeError(cause: unknown): string { + return cause instanceof Error ? cause.message : String(cause); +} + +const PackageManifestSchema = Type.Object({ version: Type.String() }); + +/** The installed pi-lsp version, so copies of different versions never share client code. */ +const PI_LSP_VERSION = Value.Parse( + PackageManifestSchema, + JSON.parse(readFileSync(new URL("../package.json", import.meta.url), "utf8")), +).version; + +/** An object's entries sorted by name, so a key does not depend on the order they were merged in. */ +function sortedEntries(record: Readonly>): [string, T][] { + return Object.entries(record).sort(([left], [right]) => (left < right ? -1 : 1)); +} + +/** + * The identity of a shared Server Instance: the pi-lsp version, Server Definition ID, workspace + * root, and everything that shapes the process and its protocol behavior. Routing-only fields, + * enablement, and `includeHintDiagnostics` are left out. The environment and timeouts are sorted + * by name; `initializationOptions` and `settings` keep the order settings resolution produced, so + * identical settings give identical keys. The version does not separate two copies of the same + * version with different code, such as a linked development checkout (ADR-0005). + */ +export function lspServerInstanceKey( + definition: LspServerDefinition, + rootPath: string, + timeouts: LspTimeouts, +): string { + return JSON.stringify([ + PI_LSP_VERSION, + definition.id, + rootPath, + definition.command, + definition.args, + sortedEntries(definition.environment), + definition.initializationOptions ?? null, + definition.settings ?? null, + sortedEntries({ ...timeouts }), + definition.maxConcurrentDiagnostics, + ]); +} + +const UNATTRIBUTABLE_EDIT_REASON = + "the shared Server Instance cannot attribute the edit to a session"; + +/** Releases a lease from its pool entry. */ +interface LspLeaseOwner { + release(lease: LspServerInstanceLease): Promise; +} + +/** A Pi session's claim on a shared Server Instance; the instance runs while it has one. */ +export class LspServerInstanceLease { + /** Bind the lease to its pool entry; only the pool creates leases. */ + constructor( + private readonly entry: LspPoolEntry, + readonly holder: LspServerInstanceLeaseHolder, + private readonly owner: LspLeaseOwner, + ) {} + + /** Whether the lease still holds its instance: not released and the instance not stopped. */ + get active(): boolean { + return this.entry.leases.has(this); + } + + /** The running client, while the lease is active and the instance is available. */ + get client(): TClient | undefined { + const view = this.view(); + return view.kind === "running" ? view.client : undefined; + } + + /** How many other sessions hold a lease on the same instance. */ + get otherLeaseholders(): number { + return this.active ? this.entry.leases.size - 1 : 0; + } + + /** The instance's state as this lease sees it, without waiting. */ + get state(): LspServerInstanceLeaseState { + return this.view().kind; + } + + /** The latest unavailable reason, when the shared instance failed. */ + get error(): string | undefined { + const view = this.view(); + return view.kind === "unavailable" ? view.error : undefined; + } + + /** Wait for the shared startup, then report the client or why there is none. */ + async ready(): Promise> { + await this.entry.started; + const { stopError } = this.entry; + if (!this.active && stopError !== undefined) return { kind: "unavailable", error: stopError }; + const view = this.view(); + // Startup has ended, so the view is never `starting` here. + return view.kind === "starting" ? { kind: "stopped" } : view; + } + + /** The instance as this lease sees it now: the one cascade every accessor derives from. */ + private view(): LspServerInstanceOutcome | { readonly kind: "starting" } { + if (!this.active) return { kind: "stopped" }; + if (this.entry.error !== undefined) return { kind: "unavailable", error: this.entry.error }; + if (!this.entry.settled) return { kind: "starting" }; + const { client } = this.entry; + return client === undefined ? { kind: "stopped" } : { kind: "running", client }; + } + + /** Release this lease; the instance stops when it was the last one. */ + release(): Promise { + return this.owner.release(this); + } +} + +/** + * Reference-counted Server Instances by key. A lease is taken on a session's first use of an + * instance; sessions leasing an instance that is still starting share its one startup. The + * instance stops at once when its last lease is released, and a Stop from any session stops it + * for every leaseholder, who start it again lazily on their next use. + */ +export class LspServerInstancePool { + /** Wrap `entries`; a pool of its own unless given the shared slot's entries. */ + constructor(private readonly entries: Map> = new Map()) {} + + /** + * Lease the instance for `key`, starting it with `start` when no instance is running or + * starting. A new instance waits for a previous one with the same key to finish stopping. + */ + acquire( + key: string, + holder: LspServerInstanceLeaseHolder, + start: (input: LspPooledStartInput) => Promise, + ): LspServerInstanceLease { + const current = this.entries.get(key); + if (current !== undefined && current.stopping === undefined) { + return this.addLease(key, current, holder); + } + const entry: LspPoolEntry = { + leases: new Set(), + lifetime: new AbortController(), + started: Promise.resolve(), + settled: false, + client: undefined, + error: undefined, + stopError: undefined, + stderrDirectory: undefined, + stopping: undefined, + }; + this.entries.set(key, entry); + const lease = this.addLease(key, entry, holder); + entry.started = this.launch(key, entry, current?.stopping, start).finally(() => { + entry.settled = true; + }); + return lease; + } + + /** Stop the instance for `key` for every leaseholder; nothing happens when none exists. */ + stop(key: string): Promise { + const entry = this.entries.get(key); + return entry === undefined ? Promise.resolve() : this.stopEntry(key, entry); + } + + private addLease( + key: string, + entry: LspPoolEntry, + holder: LspServerInstanceLeaseHolder, + ): LspServerInstanceLease { + const lease = new LspServerInstanceLease(entry, holder, { + release: (released) => { + if (!entry.leases.delete(released)) return entry.stopping ?? Promise.resolve(); + return entry.leases.size === 0 ? this.stopEntry(key, entry) : Promise.resolve(); + }, + }); + entry.leases.add(lease); + return lease; + } + + private async launch( + key: string, + entry: LspPoolEntry, + previousStop: Promise | undefined, + start: (input: LspPooledStartInput) => Promise, + ): Promise { + const { signal } = entry.lifetime; + if (previousStop !== undefined) { + try { + await previousStop; + } catch (cause) { + // The leases waiting for a failed Stop fail with it, and the next use starts afresh. + entry.stopError = describeError(cause); + entry.leases.clear(); + if (this.entries.get(key) === entry) this.entries.delete(key); + return; + } + } + if (signal.aborted) return; + // A random name in the temporary directory, so no other process can predict or pre-create it. + entry.stderrDirectory = join(tmpdir(), `pi-lsp-server-${randomUUID()}`); + try { + const client = await start({ + signal, + stderrPath: join(entry.stderrDirectory, "stderr.log"), + onUnavailable: (cause) => { + if (!signal.aborted) entry.error = describeError(cause); + }, + onWorkspaceEdit: (edit, positionEncoding) => { + const [sole, ...others] = entry.leases; + if (sole === undefined || others.length > 0) { + return Promise.reject(new Error(UNATTRIBUTABLE_EDIT_REASON)); + } + return sole.holder.onWorkspaceEdit(edit, positionEncoding); + }, + }); + if (signal.aborted || entry.error !== undefined) { + await client.shutdown().catch(() => undefined); + return; + } + entry.client = client; + } catch (cause) { + // A startup a Stop or the last release aborted is not a failure of the instance. + if (!signal.aborted) entry.error = describeError(cause); + } + } + + private stopEntry(key: string, entry: LspPoolEntry): Promise { + if (entry.stopping !== undefined) return entry.stopping; + entry.leases.clear(); + entry.lifetime.abort(); + entry.stopping = (async () => { + try { + await entry.started; + await entry.client?.shutdown(); + } finally { + entry.client = undefined; + if (entry.stderrDirectory !== undefined) { + await rm(entry.stderrDirectory, { force: true, recursive: true }).catch(() => undefined); + } + if (this.entries.get(key) === entry) this.entries.delete(key); + } + })(); + return entry.stopping; + } +} + +function sharedEntries(): Map> { + const existing: unknown = Object.getOwnPropertyDescriptor( + globalThis, + LSP_SERVER_INSTANCE_POOL_SLOT, + )?.value; + // Every copy of pi-lsp writes this slot with a Map, and keys name the version that owns each + // entry, so a copy only ever reads entries of its own shape. + // oxlint-disable-next-line anti-slop/require-safety-comment-for-type-assertion -- SAFETY: the slot's key names its value type, only pi-lsp writes it, and entry keys include the owning version. + if (existing instanceof Map) return existing as Map>; + const created = new Map>(); + Object.defineProperty(globalThis, LSP_SERVER_INSTANCE_POOL_SLOT, { + value: created, + configurable: true, + }); + return created; +} + +/** The pool every Pi session in this process shares. */ +export function sharedLspServerInstancePool(): LspServerInstancePool { + return new LspServerInstancePool(sharedEntries()); +} diff --git a/packages/pi-lsp/src/lsp-server-manager.ts b/packages/pi-lsp/src/lsp-server-manager.ts index 7c2cb884..0fb33d65 100644 --- a/packages/pi-lsp/src/lsp-server-manager.ts +++ b/packages/pi-lsp/src/lsp-server-manager.ts @@ -10,8 +10,17 @@ import { resolve, sep, } from "node:path"; +import type { WorkspaceEdit } from "vscode-languageserver-protocol"; import { LspInputError } from "./lsp-input-error.js"; +import type { LspPositionEncoding } from "./lsp-position-encoding.js"; import { LspServerClientError } from "./lsp-server-client.js"; +import { + type LspPooledStartInput, + type LspServerInstanceLease, + type LspServerInstanceLeaseHolder, + lspServerInstanceKey, + LspServerInstancePool, +} from "./lsp-server-instance-pool.js"; import type { LspServerDefinition, LspServerEnablement, @@ -72,16 +81,15 @@ export interface LspManagedServerClient { shutdown(): Promise; } -/** Provides all parsed inputs needed to start one language-server process. */ -export interface LspServerStartInput { +/** + * Provides all parsed inputs needed to start one language-server process: the pool's stderr + * file, failure and edit callbacks, and abort signal, plus the instance's configuration. + */ +export interface LspServerStartInput extends LspPooledStartInput { /** Complete configured Server Definition. */ readonly definition: LspServerDefinition; - /** Marks a started instance unavailable after a process or protocol failure. */ - readonly onUnavailable: (cause: unknown) => void; /** Workspace root selected for this Server Instance. */ readonly rootPath: string; - /** Cancels initialization when the Instance is stopped before startup completes. */ - readonly signal: AbortSignal; /** Resolved request and lifecycle timeout policy. */ readonly timeouts: LspTimeouts; } @@ -177,7 +185,12 @@ export interface LspServerStatusEntry { readonly rootPath?: string; /** Configured server ID. */ readonly serverId: string; - /** Session lifecycle state. */ + /** How many other Pi sessions lease this running Server Instance; absent when none does. */ + readonly sharedWithSessions?: number; + /** + * Lifecycle state as this session sees it: an instance it holds no Server Instance Lease on + * is `stopped`, even while other sessions use it. + */ readonly state: "configured" | "disabled" | "running" | "starting" | "stopped" | "unavailable"; } @@ -253,6 +266,16 @@ export const LSP_ROOT_DISCOVERY_DIRECTORY_LIMIT = 4096; /** Directories root-marker discovery never descends into: installed dependencies. */ const ROOT_DISCOVERY_SKIPPED_DIRECTORIES: ReadonlySet = new Set(["node_modules"]); +/** A server-initiated Workspace Edit for this session to hold as a preview. */ +export interface LspServerWorkspaceEditInput { + /** Configured server ID of the instance that sent it. */ + readonly serverId: string; + /** The server's Workspace Edit, unvalidated. */ + readonly edit: WorkspaceEdit; + /** Position encoding the instance negotiated. */ + readonly positionEncoding: LspPositionEncoding; +} + /** Construction inputs for one session-scoped language-server manager. */ export interface LspServerManagerInput { /** Pi session working directory used for relative paths and root fallback. */ @@ -270,6 +293,16 @@ export interface LspServerManagerInput { readonly homeDirectory?: string; /** Most directories one root-marker discovery lists; defaults to `LSP_ROOT_DISCOVERY_DIRECTORY_LIMIT`. */ readonly rootDiscoveryDirectoryLimit?: number; + /** + * Server Instances shared with other sessions' managers; defaults to a pool of this manager's + * own, which shares nothing. + */ + readonly pool?: LspServerInstancePool; + /** + * Hold a server-initiated Workspace Edit as a preview in this session and return its ID; a + * session without it rejects such edits. + */ + readonly onWorkspaceEdit?: (input: LspServerWorkspaceEditInput) => Promise; } /** Removes Pi's optional leading path sigil before file routing. */ @@ -640,14 +673,15 @@ async function runServerOperation( } } -/** Own lazy Server Instance creation, routing, failure state, restart, and session shutdown. */ +/** + * Own one session's routing, enablement, known routes, and status, and its Server Instance + * Leases on the pool's lazily started, shared Server Instances. + */ export class LspServerManager { - private readonly clients = new Map(); - private readonly inFlightStarts = new Map>>(); - private readonly inFlightStops = new Map>(); private readonly knownRoutes = new Map(); - private readonly unavailable = new Map(); - private readonly instanceLifetimes = new Map(); + /** This session's Server Instance Lease for each known route that holds one. */ + private readonly leases = new Map>(); + private readonly pool: LspServerInstancePool; private closed = false; private configuredEnablement: ReadonlyMap; @@ -656,6 +690,7 @@ export class LspServerManager) { this.configuredEnablement = input.settings.enablement; + this.pool = input.pool ?? new LspServerInstancePool(); } /** Resolve eligibility independently of whether any Instance is running. */ @@ -668,7 +703,11 @@ export class LspServerManager, session: ReadonlyMap, @@ -676,9 +715,9 @@ export class LspServerManager !this.getEnablement(route.serverId).enabled) - .map((route) => this.stopServer(route.serverId, route.rootPath)), + [...this.knownRoutes.entries()] + .filter(([, route]) => !this.getEnablement(route.serverId).enabled) + .map(([key]) => this.releaseLease(key)), ); } @@ -698,18 +737,19 @@ export class LspServerManager 0) { + servers.push({ ...instance, sharedWithSessions: others, state }); } else { - servers.push({ ...instance, state: "stopped" }); + servers.push({ ...instance, state }); } } } @@ -877,7 +917,7 @@ export class LspServerManager(); for (const filePath of [ resolve(this.input.cwd, normalizeLspFilePath(loaded.queriedFilePath)), @@ -1070,47 +1110,55 @@ export class LspServerManager { const key = lspInstanceKey(serverId, resolve(this.input.cwd, normalizeLspFilePath(rootPath))); - if (!this.knownRoutes.has(key)) { + const route = this.knownRoutes.get(key); + if (route === undefined) { throw new Error(`Pi LSP: no known instance of ${serverId} for ${rootPath}`); } - const existingStop = this.inFlightStops.get(key); - if (existingStop !== undefined) return existingStop; - const lifetime = this.instanceLifetimes.get(key); - this.instanceLifetimes.delete(key); - lifetime?.abort(); - const client = this.clients.get(key); - this.clients.delete(key); - this.unavailable.delete(key); - const stop = (async () => { - await this.inFlightStarts.get(key); - await client?.shutdown(); - })(); - this.inFlightStops.set(key, stop); - try { - await stop; - } finally { - this.inFlightStops.delete(key); + const definition = this.input.settings.servers.get(serverId); + if (definition === undefined) { + // Without the definition there is no key to stop; still never leave this session's lease. + await this.releaseLease(key); + return; } + this.leases.delete(key); + await this.pool.stop(this.poolKey(definition, route)); } - /** Gracefully stop every client once and clear all session-scoped instance state. */ + /** Release every Server Instance Lease once and clear all session-scoped instance state. */ async shutdown(): Promise { this.closed = true; - await Promise.allSettled( - [...this.knownRoutes.values()].map((route) => - this.stopServer(route.serverId, route.rootPath), - ), - ); - this.clients.clear(); - this.inFlightStarts.clear(); - this.instanceLifetimes.clear(); - this.unavailable.clear(); + await Promise.allSettled([...this.leases.keys()].map((key) => this.releaseLease(key))); this.knownRoutes.clear(); } + /** Release this session's lease on one route's instance, which stops if it was the last. */ + private async releaseLease(key: string): Promise { + const lease = this.leases.get(key); + this.leases.delete(key); + await lease?.release(); + } + + private poolKey(definition: LspServerDefinition, route: LspServerRoute): string { + return lspServerInstanceKey(definition, route.rootPath, this.input.settings.timeouts); + } + + /** This session's side of a lease on one Server Definition's instance: its edits become previews here. */ + private leaseHolder(serverId: string): LspServerInstanceLeaseHolder { + const { onWorkspaceEdit } = this.input; + return { + onWorkspaceEdit: (edit, positionEncoding) => + onWorkspaceEdit === undefined + ? Promise.reject(new Error("this session holds no Workspace Edit Previews")) + : onWorkspaceEdit({ serverId, edit, positionEncoding }), + }; + } + /** * Select the routes a request may use: `serverId`'s route, or every enabled matching route. * Without `serverId`, when no route applies and every Server Definition handling the file's @@ -1167,105 +1215,60 @@ export class LspServerManager> { + private async ensureClient(route: LspServerRoute): Promise> { if (this.closed) { - return Promise.resolve({ + return { kind: "failure", failure: unavailableFailure(route, "session runtime is shut down"), - }); + }; } if (!this.getEnablement(route.serverId).enabled) { - return Promise.resolve({ + return { kind: "failure", failure: { code: "server-disabled", message: `Pi LSP: server ${route.serverId} is disabled; enable it with /lsp enable ${route.serverId} first`, serverId: route.serverId, }, - }); + }; } - const key = lspInstanceKey(route.serverId, route.rootPath); - const stopping = this.inFlightStops.get(key); - if (stopping !== undefined) { - return stopping.then( - () => this.ensureClient(route), - (cause): LspServerResolution => ({ - kind: "failure", - failure: unavailableFailure(route, describeLspError(cause)), - }), - ); + const definition = this.input.settings.servers.get(route.serverId); + if (definition === undefined) { + return { kind: "failure", failure: this.noMatchingFailure(route.serverId, route.rootPath) }; } + const key = lspInstanceKey(route.serverId, route.rootPath); this.knownRoutes.set(key, route); - const unavailableReason = this.unavailable.get(key); - if (unavailableReason !== undefined) { - return Promise.resolve({ - kind: "failure", - failure: unavailableFailure(route, unavailableReason), - }); - } - const client = this.clients.get(key); - const definition = this.input.settings.servers.get(route.serverId); - if (client !== undefined && definition !== undefined) { - return Promise.resolve({ - kind: "success", - instance: { client, definition, route }, - }); + let lease = this.leases.get(key); + if (lease?.active !== true) { + // The pool may run this startup after this session has shut down, so it captures only the + // instance's configuration and the start function, never the manager. + const { startClient, settings } = this.input; + const { rootPath } = route; + const { timeouts } = settings; + lease = this.pool.acquire( + this.poolKey(definition, route), + this.leaseHolder(route.serverId), + (start) => startClient({ ...start, definition, rootPath, timeouts }), + ); + this.leases.set(key, lease); } - const inFlight = this.inFlightStarts.get(key); - if (inFlight !== undefined) return inFlight; - if (definition === undefined) { - return Promise.resolve({ + const outcome = await lease.ready(); + if (this.closed) { + // A startup this session triggered may outlive it for other leaseholders, but not for it. + return { kind: "failure", - failure: this.noMatchingFailure(route.serverId, route.rootPath), - }); + failure: unavailableFailure(route, "session runtime is shut down"), + }; } - - const lifetime = new AbortController(); - this.instanceLifetimes.set(key, lifetime); - const start = this.startClient(key, route, definition, lifetime); - this.inFlightStarts.set(key, start); - void start.finally(() => { - if (this.inFlightStarts.get(key) === start) this.inFlightStarts.delete(key); - }); - return start; - } - - private async startClient( - key: string, - route: LspServerRoute, - definition: LspServerDefinition, - lifetime: AbortController, - ): Promise> { - try { - const client = await this.input.startClient({ - definition, - onUnavailable: (error) => { - if (this.instanceLifetimes.get(key) === lifetime) { - this.unavailable.set(key, describeLspError(error)); - } - }, - rootPath: route.rootPath, - signal: lifetime.signal, - timeouts: this.input.settings.timeouts, - }); - if (this.instanceLifetimes.get(key) !== lifetime) { - await client.shutdown(); - return { - kind: "failure", - failure: unavailableFailure(route, "Instance was stopped during startup"), - }; - } - const failure = this.unavailable.get(key); - if (failure !== undefined) { - await client.shutdown().catch(() => {}); - return { kind: "failure", failure: unavailableFailure(route, failure) }; - } - this.clients.set(key, client); - return { kind: "success", instance: { client, definition, route } }; - } catch (error) { - const message = describeLspError(error); - if (this.instanceLifetimes.get(key) === lifetime) this.unavailable.set(key, message); - return { kind: "failure", failure: unavailableFailure(route, message) }; + if (outcome.kind === "running") { + return { kind: "success", instance: { client: outcome.client, definition, route } }; } + return { + kind: "failure", + failure: unavailableFailure( + route, + outcome.kind === "unavailable" ? outcome.error : "Instance was stopped during startup", + ), + }; } } diff --git a/packages/pi-lsp/src/lsp-session-files.ts b/packages/pi-lsp/src/lsp-session-files.ts index 2c0114f6..8c58fd61 100644 --- a/packages/pi-lsp/src/lsp-session-files.ts +++ b/packages/pi-lsp/src/lsp-session-files.ts @@ -2,28 +2,24 @@ import { chmod, mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; -/** Owns private Result Spill and bounded stderr files for one Pi session. */ +/** + * Owns the private Result Spill files of one Pi session. Server stderr files belong to the + * shared Server Instance pool instead, because an instance can outlive the session that started it. + */ export interface LspSessionFiles { /** Private directory removed when the Pi session shuts down. */ readonly directoryPath: string; /** Write complete truncated tool output to a Result Spill file. */ writeResultSpill(output: string): Promise; - /** Create or return the bounded stderr file path for one language server. */ - getServerStderrPath(serverId: string): Promise; /** Remove all session files after queued writes finish. */ close(): Promise; } -interface ServerStderrFile { - readonly path: string; -} - class LspSessionFileStore implements LspSessionFiles { private closed = false; private closePromise: Promise | undefined; private nextFileIndex = 0; private writeQueue: Promise = Promise.resolve(); - private readonly stderrFiles = new Map(); /** Create a private file store rooted at the given session directory. */ constructor(readonly directoryPath: string) {} @@ -37,15 +33,6 @@ class LspSessionFileStore implements LspSessionFiles { }); } - getServerStderrPath(serverId: string): Promise { - const stderrFile = this.serverStderrFile(serverId); - return this.enqueueSessionFileWrite(async () => { - await writeFile(stderrFile.path, "", { mode: 0o600 }); - await chmod(stderrFile.path, 0o600); - return stderrFile.path; - }); - } - close(): Promise { if (this.closePromise !== undefined) return this.closePromise; this.closed = true; @@ -56,16 +43,6 @@ class LspSessionFileStore implements LspSessionFiles { return this.closePromise; } - private serverStderrFile(serverId: string): ServerStderrFile { - const existing = this.stderrFiles.get(serverId); - if (existing !== undefined) return existing; - const stderrFile = { - path: join(this.directoryPath, `server-stderr-${this.nextFileIndex++}.log`), - }; - this.stderrFiles.set(serverId, stderrFile); - return stderrFile; - } - private enqueueSessionFileWrite(write: () => Promise): Promise { if (this.closed) return Promise.reject(new Error("Pi LSP: session files are closed")); const result = this.writeQueue.then(write); @@ -77,7 +54,7 @@ class LspSessionFileStore implements LspSessionFiles { } } -/** Create a mode-safe temporary directory for Result Spills and language-server stderr files. */ +/** Create a mode-safe temporary directory for Result Spills. */ export async function createLspSessionFiles(sessionDirectory: string): Promise { const parentDirectory = sessionDirectory.length > 0 ? sessionDirectory : tmpdir(); await mkdir(parentDirectory, { mode: 0o700, recursive: true }); diff --git a/packages/pi-lsp/src/lsp-tool-contract.ts b/packages/pi-lsp/src/lsp-tool-contract.ts index c2ddf22f..3fad6cf8 100644 --- a/packages/pi-lsp/src/lsp-tool-contract.ts +++ b/packages/pi-lsp/src/lsp-tool-contract.ts @@ -675,6 +675,13 @@ export const LspStatusOutputSchema = Type.Object({ ]), root_path: Type.Optional(Type.String()), error: Type.Optional(Type.String()), + shared_with_sessions: Type.Optional( + Type.Integer({ + minimum: 1, + description: + "How many other Pi sessions in this process lease the running Server Instance; absent when none", + }), + ), languages: Type.Record(Type.String(), Type.Array(Type.String()), { description: "Language ID to the file extensions and exact filenames the Server Definition handles", diff --git a/packages/pi-lsp/src/lsp-tool.ts b/packages/pi-lsp/src/lsp-tool.ts index d07b07d9..227e971a 100644 --- a/packages/pi-lsp/src/lsp-tool.ts +++ b/packages/pi-lsp/src/lsp-tool.ts @@ -56,6 +56,7 @@ import { type ServerCapabilities, type TextDocumentPositionParams, } from "vscode-languageserver-protocol/node"; +import { lspSharedWithSessionsText } from "./lsp-command.js"; import { formatLspDiagnosticsReadText, isLspDiagnosticsOperation } from "./lsp-diagnostics-text.js"; import { formatLspHoverReadText } from "./lsp-hover-text.js"; import { LspInputError } from "./lsp-input-error.js"; @@ -1090,8 +1091,9 @@ function listedStatusServers(status: LspServerManagerStatus, all: boolean) { } /** - * Render status as one `server_id state [root] language(extensions,...)... [error: ...]` line per - * Server Definition or Server Instance, followed by settings warnings. Unless `all` is set, only + * Render status as one + * `server_id state [root] language(extensions,...)... [error: ...] [shared with N other sessions]` + * line per Server Definition or Server Instance, followed by settings warnings. Unless `all` is set, only * the `LISTED_STATES` get a line, and the enabled Server Definitions without a Server Instance * are counted in one summary line. */ @@ -1110,6 +1112,9 @@ function formatStatusText( ...(server.rootPath === undefined ? [] : [lspDisplayPath(cwd, server.rootPath)]), ...languages, ...(server.error === undefined ? [] : [`error: ${collapseLspWhitespace(server.error)}`]), + ...(server.sharedWithSessions === undefined + ? [] + : [lspSharedWithSessionsText(server.sharedWithSessions)]), ].join(" "); }); // Every omitted entry is a Server Definition without a Server Instance, so extra roots and @@ -2041,6 +2046,7 @@ async function executeLspOperation( languages: statusLanguages(server.languages), root_path: server.rootPath, server_id: server.serverId, + shared_with_sessions: server.sharedWithSessions, state: server.state, })), not_started: status.servers.length - listed.length, diff --git a/packages/pi-lsp/src/pi-lsp-extension.ts b/packages/pi-lsp/src/pi-lsp-extension.ts index d31634cc..7b71df92 100644 --- a/packages/pi-lsp/src/pi-lsp-extension.ts +++ b/packages/pi-lsp/src/pi-lsp-extension.ts @@ -18,7 +18,6 @@ import { Value } from "typebox/value"; import { DocumentDiagnosticRequest, DocumentSymbolRequest, - PositionEncodingKind, ReferencesRequest, type Diagnostic, } from "vscode-languageserver-protocol/node"; @@ -66,6 +65,7 @@ import { selectLspCommand, } from "./lsp-command.js"; import { LspServerClient } from "./lsp-server-client.js"; +import { sharedLspServerInstancePool } from "./lsp-server-instance-pool.js"; import { writeLspEnablement } from "./lsp-settings-store.js"; import { LspServerManager, @@ -540,9 +540,11 @@ export class PiLspLifecycleController { const manager = new LspServerManager({ cwd: context.cwd, settings, - startClient: async ({ definition, onUnavailable, rootPath, timeouts, signal }) => { - let client: LspServerClient | undefined; - client = await LspServerClient.start({ + pool: sharedLspServerInstancePool(), + // The pool may run this for a startup that outlives this session, so it captures none of + // the session's state: stderr, failures, and server edits go through the pool. + startClient: ({ definition, rootPath, timeouts, ...pooled }) => + LspServerClient.start({ serverId: definition.id, rootPath, command: definition.command, @@ -552,20 +554,10 @@ export class PiLspLifecycleController { settings: definition.settings ?? null, maxConcurrentDiagnostics: definition.maxConcurrentDiagnostics, timeouts, - signal, - stderrPath: await sessionFiles.getServerStderrPath(`${definition.id}\u0000${rootPath}`), - onUnavailable, - onWorkspaceEdit: async (edit) => - ( - await workspaceEdits.createPreview({ - edit, - serverId: definition.id, - positionEncoding: client?.positionEncoding ?? PositionEncodingKind.UTF16, - }) - ).preview_id, - }); - return client; - }, + ...pooled, + }), + onWorkspaceEdit: async ({ edit, serverId, positionEncoding }) => + (await workspaceEdits.createPreview({ edit, serverId, positionEncoding })).preview_id, }); this.session = { cwd: context.cwd, diff --git a/packages/pi-lsp/test/extension-harness.ts b/packages/pi-lsp/test/extension-harness.ts new file mode 100644 index 00000000..5057bf5a --- /dev/null +++ b/packages/pi-lsp/test/extension-harness.ts @@ -0,0 +1,197 @@ +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { resolve } from "node:path"; +import { + DefaultResourceLoader, + ExtensionRunner, + ModelRegistry, + ModelRuntime, + SessionManager, + SettingsManager, + type SessionShutdownEvent, + type SessionStartEvent, +} from "@earendil-works/pi-coding-agent"; +import { expect, vi } from "vitest"; +import { createPiLspExtension } from "../src/pi-lsp-extension.js"; +import type { LspSettingsDocumentInput } from "../src/pi-lsp-settings.js"; + +/** One Pi session running the pi-lsp extension through Pi's own resource loader and runner. */ +export interface ExtensionHarness { + readonly agentDirectory: string; + readonly notifications: string[]; + readonly resourceLoader: DefaultResourceLoader; + readonly runner: ExtensionRunner; + readonly sessionDirectory: string; + readonly sessionManager: SessionManager; + readonly settingsManager: SettingsManager; +} + +/** A project and Pi agent directory that several harnesses can share, as sessions in one process do. */ +export interface ExtensionHarnessDirectories { + readonly cwd: string; + readonly agentDirectory: string; +} + +const temporaryDirectories: string[] = []; + +/** Create a temporary directory that `removeHarnessDirectories` deletes. */ +export async function makeTemporaryDirectory(prefix: string): Promise { + const directory = await mkdtemp(resolve(tmpdir(), prefix)); + temporaryDirectories.push(directory); + return directory; +} + +/** Delete every directory the harnesses created. */ +export async function removeHarnessDirectories(): Promise { + await Promise.all( + temporaryDirectories + .splice(0) + .map((directory) => rm(directory, { force: true, recursive: true })), + ); +} + +/** + * Load the extension into a new Pi session. Without `directories`, the session gets a fresh + * project, whose untrusted `.pi/settings.json` holds an unknown `lsp` field, and a fresh agent + * directory holding `globalSettings`; with them, it joins that project and agent directory as is. + */ +export async function createExtensionHarness( + projectTrusted: boolean, + globalSettings: LspSettingsDocumentInput = {}, + noSession = false, + directories?: ExtensionHarnessDirectories, +): Promise { + const sessionDirectory = await makeTemporaryDirectory("pi-lsp-extension-sessions-"); + let cwd: string; + let agentDirectory: string; + if (directories === undefined) { + cwd = await makeTemporaryDirectory("pi-lsp-extension-cwd-"); + agentDirectory = await makeTemporaryDirectory("pi-lsp-extension-agent-"); + await writeFile(resolve(agentDirectory, "settings.json"), JSON.stringify(globalSettings)); + await mkdir(resolve(cwd, ".pi")); + await writeFile( + resolve(cwd, ".pi/settings.json"), + JSON.stringify({ lsp: { unknownField: true } }), + ); + } else { + ({ cwd, agentDirectory } = directories); + } + + if (noSession) { + for (const name of ["TMPDIR", "TMP", "TEMP"]) vi.stubEnv(name, sessionDirectory); + } + const sessionManager = noSession + ? SessionManager.inMemory(cwd) + : SessionManager.create(cwd, sessionDirectory); + const settingsManager = SettingsManager.create(cwd, agentDirectory, { projectTrusted }); + const resourceLoader = new DefaultResourceLoader({ + cwd, + agentDir: agentDirectory, + settingsManager, + extensionFactories: [ + { + name: "pi-lsp-lifecycle-test", + factory: createPiLspExtension({ getAgentDirectory: () => agentDirectory }), + }, + ], + noContextFiles: true, + noPromptTemplates: true, + noSkills: true, + noThemes: true, + }); + await resourceLoader.reload(); + const extensions = resourceLoader.getExtensions(); + expect(extensions.errors).toEqual([]); + expect(extensions.extensions).toHaveLength(1); + + const modelRuntime = await ModelRuntime.create({ + authPath: resolve(agentDirectory, "auth.json"), + modelsPath: null, + refreshOnCreate: false, + }); + const runner = new ExtensionRunner( + extensions.extensions, + extensions.runtime, + cwd, + sessionManager, + new ModelRegistry(modelRuntime), + ); + runner.bindCore( + { + sendMessage: () => undefined, + sendUserMessage: () => undefined, + appendEntry: (customType, data) => sessionManager.appendCustomEntry(customType, data), + setSessionName: (name) => sessionManager.appendSessionInfo(name), + getSessionName: () => sessionManager.getSessionName(), + setLabel: (entryId, label) => sessionManager.appendLabelChange(entryId, label), + getActiveTools: () => [], + getAllTools: () => [], + getSettings: () => ({}), + setActiveTools: () => undefined, + refreshTools: () => undefined, + getCommands: () => [], + setModel: async () => true, + getThinkingLevel: () => "medium", + setThinkingLevel: () => undefined, + }, + { + getModel: () => undefined, + getScopedModels: () => [], + isIdle: () => true, + isProjectTrusted: () => projectTrusted, + getSignal: () => undefined, + abort: () => undefined, + hasPendingMessages: () => false, + shutdown: () => undefined, + getContextUsage: () => undefined, + compact: () => undefined, + getSystemPrompt: () => "Pi LSP lifecycle test", + }, + ); + const notifications: string[] = []; + runner.setUIContext( + { + ...runner.getUIContext(), + notify: (message) => notifications.push(message), + }, + "rpc", + ); + return { + agentDirectory, + notifications, + resourceLoader, + runner, + sessionDirectory, + sessionManager, + settingsManager, + }; +} + +/** Deliver Pi's `session_start` for a startup. */ +export async function startExtension(harness: ExtensionHarness): Promise { + await harness.runner.emit({ + type: "session_start", + reason: "startup", + } satisfies SessionStartEvent); +} + +/** Deliver Pi's `session_shutdown` for a quit. */ +export async function shutdownExtension(harness: ExtensionHarness): Promise { + await harness.runner.emit({ + type: "session_shutdown", + reason: "quit", + } satisfies SessionShutdownEvent); +} + +/** The `globalThis` slot of the Server Instance pool every session in the process shares. */ +const SHARED_POOL_SLOT = Symbol.for("@ian-pascoe/pi-lsp/server-instance-pool.v1"); + +/** + * Forget the shared Server Instance pool, so the next test starts with an empty one, and return how + * many instances it still held: a test that shut down every session leaves none. + */ +export function resetSharedServerInstancePool(): number { + const slot: unknown = Object.getOwnPropertyDescriptor(globalThis, SHARED_POOL_SLOT)?.value; + Reflect.deleteProperty(globalThis, SHARED_POOL_SLOT); + return slot instanceof Map ? slot.size : 0; +} diff --git a/packages/pi-lsp/test/extension-session.ts b/packages/pi-lsp/test/extension-session.ts index 08895d04..0e18c255 100644 --- a/packages/pi-lsp/test/extension-session.ts +++ b/packages/pi-lsp/test/extension-session.ts @@ -10,6 +10,7 @@ import type { } from "@earendil-works/pi-coding-agent"; import type { LspSettingsDocumentInput } from "../src/pi-lsp-settings.js"; import { createPiLspExtension, type PiLspLifecycleEffects } from "../src/pi-lsp-extension.js"; +import { resetSharedServerInstancePool } from "./extension-harness.js"; /** The lifecycle events this harness delivers to the extension. */ type HarnessEvent = @@ -71,14 +72,19 @@ export interface ExtensionSessionOptions { const temporaryDirectories: string[] = []; const shutdowns: Array<() => Promise> = []; -/** Shut down every started session and remove its temporary directories. */ +/** + * Shut down every started session, reset the shared Server Instance pool, and remove the + * sessions' temporary directories. Throws when the pool still held an instance. + */ export async function closeExtensionSessions(): Promise { await Promise.all(shutdowns.splice(0).map((shutdown) => shutdown())); + const leftOver = resetSharedServerInstancePool(); await Promise.all( temporaryDirectories .splice(0) .map((directory) => rm(directory, { force: true, recursive: true })), ); + if (leftOver > 0) throw new Error(`${leftOver} Server Instances outlived their sessions`); } /** Drive the extension factory exactly as Pi does, in a fresh temporary project. */ diff --git a/packages/pi-lsp/test/fixtures/fake-lsp-server.mjs b/packages/pi-lsp/test/fixtures/fake-lsp-server.mjs index f3628e9a..19d7a87b 100644 --- a/packages/pi-lsp/test/fixtures/fake-lsp-server.mjs +++ b/packages/pi-lsp/test/fixtures/fake-lsp-server.mjs @@ -131,6 +131,23 @@ function diagnostics(message = "fake diagnostic", uri) { : []; } +/** Ask the client to apply an edit inserting "fake" at the start of `uri`, unsolicited. */ +function requestFakeEdit(uri) { + return sendRequest("workspace/applyEdit", { + label: "fake edit", + edit: { + changes: { + [uri]: [ + { + range: { start: { line: 0, character: 0 }, end: { line: 0, character: 0 } }, + newText: "fake", + }, + ], + }, + }, + }); +} + async function exerciseClientRequests() { const results = await Promise.all([ sendRequest("workspace/configuration", { @@ -142,19 +159,8 @@ async function exerciseClientRequests() { registrations: [{ id: "fake-folding", method: "textDocument/foldingRange" }], }), sendRequest("workspace/diagnostic/refresh"), - sendRequest("workspace/applyEdit", { - label: "fake edit", - edit: { - changes: { - "file:///fake.ts": [ - { - range: { start: { line: 0, character: 0 }, end: { line: 0, character: 0 } }, - newText: "fake", - }, - ], - }, - }, - }), + // FAKE_APPLY_EDIT_ON_HOVER moves the server-initiated edit from startup to each hover. + process.env.FAKE_APPLY_EDIT_ON_HOVER === "1" ? null : requestFakeEdit("file:///fake.ts"), ]); state.configuration = results[0]; state.workspaceFolders = results[1]; @@ -175,6 +181,10 @@ async function handleRequest(message) { } switch (message.method) { case "initialize": + // FAKE_INITIALIZE_DELAY_MS answers initialize only after that many milliseconds. + if (process.env.FAKE_INITIALIZE_DELAY_MS) { + await new Promise((done) => setTimeout(done, Number(process.env.FAKE_INITIALIZE_DELAY_MS))); + } state.rootUri = message.params?.rootUri ?? message.params?.workspaceFolders?.[0]?.uri ?? null; state.initializationOptions = message.params?.initializationOptions ?? null; state.textDocumentCapabilities = message.params?.capabilities?.textDocument ?? null; @@ -200,6 +210,20 @@ async function handleRequest(message) { capabilities, }); return; + case "textDocument/hover": { + // With FAKE_APPLY_EDIT_ON_HOVER, the hover text is the client's reply to an edit request + // for the hovered document, sent before the hover is answered. + const reply = + process.env.FAKE_APPLY_EDIT_ON_HOVER === "1" + ? await requestFakeEdit(message.params.textDocument.uri) + : null; + // FAKE_APPLY_EDIT_LOG names a file that receives each edit reply as one JSON line. + if (reply !== null && process.env.FAKE_APPLY_EDIT_LOG) { + appendFileSync(process.env.FAKE_APPLY_EDIT_LOG, `${JSON.stringify(reply)}\n`); + } + respond(message.id, { contents: { kind: "plaintext", value: JSON.stringify(reply) } }); + return; + } case "textDocument/documentSymbol": { // FAKE_SYMBOLS function declarations, one per line. const count = Number(process.env.FAKE_SYMBOLS ?? "1"); @@ -398,6 +422,11 @@ function parseInput() { } } +// FAKE_LAUNCH_LOG names a file that receives this process's ID, one line per launch. +if (process.env.FAKE_LAUNCH_LOG) { + appendFileSync(process.env.FAKE_LAUNCH_LOG, `${process.pid}\n`); +} + process.stdin.on("data", (chunk) => { input = Buffer.concat([input, chunk]); parseInput(); diff --git a/packages/pi-lsp/test/lsp-server-manager.test.ts b/packages/pi-lsp/test/lsp-server-manager.test.ts index 1c50abfd..b0b7651d 100644 --- a/packages/pi-lsp/test/lsp-server-manager.test.ts +++ b/packages/pi-lsp/test/lsp-server-manager.test.ts @@ -20,6 +20,7 @@ import type { LspServerEnablement, ResolvedLspSettings, } from "../src/pi-lsp-settings.js"; +import { LspServerInstancePool } from "../src/lsp-server-instance-pool.js"; import { testTimeouts } from "./lsp-timeouts.js"; const configuredServers: readonly LspServerRoutingDefinition[] = [ @@ -351,6 +352,36 @@ async function createRoutedFileFixture(): Promise<{ cwd: string; filePath: strin } describe("session-scoped LSP server manager", () => { + test("a Stop whose Server Definition is gone still releases the session's lease", async () => { + const { cwd, filePath } = await createRoutedFileFixture(); + const pool = new LspServerInstancePool(); + const factory = createRecordingClientFactory(); + const parentServers = new Map([["typescript", serverDefinition("typescript")]]); + const parent = new LspServerManager({ + cwd, + settings: { ...resolvedSettings([]), servers: parentServers }, + startClient: factory.start, + pool, + }); + const child = new LspServerManager({ + cwd, + settings: resolvedSettings(["typescript"]), + startClient: factory.start, + pool, + }); + await parent.getCapabilities("typescript", filePath); + await child.getCapabilities("typescript", filePath); + expect(child.getStatus().servers[0]).toMatchObject({ sharedWithSessions: 1 }); + + parentServers.delete("typescript"); + await parent.stopServer("typescript", resolve(cwd, "packages/example")); + + expect(child.getStatus().servers[0]?.sharedWithSessions).toBeUndefined(); + await child.shutdown(); + expect(factory.clients.map(({ shutdownCount }) => shutdownCount)).toEqual([1]); + await parent.shutdown(); + }); + test("stops only the selected root and starts it lazily on the next request", async () => { const { cwd, filePath } = await createRoutedFileFixture(); const otherRoot = resolve(cwd, "other"); @@ -866,7 +897,7 @@ describe("session-scoped LSP server manager", () => { await manager.setEnablement(new Map(), new Map([["typescript", false]])); await expect(request).resolves.toMatchObject({ kind: "failure", - failure: { message: expect.stringContaining("request cancelled") }, + failure: { message: expect.stringContaining("Instance was stopped during startup") }, }); const pid = Number(await readFile(stderrPath, "utf8")); expect(() => process.kill(pid, 0)).toThrow(); @@ -916,7 +947,7 @@ describe("session-scoped LSP server manager", () => { await disabling; await expect(request).resolves.toMatchObject({ kind: "failure", - failure: { message: expect.stringContaining("request cancelled") }, + failure: { message: expect.stringContaining("Instance was stopped during startup") }, }); expect(() => process.kill(processId, 0)).toThrow(); pid = undefined; diff --git a/packages/pi-lsp/test/lsp-session-files.test.ts b/packages/pi-lsp/test/lsp-session-files.test.ts index cd2fcdc0..275245ac 100644 --- a/packages/pi-lsp/test/lsp-session-files.test.ts +++ b/packages/pi-lsp/test/lsp-session-files.test.ts @@ -30,17 +30,10 @@ test.each(["persisted", "in-memory"])( }, ); -test("reserves empty mode-safe stderr paths and removes session files on shutdown", async () => { +test("rejects writes after removing session files on shutdown", async () => { const sessionDirectory = await mkdtemp(join(tmpdir(), "pi-lsp-session-")); const files = await createLspSessionFiles(sessionDirectory); try { - const emptyStderrPath = await files.getServerStderrPath("empty-server"); - const traversalStderrPath = await files.getServerStderrPath("../../outside-session-files"); - - expect(await readFile(emptyStderrPath)).toEqual(Buffer.alloc(0)); - expect(dirname(traversalStderrPath)).toBe(files.directoryPath); - expect((await stat(emptyStderrPath)).mode & 0o777).toBe(0o600); - await files.close(); await expect(stat(files.directoryPath)).rejects.toMatchObject({ code: "ENOENT" }); await expect(files.writeResultSpill("late output")).rejects.toThrow( diff --git a/packages/pi-lsp/test/lsp-shared-server-instances.test.ts b/packages/pi-lsp/test/lsp-shared-server-instances.test.ts new file mode 100644 index 00000000..07b560f8 --- /dev/null +++ b/packages/pi-lsp/test/lsp-shared-server-instances.test.ts @@ -0,0 +1,431 @@ +import { mkdir, readFile, stat, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { dirname, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import { Type } from "typebox"; +import { Value } from "typebox/value"; +import { afterEach, describe, expect, test } from "vitest"; +import { + createExtensionHarness, + type ExtensionHarness, + type ExtensionHarnessDirectories, + makeTemporaryDirectory, + removeHarnessDirectories, + resetSharedServerInstancePool, + shutdownExtension, + startExtension, +} from "./extension-harness.js"; +import { toToolContext } from "./tool-context.js"; + +const fakeServerPath = fileURLToPath(new URL("fixtures/fake-lsp-server.mjs", import.meta.url)); + +/** A project two sessions open, whose fake server records every launch. */ +interface SharedProject { + readonly directories: ExtensionHarnessDirectories; + readonly filePath: string; + /** Process IDs of the fake servers launched so far, in launch order. */ + launches(): Promise; +} + +async function createSharedProject( + environment: Readonly> = {}, + server: { readonly maxConcurrentDiagnostics?: number } = {}, +): Promise { + const cwd = await makeTemporaryDirectory("pi-lsp-shared-cwd-"); + const agentDirectory = await makeTemporaryDirectory("pi-lsp-shared-agent-"); + const launchLog = resolve(agentDirectory, "launches.log"); + await writeFile( + resolve(agentDirectory, "settings.json"), + JSON.stringify({ + lsp: { + timeouts: { diagnosticsMs: 2_000, initializeMs: 5_000, shutdownMs: 1_000 }, + servers: { + fake: { + command: process.execPath, + args: [fakeServerPath], + environment: { FAKE_LAUNCH_LOG: launchLog, ...environment }, + languages: [{ extensions: [".ts"], languageId: "typescript" }], + ...server, + }, + }, + }, + }), + ); + const filePath = resolve(cwd, "source.ts"); + await writeFile(filePath, "const value = 1;\n"); + return { + directories: { cwd, agentDirectory }, + filePath, + launches: async () => + (await readFile(launchLog, "utf8").catch(() => "")) + .split("\n") + .filter((line) => line !== "") + .map(Number), + }; +} + +const openSessions: ExtensionHarness[] = []; + +async function openSession(project: SharedProject, projectTrusted = false) { + const harness = await createExtensionHarness(projectTrusted, {}, false, project.directories); + openSessions.push(harness); + await startExtension(harness); + return harness; +} + +async function runTool( + harness: ExtensionHarness, + name: string, + parameters: Readonly>, +) { + const tool = harness.runner.getToolDefinition(name); + if (tool === undefined) throw new Error(`Expected ${name}`); + return tool.execute( + name, + parameters, + undefined, + undefined, + toToolContext(harness.runner.createContext()), + ); +} + +async function runCommand(harness: ExtensionHarness, args: string): Promise { + const command = harness.runner.getCommand("lsp"); + if (command === undefined) throw new Error("Expected /lsp"); + await command.handler(args, harness.runner.createCommandContext()); +} + +/** The `lsp_status` text and the Structured Result's first server entry. */ +async function status(harness: ExtensionHarness) { + const result = await runTool(harness, "lsp_status", {}); + const [part] = result.content; + return { + text: part?.type === "text" ? part.text : "", + server: Value.Parse(StatusServersSchema, result.structuredContent).servers[0], + }; +} + +const StatusServersSchema = Type.Object( + { + servers: Type.Array( + Type.Object( + { + state: Type.String(), + error: Type.Optional(Type.String()), + shared_with_sessions: Type.Optional(Type.Integer()), + }, + { additionalProperties: true }, + ), + ), + }, + { additionalProperties: true }, +); + +function isRunning(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +/** + * Open a parent and a child session on `project` and route both to its fake server, returning the + * process ID of the one launch they share. + */ +async function openLeasingSessions(project: SharedProject) { + const parent = await openSession(project); + const child = await openSession(project); + const start = { server_id: "fake", file_path: project.filePath }; + await runTool(parent, "lsp_capabilities", start); + await runTool(child, "lsp_capabilities", start); + const launches = await project.launches(); + const [pid] = launches; + if (pid === undefined) throw new Error("Expected a launch"); + return { parent, child, start, launches, pid }; +} + +afterEach(async () => { + await Promise.all(openSessions.splice(0).map((harness) => shutdownExtension(harness))); + const leftOver = resetSharedServerInstancePool(); + await removeHarnessDirectories(); + expect(leftOver).toBe(0); +}); + +describe("Server Instances shared across sessions in one Pi process", () => { + test("two sessions with one cwd and identical settings launch one process per Server Definition and root", async () => { + const project = await createSharedProject(); + + const { launches } = await openLeasingSessions(project); + + expect(launches).toHaveLength(1); + }); + + test("a project override only one session sees gets its own process", async () => { + const project = await createSharedProject(); + const { cwd, agentDirectory } = project.directories; + const globalSettings = JSON.parse( + await readFile(resolve(agentDirectory, "settings.json"), "utf8"), + ); + await mkdir(resolve(cwd, ".pi")); + await writeFile( + resolve(cwd, ".pi/settings.json"), + JSON.stringify({ + lsp: { + servers: { + fake: { ...globalSettings.lsp.servers.fake, settings: { project: true } }, + }, + }, + }), + ); + const trusting = await openSession(project, true); + const untrusting = await openSession(project, false); + const start = { server_id: "fake", file_path: project.filePath }; + + await runTool(trusting, "lsp_capabilities", start); + await runTool(untrusting, "lsp_capabilities", start); + + expect(await project.launches()).toHaveLength(2); + }); + test("sessions routing to a cold instance at once share one launch", async () => { + const project = await createSharedProject({ FAKE_INITIALIZE_DELAY_MS: "200" }); + const parent = await openSession(project); + const child = await openSession(project); + const start = { server_id: "fake", file_path: project.filePath }; + + await Promise.all([ + runTool(parent, "lsp_capabilities", start), + runTool(child, "lsp_capabilities", start), + ]); + + expect(await project.launches()).toHaveLength(1); + }); + + test("a startup continues when the session that triggered it shuts down while another holds a lease", async () => { + const project = await createSharedProject({ FAKE_INITIALIZE_DELAY_MS: "300" }); + const parent = await openSession(project); + const child = await openSession(project); + const start = { server_id: "fake", file_path: project.filePath }; + + const triggered = runTool(parent, "lsp_capabilities", start); + triggered.catch(() => undefined); + await expect.poll(() => project.launches()).toHaveLength(1); + const joined = runTool(child, "lsp_capabilities", start); + await expect.poll(async () => (await status(child)).server?.state).toBe("starting"); + await shutdownExtension(parent); + + await expect(joined).resolves.toMatchObject({ structuredContent: { server_id: "fake" } }); + await expect(triggered).rejects.toThrow("session runtime is shut down"); + expect(await project.launches()).toHaveLength(1); + expect((await status(child)).server?.state).toBe("running"); + }); + + test("a restart during a shared startup reports a fresh running instance, not the stopped startup's failure", async () => { + const project = await createSharedProject({ FAKE_INITIALIZE_DELAY_MS: "300" }); + const parent = await openSession(project); + const child = await openSession(project); + const start = { server_id: "fake", file_path: project.filePath }; + + const starting = runTool(parent, "lsp_capabilities", start); + starting.catch(() => undefined); + // The first process is launched and waiting to answer initialize. + await expect.poll(() => project.launches()).toHaveLength(1); + const restarted = await runTool(child, "lsp_restart", start); + + expect(restarted.structuredContent).toMatchObject({ server_id: "fake" }); + await expect(starting).rejects.toThrow("Instance was stopped during startup"); + expect((await status(child)).server).toMatchObject({ state: "running" }); + expect((await status(child)).server?.error).toBeUndefined(); + await runTool(parent, "lsp_capabilities", start); + expect((await status(parent)).server).toMatchObject({ + state: "running", + shared_with_sessions: 1, + }); + expect(await project.launches()).toHaveLength(2); + }); + + test("an instance stops when its last leaseholder shuts down, and a session that never routes to it takes no lease", async () => { + const project = await createSharedProject(); + const { parent, child, pid } = await openLeasingSessions(project); + const bystander = await openSession(project); + + await shutdownExtension(child); + expect(isRunning(pid)).toBe(true); + expect((await status(parent)).server).toMatchObject({ state: "running" }); + expect((await status(parent)).server?.shared_with_sessions).toBeUndefined(); + + await shutdownExtension(parent); + expect(isRunning(pid)).toBe(false); + expect((await status(bystander)).text).toBe( + "No Server Instances started.\n+1 configured, not started (pass all: true to list)", + ); + }); + + test("/lsp stop in one session stops the process for both, and each relaunches it lazily", async () => { + const project = await createSharedProject(); + const { parent, child, start, pid } = await openLeasingSessions(project); + + await runCommand(parent, `stop fake ${JSON.stringify(project.directories.cwd)}`); + + expect(isRunning(pid)).toBe(false); + expect((await status(parent)).server?.state).toBe("stopped"); + expect((await status(child)).server?.state).toBe("stopped"); + await runTool(child, "lsp_capabilities", start); + await runTool(parent, "lsp_capabilities", start); + expect(await project.launches()).toHaveLength(2); + expect((await status(parent)).server).toMatchObject({ + state: "running", + shared_with_sessions: 1, + }); + }); + + test("a crash makes the instance unavailable in both sessions, and lsp_restart from either recovers both", async () => { + const project = await createSharedProject(); + const { parent, child, start, pid } = await openLeasingSessions(project); + + process.kill(pid, "SIGKILL"); + + await expect.poll(async () => (await status(parent)).server?.state).toBe("unavailable"); + expect((await status(child)).server?.state).toBe("unavailable"); + await expect(runTool(parent, "lsp_capabilities", start)).rejects.toThrow("unavailable"); + await runTool(child, "lsp_restart", start); + await runTool(parent, "lsp_capabilities", start); + expect(await project.launches()).toHaveLength(2); + expect((await status(parent)).server?.state).toBe("running"); + }); + + test("disabling the Server Definition in one session releases only its lease", async () => { + const project = await createSharedProject(); + const { parent, child, start, pid } = await openLeasingSessions(project); + + await runCommand(parent, "disable fake"); + + expect(isRunning(pid)).toBe(true); + expect((await status(parent)).server?.state).toBe("disabled"); + await runTool(child, "lsp_capabilities", start); + expect(await project.launches()).toHaveLength(1); + expect((await status(child)).server).toMatchObject({ state: "running" }); + expect((await status(child)).server?.shared_with_sessions).toBeUndefined(); + }); + + test("a server-initiated edit becomes a preview only in a sole leaseholder's session", async () => { + const replyLog = resolve(await makeTemporaryDirectory("pi-lsp-shared-edits-"), "replies.log"); + const project = await createSharedProject({ + FAKE_APPLY_EDIT_ON_HOVER: "1", + FAKE_APPLY_EDIT_LOG: replyLog, + }); + const parent = await openSession(project); + const child = await openSession(project); + const hover = { file_path: project.filePath, line: 1, character: 1 }; + + const sole = await runTool(parent, "lsp_hover", hover); + const [previewId] = Value.Parse( + Type.Object({ server_preview_ids: Type.Array(Type.String()) }), + sole.structuredContent, + ).server_preview_ids; + expect(JSON.stringify(sole.content)).toContain("workspace edit captured as preview"); + + const shared = await runTool(child, "lsp_hover", hover); + expect(JSON.stringify(shared.content)).toContain( + "the shared Server Instance cannot attribute the edit to a session", + ); + expect(shared.structuredContent).not.toHaveProperty("server_preview_ids"); + const parentNext = await runTool(parent, "lsp_status", {}); + expect(parentNext.structuredContent).not.toHaveProperty("server_preview_ids"); + const replies = (await readFile(replyLog, "utf8")) + .split("\n") + .filter((line) => line !== "") + .map((line) => JSON.parse(line)); + expect(replies).toEqual([ + { + applied: false, + failureReason: `Pi LSP: workspace edit captured as preview ${previewId}`, + }, + { + applied: false, + failureReason: + "Pi LSP: workspace edit preview rejected: the shared Server Instance cannot attribute the edit to a session", + }, + ]); + }); + + test("a shared instance's stderr file outlives the session that started it and is removed when the instance stops", async () => { + const project = await createSharedProject({ FAKE_STDERR_BYTES: "16" }); + const { parent, child, pid } = await openLeasingSessions(project); + + await shutdownExtension(parent); + process.kill(pid, "SIGKILL"); + await expect.poll(async () => (await status(child)).server?.state).toBe("unavailable"); + const stderrPath = /stderr (\S+)\)/u.exec((await status(child)).server?.error ?? "")?.[1]; + if (stderrPath === undefined) throw new Error("Expected the error to cite the stderr file"); + expect(stderrPath.startsWith(parent.sessionDirectory)).toBe(false); + expect(dirname(dirname(stderrPath))).toBe(tmpdir()); + expect((await stat(dirname(stderrPath))).mode & 0o777).toBe(0o700); + expect((await stat(stderrPath)).mode & 0o777).toBe(0o600); + expect(await readFile(stderrPath, "utf8")).toContain("END"); + + await runCommand(child, `stop fake ${JSON.stringify(project.directories.cwd)}`); + await expect(stat(stderrPath)).rejects.toMatchObject({ code: "ENOENT" }); + }); + + test("lsp_status and /lsp count the other sessions leasing a running instance", async () => { + const project = await createSharedProject(); + const parent = await openSession(project); + const child = await openSession(project); + const start = { server_id: "fake", file_path: project.filePath }; + await runTool(parent, "lsp_capabilities", start); + const single = await status(parent); + const { cwd } = project.directories; + expect(single.text).toBe(`fake running ${cwd} typescript(.ts)`); + expect(single.server).not.toHaveProperty("shared_with_sessions"); + + await runTool(child, "lsp_capabilities", start); + + for (const session of [parent, child]) { + const shared = await status(session); + expect(shared.text).toBe(`fake running ${cwd} typescript(.ts) shared with 1 other session`); + expect(shared.server?.shared_with_sessions).toBe(1); + } + const rows: string[][] = []; + parent.runner.setUIContext( + { + ...parent.runner.getUIContext(), + select: async (_title, options) => { + rows.push(options); + return undefined; + }, + }, + "rpc", + ); + await runCommand(parent, ""); + expect(rows).toEqual([ + [expect.stringMatching(/^fake — running — .* — shared with 1 other session$/u)], + ]); + await shutdownExtension(child); + expect((await status(parent)).text).toBe(single.text); + }); + + test("the diagnostics limit holds across sessions sharing one process", async () => { + const peakLog = resolve(await makeTemporaryDirectory("pi-lsp-shared-peak-"), "peak.log"); + // The server answers pulls only, each after a delay, so concurrent pulls overlap and are counted. + const project = await createSharedProject( + { FAKE_PUSH: "none", FAKE_DIAGNOSTIC_LATENCY_MS: "150", FAKE_PEAK_LOG: peakLog }, + { maxConcurrentDiagnostics: 1 }, + ); + const otherFile = resolve(project.directories.cwd, "other.ts"); + await writeFile(otherFile, "const other = 2;\n"); + const parent = await openSession(project); + const child = await openSession(project); + + await Promise.all([ + runTool(parent, "lsp_diagnostics", { file_path: project.filePath }), + runTool(child, "lsp_diagnostics", { file_path: otherFile }), + runTool(parent, "lsp_diagnostics", { file_path: otherFile }), + runTool(child, "lsp_diagnostics", { file_path: project.filePath }), + ]); + + expect(await project.launches()).toHaveLength(1); + expect(await readFile(peakLog, "utf8")).toBe("1"); + }); +}); diff --git a/packages/pi-lsp/test/lsp-tool-output.test.ts b/packages/pi-lsp/test/lsp-tool-output.test.ts index 40bb693d..1ad0af9c 100644 --- a/packages/pi-lsp/test/lsp-tool-output.test.ts +++ b/packages/pi-lsp/test/lsp-tool-output.test.ts @@ -87,7 +87,6 @@ function memorySessionFiles() { spills.push(output); return Promise.resolve(`/tmp/spill/${spills.length - 1}.txt`); }, - getServerStderrPath: () => Promise.resolve("/tmp/spill/stderr.log"), close: () => Promise.resolve(), }; return { sessionFiles, spills }; diff --git a/packages/pi-lsp/test/pi-lsp-extension.test.ts b/packages/pi-lsp/test/pi-lsp-extension.test.ts index cdf4d644..bd569596 100644 --- a/packages/pi-lsp/test/pi-lsp-extension.test.ts +++ b/packages/pi-lsp/test/pi-lsp-extension.test.ts @@ -1,5 +1,5 @@ import { toToolContext } from "./tool-context.js"; -import { mkdir, mkdtemp, readFile, readdir, rm, writeFile } from "node:fs/promises"; +import { mkdir, readFile, readdir, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { resolve } from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; @@ -8,25 +8,13 @@ import { getModel } from "@earendil-works/pi-ai/compat"; import { type AgentSession, createAgentSession, - DefaultResourceLoader, - ExtensionRunner, initTheme, - ModelRegistry, - ModelRuntime, - SessionManager, - SettingsManager, - type SessionShutdownEvent, - type SessionStartEvent, type ToolResultEvent, type TurnEndEvent, } from "@earendil-works/pi-coding-agent"; import { afterEach, describe, expect, test, vi } from "vitest"; import { expectClickToggles } from "@ian-pascoe/pi-utils/ui-testing"; -import { - createPiLspExtension, - failureDiagnosticOutcome, - normalizedDiagnosticOutcome, -} from "../src/pi-lsp-extension.js"; +import { failureDiagnosticOutcome, normalizedDiagnosticOutcome } from "../src/pi-lsp-extension.js"; import { appendPostEditDiagnostics } from "../src/lsp-post-edit-diagnostics.js"; import { POST_EDIT_DIAGNOSTICS_ENTRY_TYPE } from "../src/lsp-post-edit-diagnostics-rendering.js"; import { LspWorkspaceEditStore } from "../src/lsp-workspace-edit.js"; @@ -35,9 +23,14 @@ import { LSP_OPERATION_NAMES, type LspWorkspaceEditPreviewRecord, } from "../src/lsp-tool-contract.js"; -import type { LspSettingsDocumentInput } from "../src/pi-lsp-settings.js"; +import { + createExtensionHarness, + removeHarnessDirectories, + resetSharedServerInstancePool, + shutdownExtension, + startExtension, +} from "./extension-harness.js"; -const temporaryDirectories: string[] = []; const agentSessions: AgentSession[] = []; const typescriptSettings = { @@ -51,141 +44,6 @@ const typescriptSettings = { }, }; -interface ExtensionHarness { - readonly agentDirectory: string; - readonly notifications: string[]; - readonly resourceLoader: DefaultResourceLoader; - readonly runner: ExtensionRunner; - readonly sessionDirectory: string; - readonly sessionManager: SessionManager; - readonly settingsManager: SettingsManager; -} - -async function makeTemporaryDirectory(prefix: string): Promise { - const directory = await mkdtemp(resolve(tmpdir(), prefix)); - temporaryDirectories.push(directory); - return directory; -} - -async function createExtensionHarness( - projectTrusted: boolean, - globalSettings: LspSettingsDocumentInput = {}, - noSession = false, -): Promise { - const cwd = await makeTemporaryDirectory("pi-lsp-extension-cwd-"); - const agentDirectory = await makeTemporaryDirectory("pi-lsp-extension-agent-"); - const sessionDirectory = await makeTemporaryDirectory("pi-lsp-extension-sessions-"); - await writeFile(resolve(agentDirectory, "settings.json"), JSON.stringify(globalSettings)); - await mkdir(resolve(cwd, ".pi")); - await writeFile( - resolve(cwd, ".pi/settings.json"), - JSON.stringify({ lsp: { unknownField: true } }), - ); - - if (noSession) { - for (const name of ["TMPDIR", "TMP", "TEMP"]) vi.stubEnv(name, sessionDirectory); - } - const sessionManager = noSession - ? SessionManager.inMemory(cwd) - : SessionManager.create(cwd, sessionDirectory); - const settingsManager = SettingsManager.create(cwd, agentDirectory, { projectTrusted }); - const resourceLoader = new DefaultResourceLoader({ - cwd, - agentDir: agentDirectory, - settingsManager, - extensionFactories: [ - { - name: "pi-lsp-lifecycle-test", - factory: createPiLspExtension({ getAgentDirectory: () => agentDirectory }), - }, - ], - noContextFiles: true, - noPromptTemplates: true, - noSkills: true, - noThemes: true, - }); - await resourceLoader.reload(); - const extensions = resourceLoader.getExtensions(); - expect(extensions.errors).toEqual([]); - expect(extensions.extensions).toHaveLength(1); - - const modelRuntime = await ModelRuntime.create({ - authPath: resolve(agentDirectory, "auth.json"), - modelsPath: null, - refreshOnCreate: false, - }); - const runner = new ExtensionRunner( - extensions.extensions, - extensions.runtime, - cwd, - sessionManager, - new ModelRegistry(modelRuntime), - ); - runner.bindCore( - { - sendMessage: () => undefined, - sendUserMessage: () => undefined, - appendEntry: (customType, data) => sessionManager.appendCustomEntry(customType, data), - setSessionName: (name) => sessionManager.appendSessionInfo(name), - getSessionName: () => sessionManager.getSessionName(), - setLabel: (entryId, label) => sessionManager.appendLabelChange(entryId, label), - getActiveTools: () => [], - getAllTools: () => [], - getSettings: () => ({}), - setActiveTools: () => undefined, - refreshTools: () => undefined, - getCommands: () => [], - setModel: async () => true, - getThinkingLevel: () => "medium", - setThinkingLevel: () => undefined, - }, - { - getModel: () => undefined, - getScopedModels: () => [], - isIdle: () => true, - isProjectTrusted: () => projectTrusted, - getSignal: () => undefined, - abort: () => undefined, - hasPendingMessages: () => false, - shutdown: () => undefined, - getContextUsage: () => undefined, - compact: () => undefined, - getSystemPrompt: () => "Pi LSP lifecycle test", - }, - ); - const notifications: string[] = []; - runner.setUIContext( - { - ...runner.getUIContext(), - notify: (message) => notifications.push(message), - }, - "rpc", - ); - return { - agentDirectory, - notifications, - resourceLoader, - runner, - sessionDirectory, - sessionManager, - settingsManager, - }; -} - -async function startExtension(harness: ExtensionHarness): Promise { - await harness.runner.emit({ - type: "session_start", - reason: "startup", - } satisfies SessionStartEvent); -} - -async function shutdownExtension(harness: ExtensionHarness): Promise { - await harness.runner.emit({ - type: "session_shutdown", - reason: "quit", - } satisfies SessionShutdownEvent); -} - function completedAssistantMessage(): AssistantMessage { return { role: "assistant", @@ -219,11 +77,9 @@ afterEach(async () => { session.dispose(); } } - await Promise.all( - temporaryDirectories - .splice(0) - .map((directory) => rm(directory, { force: true, recursive: true })), - ); + const leftOver = resetSharedServerInstancePool(); + await removeHarnessDirectories(); + expect(leftOver).toBe(0); }); describe("Pi LSP extension lifecycle", () => {