From 665fdeeee147f21800fb262c803db2565e48aa5c Mon Sep 17 00:00:00 2001 From: Jonathan McPherson Date: Fri, 17 Jul 2026 13:38:33 -0700 Subject: [PATCH 1/2] lazy start Quarto LSP --- apps/vscode/src/lsp/client.ts | 191 +++++++++++++++++---- apps/vscode/src/main.ts | 6 +- apps/vscode/src/providers/editor/editor.ts | 12 +- apps/vscode/src/providers/zotero/zotero.ts | 48 +++--- apps/vscode/src/test/r-project.test.ts | 8 + apps/vscode/src/test/symbols.test.ts | 9 +- apps/vscode/src/test/test-utils.ts | 43 +++++ 7 files changed, 248 insertions(+), 69 deletions(-) diff --git a/apps/vscode/src/lsp/client.ts b/apps/vscode/src/lsp/client.ts index 6737fb3bd..9d46deadd 100644 --- a/apps/vscode/src/lsp/client.ts +++ b/apps/vscode/src/lsp/client.ts @@ -29,6 +29,8 @@ import { DocumentSymbol, Range, SymbolKind, + Disposable, + languages, } from "vscode"; import { LanguageClient, @@ -72,19 +74,59 @@ import { embeddedSemanticTokensProvider } from "../providers/semantic-tokens"; import { getHover, getSignatureHelpHover } from "../core/hover"; import { imageHover } from "../providers/hover-image"; import { LspInitializationOptions, QuartoContext } from "quarto-core"; +import { lspClientTransport } from "core-node"; +import { JsonRpcRequestTransport } from "core"; import { extensionHost } from "../host"; import semver from "semver"; import { EmbeddedLanguage } from "../vdoc/languages"; import { SymbolInformation } from "vscode"; -let client: LanguageClient; +// The active language client. Retained at module scope so `deactivate` can stop +// it. It is created eagerly during activation but the underlying server process +// is not spawned until the client is actually needed (see `ensureStarted`). +let client: LanguageClient | undefined; -export async function activateLsp( +/** + * Handle returned by {@link activateLsp}. It lets consumers talk to the Quarto + * language server without forcing it to start at extension activation. The + * server process (~100MB) is spawned lazily the first time it is actually + * needed: when a document it serves is opened, or when a request is issued + * through {@link QuartoLspClient.lspRequest}. + */ +export interface QuartoLspClient { + /** + * JSON-RPC transport that lazily starts the language server on first use and + * waits for it to be running before issuing the request. + */ + lspRequest: JsonRpcRequestTransport; + + /** + * Register a callback invoked each time the server reaches the running state. + * Registering does NOT itself start the server; use this for work that should + * happen "if and when" the server comes up (e.g. pushing configuration). If + * the server is already running the callback is invoked immediately. + */ + onReady: (callback: (client: LanguageClient) => void) => Disposable; + + /** + * Start the language server if it has not already been started. Idempotent: + * repeated calls return the same promise. + */ + ensureStarted: () => Promise; + + /** + * The underlying language client if it is currently running, otherwise + * undefined. Useful for "fire only if already running" behavior. + */ + runningClient: () => LanguageClient | undefined; +} + +export function activateLsp( context: ExtensionContext, quartoContext: QuartoContext, engine: MarkdownEngine, outputChannel: LogOutputChannel, -) { +): QuartoLspClient { // The server is implemented in node const serverModule = context.asAbsolutePath( @@ -137,64 +179,147 @@ export async function activateLsp( "**/_{brand,quarto,metadata,extension}*.{yml,yaml}" : "**/_{quarto,metadata,extension}*.{yml,yaml}"; + // documents this server handles; also used to decide when to lazily start it + const documentSelector = [ + { scheme: "*", language: "quarto" }, + { + scheme: "*", + language: "yaml", + pattern: documentSelectorPattern, + }, + ]; + const clientOptions: LanguageClientOptions = { initializationOptions, - documentSelector: [ - { scheme: "*", language: "quarto" }, - { - scheme: "*", - language: "yaml", - pattern: documentSelectorPattern, - }, - ], + documentSelector, middleware, outputChannel }; - // Create the language client and start the client. - client = new LanguageClient( + // Create the language client. NOTE: we deliberately do NOT start it here; + // startup is deferred until the server is actually needed (see below). + const languageClient = new LanguageClient( "quarto-lsp", "Quarto LSP", serverOptions, clientOptions ); + client = languageClient; + + // callbacks to invoke each time the server reaches the running state + const readyCallbacks = new Set<(client: LanguageClient) => void>(); + const onReady = (callback: (client: LanguageClient) => void): Disposable => { + readyCallbacks.add(callback); + // if the server is already running, invoke immediately + if (languageClient.state === State.Running) { + callback(languageClient); + } + return new Disposable(() => { + readyCallbacks.delete(callback); + }); + }; - // Helper to send current theme to LSP server + // Helper to send current theme to LSP server (no-op unless it is running) const sendThemeNotification = () => { - if (client) { + if (languageClient.state === State.Running) { const kind = (window.activeColorTheme.kind === ColorThemeKind.Light || window.activeColorTheme.kind === ColorThemeKind.HighContrastLight) ? "light" : "dark"; - client.sendNotification("quarto/didChangeActiveColorTheme", { kind }); + languageClient.sendNotification("quarto/didChangeActiveColorTheme", { kind }); } }; - // Listen for theme changes and notify the server + // Send the computed theme whenever the server starts, and on theme changes + onReady(() => sendThemeNotification()); context.subscriptions.push( window.onDidChangeActiveColorTheme(() => { sendThemeNotification(); }) ); - // return once the server is running - return new Promise((resolve, reject) => { - - const handler = client.onDidChangeState(e => { - if (e.newState === State.Running) { - handler.dispose(); - // Send computed theme on startup - sendThemeNotification(); - resolve(client); - } else if (e.newState === State.Stopped) { - reject(new Error("Failed to start Quarto LSP Server")); - } + // Start the server on first use. Idempotent: the promise is memoized so + // repeated calls (and multiple triggers) only launch the server once. + let startPromise: Promise | undefined; + const ensureStarted = (): Promise => { + if (!startPromise) { + outputChannel.info("Starting Quarto LSP server."); + startPromise = new Promise((resolve, reject) => { + const handler = languageClient.onDidChangeState(e => { + if (e.newState === State.Running) { + handler.dispose(); + // notify readiness listeners (theme, zotero config, ...) + readyCallbacks.forEach(cb => cb(languageClient)); + resolve(languageClient); + } else if (e.newState === State.Stopped) { + handler.dispose(); + reject(new Error("Failed to start Quarto LSP Server")); + } + }); + // Start the client. This will also launch the server. + languageClient.start(); + }); + } + return startPromise; + }; + + // Lazy JSON-RPC transport: starts the server on first use, then forwards. + let transport: JsonRpcRequestTransport | undefined; + const lspRequest: JsonRpcRequestTransport = async (method, params) => { + const started = await ensureStarted(); + if (!transport) { + transport = lspClientTransport(started); + } + return transport(method, params); + }; + + const runningClient = () => + languageClient.state === State.Running ? languageClient : undefined; + + const startLazily = () => { + void ensureStarted().catch(() => { + // failure is reported to the output channel by the client itself }); + }; - // Start the client. This will also launch the server - client.start(); - }); + // Lazily start the server when a document it serves is opened. Check the + // documents already open at activation, then listen for newly opened ones. + const maybeStartForDocument = (doc: TextDocument) => { + if (languages.match(documentSelector, doc) > 0) { + startLazily(); + } + }; + workspace.textDocuments.forEach(maybeStartForDocument); + context.subscriptions.push( + workspace.onDidOpenTextDocument(maybeStartForDocument) + ); + + // Also start the server if the workspace already contains Quarto content, so + // that workspace-wide features (symbol search, cross-file navigation, project + // indexing) work before any document is opened. This keeps behavior identical + // to eager startup for real Quarto projects, while non-Quarto sessions (e.g. + // editing R files in a workspace with no Quarto files) never spawn the server. + if (workspace.workspaceFolders?.length) { + const contentGlobs = ["**/*.{qmd,rmd}", documentSelectorPattern]; + void (async () => { + for (const glob of contentGlobs) { + // stop early if some other trigger (e.g. an open document) already started it + if (startPromise) { + return; + } + const found = await workspace.findFiles(glob, undefined, 1); + if (found.length > 0) { + startLazily(); + return; + } + } + })(); + } + + return { lspRequest, onReady, ensureStarted, runningClient }; } export function deactivate(): Thenable | undefined { - if (!client) { + // Nothing to stop if the server was never started (state stays Stopped until + // the first `start()` call). + if (!client || client.state === State.Stopped) { return undefined; } return client.stop(); diff --git a/apps/vscode/src/main.ts b/apps/vscode/src/main.ts index a1c046502..658532e19 100644 --- a/apps/vscode/src/main.ts +++ b/apps/vscode/src/main.ts @@ -128,14 +128,14 @@ export async function activate(context: vscode.ExtensionContext): Promise { +export async function activateZotero(context: ExtensionContext, lsp: QuartoLspClient): Promise { - // establish zotero connection - const lspRequest = lspClientTransport(lspClient); - const zotero = editorZoteroJsonRpcServer(lspRequest); + // establish zotero connection (lazy: does not force the LSP to start) + const zotero = editorZoteroJsonRpcServer(lsp.lspRequest); - // set quarto config for back end - await syncZoteroConfig(context, zotero); + // sync quarto config to the back end (whenever the LSP server is running) + syncZoteroConfig(context, zotero, lsp); // register commands const commands: Command[] = []; @@ -50,7 +48,7 @@ export async function activateZotero(context: ExtensionContext, lspClient: Langu } -async function syncZoteroConfig(context: ExtensionContext, zotero: ZoteroServer) { +function syncZoteroConfig(context: ExtensionContext, zotero: ZoteroServer, lsp: QuartoLspClient) { const kZoteroConfig = "quarto.zotero"; const kLibrary = "library"; @@ -60,15 +58,8 @@ async function syncZoteroConfig(context: ExtensionContext, zotero: ZoteroServer) const kGroupLibraries = "groupLibraries"; const kZoteroGroupLibraries = `${kZoteroConfig}.${kGroupLibraries}`; - // set initial config - const setLspLibraryConfig = async (retry?: number) => { - // if this isn't a retry then provide an initial delay (for the lsp to be ready) - if (retry === undefined) { - await sleep(500); - // if this is our final retry then bail - } else if (retry === 5) { - return; - } + // push the current library config to the LSP server + const pushLibraryConfig = async () => { const zoteroConfig = workspace.getConfiguration(kZoteroConfig); const type = zoteroConfig.get<"none" | "local" | "web">(kLibrary, "local"); const dataDir = zoteroConfig.get(kDataDir, ""); @@ -82,10 +73,21 @@ async function syncZoteroConfig(context: ExtensionContext, zotero: ZoteroServer) } catch (error) { const message = error instanceof Error ? error.message : JSON.stringify(error); console.log("Error setting zotero library config: " + message); - setTimeout(() => setLspLibraryConfig((retry || 0) + 1), 1000); } }; - await setLspLibraryConfig(); + + // push config whenever the LSP server starts (this both handles the initial + // sync and re-syncs after any config change made while the server was down). + // We deliberately do NOT start the server just to push config; if it isn't + // running the config will be picked up the next time it starts. + context.subscriptions.push(lsp.onReady(() => { void pushLibraryConfig(); })); + + // push config on change, but only if the server is already running + const pushLibraryConfigIfRunning = async () => { + if (lsp.runningClient()) { + await pushLibraryConfig(); + } + }; // note initial group library config (for detecting changes) const zoteroConfig = workspace.getConfiguration(kZoteroConfig); @@ -94,7 +96,7 @@ async function syncZoteroConfig(context: ExtensionContext, zotero: ZoteroServer) // monitor changes to web api key and update lsp context.secrets.onDidChange(async (e) => { if (e.key === kQuartoZoteroWebApiKey) { - await setLspLibraryConfig(); + await pushLibraryConfigIfRunning(); } }); @@ -111,7 +113,7 @@ async function syncZoteroConfig(context: ExtensionContext, zotero: ZoteroServer) !(await safeReadZoteroApiKey(context))) { await commands.executeCommand(kZoteroConfigureLibrary); } else { - await setLspLibraryConfig(); + await pushLibraryConfigIfRunning(); } } diff --git a/apps/vscode/src/test/r-project.test.ts b/apps/vscode/src/test/r-project.test.ts index e30e4bdef..ee2e8f696 100644 --- a/apps/vscode/src/test/r-project.test.ts +++ b/apps/vscode/src/test/r-project.test.ts @@ -1,10 +1,18 @@ import * as vscode from "vscode"; import * as assert from "assert"; +import { waitForWorkspaceSymbol } from "./test-utils"; // This file is for testing behaviour that is specific to R projects, i.e. // projects with a top-level `DESCRIPTION` file. suite("Workspace Symbols - R Project", function () { + suiteSetup(async function () { + // The LSP now starts lazily, so wait for it to be running and have indexed + // the workspace before asserting on symbol results. + this.timeout(30000); + await waitForWorkspaceSymbol("Symbols-Header-1"); + }); + teardown(async function () { await vscode.workspace .getConfiguration("quarto") diff --git a/apps/vscode/src/test/symbols.test.ts b/apps/vscode/src/test/symbols.test.ts index f5ee1834b..a078c5823 100644 --- a/apps/vscode/src/test/symbols.test.ts +++ b/apps/vscode/src/test/symbols.test.ts @@ -1,9 +1,16 @@ import * as path from "path"; import * as vscode from "vscode"; import * as assert from "assert"; -import { WORKSPACE_PATH } from "./test-utils"; +import { WORKSPACE_PATH, waitForWorkspaceSymbol } from "./test-utils"; suite("Workspace Symbols", function () { + suiteSetup(async function () { + // The LSP now starts lazily, so wait for it to be running and have indexed + // the workspace before asserting on symbol results. + this.timeout(30000); + await waitForWorkspaceSymbol("Symbols-Header-1"); + }); + teardown(async function () { await vscode.workspace .getConfiguration("quarto") diff --git a/apps/vscode/src/test/test-utils.ts b/apps/vscode/src/test/test-utils.ts index 0cb140d22..e044cd130 100644 --- a/apps/vscode/src/test/test-utils.ts +++ b/apps/vscode/src/test/test-utils.ts @@ -29,6 +29,49 @@ export function wait(ms: number) { return new Promise(resolve => setTimeout(resolve, ms)); } +/** + * Polls `condition` until it returns true or `timeout` elapses. Useful for + * waiting on asynchronous, eventually-consistent state such as the Quarto LSP + * finishing its (now lazy) startup and indexing before making assertions. + */ +export async function waitForCondition( + condition: () => boolean | Promise, + { timeout = 15000, interval = 100, message = "condition" }: { timeout?: number; interval?: number; message?: string } = {} +): Promise { + const start = Date.now(); + for (; ;) { + if (await condition()) { + return; + } + if (Date.now() - start >= timeout) { + throw new Error(`Timed out after ${timeout}ms waiting for ${message}`); + } + await wait(interval); + } +} + +/** + * Waits until the (lazily started) Quarto LSP is running and has indexed the + * workspace, detected by the presence of a known workspace symbol. Sets + * `symbols.exportToWorkspace` to "all" so the probe symbol is visible + * regardless of project type (e.g. R projects filter symbols by default). + */ +export async function waitForWorkspaceSymbol(name: string) { + await vscode.workspace + .getConfiguration("quarto") + .update("symbols.exportToWorkspace", "all"); + await waitForCondition( + async () => { + const symbols = await vscode.commands.executeCommand( + "vscode.executeWorkspaceSymbolProvider", + "" + ); + return !!symbols?.find((s) => s.name === name); + }, + { message: `Quarto LSP workspace symbol "${name}"` } + ); +} + export async function openAndShowExamplesTextDocument( fileName: string, showOptions?: vscode.TextDocumentShowOptions From 29d77bc9ad9221a56346a9d04b032d4c95367497 Mon Sep 17 00:00:00 2001 From: Jonathan McPherson Date: Fri, 17 Jul 2026 17:09:01 -0700 Subject: [PATCH 2/2] sync Zotero config --- apps/vscode/src/providers/zotero/zotero.ts | 56 ++++++++++++++++++---- apps/vscode/src/test/convert.test.ts | 5 ++ 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/apps/vscode/src/providers/zotero/zotero.ts b/apps/vscode/src/providers/zotero/zotero.ts index f3ac677e8..90ae4580f 100644 --- a/apps/vscode/src/providers/zotero/zotero.ts +++ b/apps/vscode/src/providers/zotero/zotero.ts @@ -29,13 +29,22 @@ const kZoteroConfigureLibrary = "quarto.zoteroConfigureLibrary"; const kZoteroSyncWebLibrary = "quarto.zoteroSyncWebLibrary"; const kZoteroUnauthorized = "quarto.zoteroUnauthorized"; +// Resolves once the initial Zotero library configuration has been pushed to the +// (lazily started) LSP server. Zotero data requests await this so that the +// backend knows which library is configured before it answers; otherwise, with +// lazy startup, a request that triggered startup could reach the server before +// `setLibraryConfig` and get back empty results. Assigned by `activateZotero`; +// defaults to a no-op so `zoteroLspProxy` is safe if Zotero was never activated. +let ensureZoteroConfigSynced: () => Promise = () => Promise.resolve(); + export async function activateZotero(context: ExtensionContext, lsp: QuartoLspClient): Promise { // establish zotero connection (lazy: does not force the LSP to start) const zotero = editorZoteroJsonRpcServer(lsp.lspRequest); - // sync quarto config to the back end (whenever the LSP server is running) - syncZoteroConfig(context, zotero, lsp); + // sync quarto config to the back end (whenever the LSP server is running); + // exposes the gate that Zotero data requests await before running + ensureZoteroConfigSynced = syncZoteroConfig(context, zotero, lsp); // register commands const commands: Command[] = []; @@ -48,7 +57,7 @@ export async function activateZotero(context: ExtensionContext, lsp: QuartoLspCl } -function syncZoteroConfig(context: ExtensionContext, zotero: ZoteroServer, lsp: QuartoLspClient) { +function syncZoteroConfig(context: ExtensionContext, zotero: ZoteroServer, lsp: QuartoLspClient): () => Promise { const kZoteroConfig = "quarto.zotero"; const kLibrary = "library"; @@ -76,11 +85,28 @@ function syncZoteroConfig(context: ExtensionContext, zotero: ZoteroServer, lsp: } }; - // push config whenever the LSP server starts (this both handles the initial - // sync and re-syncs after any config change made while the server was down). - // We deliberately do NOT start the server just to push config; if it isn't - // running the config will be picked up the next time it starts. - context.subscriptions.push(lsp.onReady(() => { void pushLibraryConfig(); })); + // Ensure the initial library config has been pushed to a running server. + // Memoized so the many potential callers (server startup, and every Zotero + // data request) only trigger a single push. It starts the server if needed, + // which is also what prevents a deadlock: a Zotero data request that awaits + // this gate before it ever hits the transport would otherwise wait forever + // for a config sync that never happens because nothing started the server. + // Note `pushLibraryConfig` uses the ungated `zotero` connection, so its own + // `setLibraryConfig` call does not wait on this gate. + let configSyncPromise: Promise | undefined; + const ensureLibraryConfigSynced = (): Promise => { + if (!configSyncPromise) { + configSyncPromise = (async () => { + await lsp.ensureStarted(); + await pushLibraryConfig(); + })(); + } + return configSyncPromise; + }; + + // push config as soon as the server starts, so citation completion is ready + // without waiting for the first Zotero request (matches prior eager behavior) + context.subscriptions.push(lsp.onReady(() => { void ensureLibraryConfigSynced(); })); // push config on change, but only if the server is already running const pushLibraryConfigIfRunning = async () => { @@ -136,6 +162,8 @@ function syncZoteroConfig(context: ExtensionContext, zotero: ZoteroServer, lsp: groupLibraries = updatedGroupLibraries; } })); + + return ensureLibraryConfigSynced; } // proxy for zotero requests that: @@ -143,7 +171,17 @@ function syncZoteroConfig(context: ExtensionContext, zotero: ZoteroServer, lsp: // (b) checks for unauthorized errors and prompts for re-authorization export function zoteroLspProxy(lspRequest: JsonRpcRequestTransport) { - const zoteroLsp = editorZoteroJsonRpcServer(lspRequest); + // Gate Zotero requests on the initial library-config sync so the backend + // knows the configured library before answering. Without this, the lazily + // started LSP could receive a data request (which itself triggered startup) + // before `setLibraryConfig`, returning empty results. The gate also starts + // the server if it isn't running yet, so a Zotero request can drive startup. + const gatedRequest: JsonRpcRequestTransport = async (method, params) => { + await ensureZoteroConfigSynced(); + return lspRequest(method, params); + }; + + const zoteroLsp = editorZoteroJsonRpcServer(gatedRequest); const handleZoteroResult = (result: ZoteroResult) => { if (result.status === 'notfound' && result.unauthorized) { diff --git a/apps/vscode/src/test/convert.test.ts b/apps/vscode/src/test/convert.test.ts index 168c0385f..be4c4d2c1 100644 --- a/apps/vscode/src/test/convert.test.ts +++ b/apps/vscode/src/test/convert.test.ts @@ -8,6 +8,11 @@ import { } from "./test-utils"; suite("Convert Commands", function () { + // Each test spawns `quarto convert`; the first (cold) CLI spawn can exceed + // mocha's default 5s timeout, especially when the lazily-started LSP is + // warming up concurrently. Give the whole suite generous headroom. + this.timeout(30000); + suiteSetup(async function () { await vscode.workspace.fs.delete(examplesOutUri(), { recursive: true }); await vscode.workspace.fs.copy(