From b0066d6ac6dabef9e7125cff2139b94682af5d31 Mon Sep 17 00:00:00 2001 From: Julia Silge Date: Wed, 9 Sep 2026 13:00:12 -0600 Subject: [PATCH 1/3] Fix Zotero silently disappearing as a citation source --- apps/vscode/src/lsp/client.ts | 3 + apps/vscode/src/main.ts | 2 +- apps/vscode/src/providers/zotero/zotero.ts | 106 ++++++++++++++---- .../editor-server/src/core/zotero/local/db.ts | 40 ++++++- 4 files changed, 127 insertions(+), 24 deletions(-) diff --git a/apps/vscode/src/lsp/client.ts b/apps/vscode/src/lsp/client.ts index b86b69ce..147096e2 100644 --- a/apps/vscode/src/lsp/client.ts +++ b/apps/vscode/src/lsp/client.ts @@ -228,6 +228,7 @@ export function activateLsp( // Start the server on first use. Idempotent: the promise is memoized so // repeated calls (and multiple triggers) only launch the server once. + // Cleared on failure below so a later call can retry. let startPromise: Promise | undefined; const ensureStarted = (): Promise => { if (!startPromise) { @@ -241,6 +242,8 @@ export function activateLsp( resolve(languageClient); } else if (e.newState === State.Stopped) { handler.dispose(); + // clear the memo so a later call retries instead of replaying this rejection + startPromise = undefined; reject(new Error("Failed to start Quarto LSP Server")); } }); diff --git a/apps/vscode/src/main.ts b/apps/vscode/src/main.ts index 0c016330..ba1c98e2 100644 --- a/apps/vscode/src/main.ts +++ b/apps/vscode/src/main.ts @@ -131,7 +131,7 @@ export async function activate(context: vscode.ExtensionContext): Promise Promise = () => Promise.resolve(); -export async function activateZotero(context: ExtensionContext, lsp: QuartoLspClient): Promise { +export async function activateZotero(context: ExtensionContext, lsp: QuartoLspClient, outputChannel: LogOutputChannel): 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); // exposes the gate that Zotero data requests await before running - ensureZoteroConfigSynced = syncZoteroConfig(context, zotero, lsp); + ensureZoteroConfigSynced = syncZoteroConfig(context, zotero, lsp, outputChannel); // register commands const commands: Command[] = []; @@ -48,7 +48,17 @@ export async function activateZotero(context: ExtensionContext, lsp: QuartoLspCl } -function syncZoteroConfig(context: ExtensionContext, zotero: ZoteroServer, lsp: QuartoLspClient): () => Promise { +function syncZoteroConfig(context: ExtensionContext, zotero: ZoteroServer, lsp: QuartoLspClient, outputChannel: LogOutputChannel): () => Promise { + + // the config push can race server startup (running != ready for custom + // requests), so failed pushes are retried at a fixed delay + const kMaxPushAttempts = 5; + const kPushRetryDelayMs = 1000; + + // after a failed retry cycle, requests skip retrying until this cooldown + // elapses, so a persistent failure doesn't retry on every citation lookup + const kSyncFailureCooldownMs = 30_000; + let retryAfter = 0; const kZoteroConfig = "quarto.zotero"; const kLibrary = "library"; @@ -58,24 +68,51 @@ function syncZoteroConfig(context: ExtensionContext, zotero: ZoteroServer, lsp: const kGroupLibraries = "groupLibraries"; const kZoteroGroupLibraries = `${kZoteroConfig}.${kGroupLibraries}`; - // push the current library config to the LSP server - const pushLibraryConfig = async () => { + // read the currently configured library settings + const readLibraryConfig = async (): Promise => { const zoteroConfig = workspace.getConfiguration(kZoteroConfig); - const type = zoteroConfig.get<"none" | "local" | "web">(kLibrary, "local"); - const dataDir = zoteroConfig.get(kDataDir, ""); - const apiKey = await safeReadZoteroApiKey(context); + return { + type: zoteroConfig.get<"none" | "local" | "web">(kLibrary, "local"), + dataDir: zoteroConfig.get(kDataDir, ""), + apiKey: await safeReadZoteroApiKey(context) + }; + }; + + // push a library config to the LSP server, returning success; takes config + // as a param so retries don't re-read config/secrets on every attempt + const pushLibraryConfig = async (config: ZoteroLibraryConfig): Promise => { try { - await zotero.setLibraryConfig({ - type, - dataDir, - apiKey - }); + await zotero.setLibraryConfig(config); + return true; } catch (error) { const message = error instanceof Error ? error.message : JSON.stringify(error); - console.log("Error setting zotero library config: " + message); + outputChannel.warn("Error setting zotero library config: " + message); + return false; } }; + // Log a failure, clear the memo so the next request retries (a single + // failure must not disable Zotero for the session, see + // https://github.com/quarto-dev/quarto/issues/1101), and warn the user + // with a retry option. + const notifySyncFailure = (message: string) => { + outputChannel.warn(message); + configSyncPromise = undefined; + retryAfter = Date.now() + kSyncFailureCooldownMs; + const kRetry = "Retry"; + void window.showWarningMessage( + "Quarto could not configure the connection to your Zotero library, " + + "so Zotero may be unavailable as a citation source.", + kRetry + ).then((result) => { + if (result === kRetry) { + // user-initiated retry bypasses the cooldown + retryAfter = 0; + void ensureLibraryConfigSynced(); + } + }); + }; + // 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, @@ -83,13 +120,35 @@ function syncZoteroConfig(context: ExtensionContext, zotero: ZoteroServer, lsp: // 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. + // `setLibraryConfig` call does not wait on this gate. After a failure, + // calls during the cooldown above resolve immediately without retrying. let configSyncPromise: Promise | undefined; const ensureLibraryConfigSynced = (): Promise => { if (!configSyncPromise) { + if (Date.now() < retryAfter) { + return Promise.resolve(); + } configSyncPromise = (async () => { - await lsp.ensureStarted(); - await pushLibraryConfig(); + try { + await lsp.ensureStarted(); + } catch (error) { + const message = error instanceof Error ? error.message : JSON.stringify(error); + notifySyncFailure("Unable to start Quarto LSP server to sync zotero library config: " + message); + return; + } + const config = await readLibraryConfig(); + for (let attempt = 1; attempt <= kMaxPushAttempts; attempt++) { + if (await pushLibraryConfig(config)) { + return; + } + if (attempt < kMaxPushAttempts) { + await sleep(kPushRetryDelayMs); + } + } + notifySyncFailure( + `Unable to sync zotero library config after ${kMaxPushAttempts} attempts; ` + + `will retry on the next zotero request after a ${kSyncFailureCooldownMs / 1000}s cooldown.` + ); })(); } return configSyncPromise; @@ -99,10 +158,13 @@ function syncZoteroConfig(context: ExtensionContext, zotero: ZoteroServer, lsp: // 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 + // push config on change, but only if the server is already running; route + // through the same gate as the initial sync so a failure here retries and + // notifies the user just like an initial sync failure would const pushLibraryConfigIfRunning = async () => { if (lsp.runningClient()) { - await pushLibraryConfig(); + configSyncPromise = undefined; + await ensureLibraryConfigSynced(); } }; diff --git a/packages/editor-server/src/core/zotero/local/db.ts b/packages/editor-server/src/core/zotero/local/db.ts index 3fd3a0c1..4147deaf 100644 --- a/packages/editor-server/src/core/zotero/local/db.ts +++ b/packages/editor-server/src/core/zotero/local/db.ts @@ -15,8 +15,25 @@ import { quartoCacheDir } from "quarto-core"; import { md5Hash } from "core-node"; import { zoteroTrace } from "../trace"; +// Concurrent callers (e.g. getCollections + getActiveCollectionSpecs) share a +// copy path per dataDir; without serializing them, one call's copy/open can +// race another's and delete the file out from under it. Queue per dataDir. +const dbQueues = new Map>(); + // eslint-disable-next-line @typescript-eslint/no-unused-vars -export async function withZoteroDb(dataDir: string, f: (db: Database) => Promise) { +export function withZoteroDb(dataDir: string, f: (db: Database) => Promise): Promise { + const previous = dbQueues.get(dataDir) ?? Promise.resolve(); + const current = previous.catch(() => undefined).then(() => withZoteroDbExclusive(dataDir, f)); + dbQueues.set(dataDir, current); + current.catch(() => undefined).finally(() => { + if (dbQueues.get(dataDir) === current) { + dbQueues.delete(dataDir); + } + }); + return current; +} + +async function withZoteroDbExclusive(dataDir: string, f: (db: Database) => Promise): Promise { // get path to actual sqlite db const dbFile = path.join(dataDir, "zotero.sqlite"); @@ -31,6 +48,9 @@ export async function withZoteroDb(dataDir: string, f: (db: Database) => Prom if (databaseIsStale) { zoteroTrace(`Copying ${dbFile}`); fs.copyFileSync(dbFile, dbCopyFile); + // node-sqlite3-wasm can't open a WAL-mode database (Zotero's default); + // clearing the WAL flag just makes the already-copied data openable. + forceLegacyJournalMode(dbCopyFile); fs.utimesSync(dbCopyFile, dbFileStat.atime, dbFileStat.mtime); } @@ -73,6 +93,24 @@ export async function withZoteroDb(dataDir: string, f: (db: Database) => Prom } +// File header offset 18-19 flags journal mode (1 = legacy, 2 = WAL); +// flipping it doesn't touch page data, just what a reader assumes. +function forceLegacyJournalMode(dbFile: string) { + const kJournalModeOffset = 18; + const kLegacyJournalMode = 1; + const fd = fs.openSync(dbFile, "r+"); + try { + const versionBytes = Buffer.alloc(2); + fs.readSync(fd, versionBytes, 0, 2, kJournalModeOffset); + if (versionBytes[0] !== kLegacyJournalMode || versionBytes[1] !== kLegacyJournalMode) { + versionBytes.fill(kLegacyJournalMode); + fs.writeSync(fd, versionBytes, 0, 2, kJournalModeOffset); + } + } finally { + fs.closeSync(fd); + } +} + function zoteroSqliteDir() { const sqliteDir = path.join(quartoCacheDir("zotero"), "sqlite"); if (!fs.existsSync(sqliteDir)) { From 6443813dcbc345ddf21ea83f5dadaec7ad4c7aaf Mon Sep 17 00:00:00 2001 From: Julia Silge Date: Wed, 9 Sep 2026 14:05:28 -0600 Subject: [PATCH 2/3] Update CHANGELOG --- apps/vscode/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/vscode/CHANGELOG.md b/apps/vscode/CHANGELOG.md index 9132c277..67312a24 100644 --- a/apps/vscode/CHANGELOG.md +++ b/apps/vscode/CHANGELOG.md @@ -2,6 +2,8 @@ ## 1.138.0 (Unreleased) +- Fixed several bugs where Zotero could fail to appear as a citation source in the visual editor. The initial Zotero library configuration push to the language server was attempted only once and failures were swallowed (a regression in 1.136.0); it is now retried, failures are logged to the Quarto output channel, and a warning with a retry option is shown if the sync ultimately fails. Concurrent citation lookups could also race on the same cached copy of the local Zotero database, and that database could not be read at all when Zotero stored it in WAL mode, the default in current Zotero versions; both are now fixed (). + ## 1.137.0 (Release on 2026-09-04) From 95a84dd1e968ab135f0db131229c27cd6c6641e4 Mon Sep 17 00:00:00 2001 From: Julia Silge Date: Fri, 11 Sep 2026 07:55:32 -0600 Subject: [PATCH 3/3] Wait for LSP server ready signal before sending custom requests --- apps/lsp/src/index.ts | 5 +++++ apps/vscode/CHANGELOG.md | 2 +- apps/vscode/src/lsp/client.ts | 25 ++++++++++++++++++++++--- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/apps/lsp/src/index.ts b/apps/lsp/src/index.ts index 59ac4108..1dae3cb6 100644 --- a/apps/lsp/src/index.ts +++ b/apps/lsp/src/index.ts @@ -323,6 +323,11 @@ connection.onInitialized(async () => { // signal that `mdLs` is now ready to serve requests: // handlers like document symbols, folding ranges, etc will now proceed resolveMdLsReady(); + + // signal to the client that startup is complete and all request handlers + // (including the custom methods registered above) are ready to serve + // requests + connection.sendNotification("quarto/serverReady"); }); diff --git a/apps/vscode/CHANGELOG.md b/apps/vscode/CHANGELOG.md index 5824a27b..2ef68e30 100644 --- a/apps/vscode/CHANGELOG.md +++ b/apps/vscode/CHANGELOG.md @@ -4,7 +4,7 @@ - In Positron, running a Python cell in a knitr document now respects the `quarto.cells.useReticulate` setting, instead of always routing it through reticulate on the R console (). - In Positron, when Positron serves language features for code cells itself (the `quarto.embeddedLanguageFeatures.native` setting), the extension no longer serves them from virtual document temp files (). -- Fixed several bugs where Zotero could fail to appear as a citation source in the visual editor. The initial Zotero library configuration push to the language server was attempted only once and failures were swallowed (a regression in 1.136.0); it is now retried, failures are logged to the Quarto output channel, and a warning with a retry option is shown if the sync ultimately fails. Concurrent citation lookups could also race on the same cached copy of the local Zotero database, and that database could not be read at all when Zotero stored it in WAL mode, the default in current Zotero versions; both are now fixed (). +- Fixed several bugs where Zotero could fail to appear as a citation source in the visual editor. The initial Zotero library configuration push to the language server was attempted only once and failures were swallowed (a regression in 1.136.0); it is now retried, failures are logged to the Quarto output channel, and a warning with a retry option is shown if the sync ultimately fails. Requests to the language server also now wait for the server to signal that it has finished starting up, fixing "Unhandled method" errors when requests raced with server startup. Concurrent citation lookups could also race on the same cached copy of the local Zotero database, and that database could not be read at all when Zotero stored it in WAL mode, the default in current Zotero versions; both are now fixed (). ## 1.137.0 (Release on 2026-09-04) diff --git a/apps/vscode/src/lsp/client.ts b/apps/vscode/src/lsp/client.ts index 29b120c3..045a55e6 100644 --- a/apps/vscode/src/lsp/client.ts +++ b/apps/vscode/src/lsp/client.ts @@ -66,7 +66,7 @@ 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 { JsonRpcRequestTransport, sleep } from "core"; import { extensionHost } from "../host"; import { kHostCellFeaturesSetting, hostOwnsCellFeatures } from "../host/cell-features"; import { hasChunkSymbols, nestCellSymbols, quartoCellSymbols } from "./cell-symbols"; @@ -89,7 +89,8 @@ let client: LanguageClient | undefined; 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. + * waits for it to be running and fully initialized (all request handlers + * registered) before issuing the request. */ lspRequest: JsonRpcRequestTransport; @@ -227,6 +228,13 @@ export function activateLsp( ); client = languageClient; + // Resolves when the server signals that it has finished its async startup + // and registered all of its request handlers, including the custom JSON-RPC + // methods served via `lspRequest`. + let resolveServerReady!: () => void; + const serverReady = new Promise(resolve => { resolveServerReady = resolve; }); + languageClient.onNotification("quarto/serverReady", () => resolveServerReady()); + // 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 => { @@ -284,10 +292,21 @@ export function activateLsp( return startPromise; }; - // Lazy JSON-RPC transport: starts the server on first use, then forwards. + // Lazy JSON-RPC transport: starts the server on first use, waits for it to + // finish initializing, then forwards. let transport: JsonRpcRequestTransport | undefined; + let readinessWarningShown = false; const lspRequest: JsonRpcRequestTransport = async (method, params) => { const started = await ensureStarted(); + // Wait for the server to register its request handlers. + const timedOut = await Promise.race([ + serverReady.then(() => false as const), + sleep(60_000).then(() => true as const) + ]); + if (timedOut && !readinessWarningShown) { + readinessWarningShown = true; + outputChannel.warn("Timed out waiting for the Quarto LSP server to finish starting up; requests to it may fail."); + } if (!transport) { transport = lspClientTransport(started); }