From e2b658e90bad58a0436c2cc33561d50bc968627b Mon Sep 17 00:00:00 2001 From: Guilhem Lemouel Date: Tue, 21 Jul 2026 12:12:52 +0200 Subject: [PATCH] fix(ai): address headless-lint review (buffer overwrite, settling, workspace isolation) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three issues from the codex review of the headless linter: - P0: an absolute import like "/f/shared" resolves via new URL() out of the __wmlint__ owned namespace back to the canonical editor URI, where ATA would setValue the fetched dependency over an open editor's unsaved buffer. The script/runnable path now passes overwriteLocalModels: () => false, matching the raw-app path; genuinely-missing import models are still created. - Settling now compares published markers to the worker's diagnostics by identity (position + code), not count, so an edit that swaps one error for another of the same count no longer settles on the stale marker. A timeout is reported as incomplete rather than complete. - The resource-type and windmill-client declaration libs live at fixed global URIs shared with the editor. A headless lint now snapshots them, installs its workspace's, and restores them afterwards, under a lock so concurrent lints of different workspaces can't interleave — a fork lint no longer leaves a nav-workspace editor validating against the wrong declarations. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/lib/components/lint/headlessLint.ts | 130 +++++++++++++----- .../components/lint/typescriptExtraLibs.ts | 25 ++++ 2 files changed, 120 insertions(+), 35 deletions(-) diff --git a/frontend/src/lib/components/lint/headlessLint.ts b/frontend/src/lib/components/lint/headlessLint.ts index 40a82d267b..c26b536706 100644 --- a/frontend/src/lib/components/lint/headlessLint.ts +++ b/frontend/src/lib/components/lint/headlessLint.ts @@ -19,7 +19,11 @@ import { readThrough, releaseOwnedModel } from './headlessModelHost' -import { ensureCustomWmillTypes, ensureResourceTypeNamespace } from './typescriptExtraLibs' +import { + ensureCustomWmillTypes, + ensureResourceTypeNamespace, + snapshotWorkspaceExtraLibs +} from './typescriptExtraLibs' import { ataSeedImport, createWindmillAta, genAtaRoot } from './typescriptAta' import { lintWithLsp } from './headlessLsp' @@ -140,18 +144,10 @@ async function lintOne( // Otherwise lint an owned model. const uri = acquireOwnedModel(ownedUri, req.content, editorLang) try { - // Bounded: these fetch over the network, and lints of one URI are serialized behind - // each other, so one hung request would wedge the tool for that item indefinitely. - if (editorLang === 'typescript') { - await withDeadline( - Promise.all([ - ensureResourceTypeNamespace(req.workspace, req.scriptLang), - ensureCustomWmillTypes(req.workspace) - ]).catch((e) => console.error('headlessLint: extra libs unavailable', e)), - timeoutMs - ) - } - + // Type acquisition writes workspace-agnostic global libs (npm types) and owned-namespace + // import models, so it stays outside the serialized section below. Bounded: it fetches + // over the network, and lints of one URI are serialized, so a hung request would wedge + // the tool for that item. if (req.scriptLang === 'bun' || req.scriptLang === 'bunnative' || req.scriptLang === 'tsx') { await withDeadline( acquireTypes(req, ownedUri).catch((e) => @@ -161,18 +157,57 @@ async function lintOne( ) } - const settled = await waitForMarkersToSettle(uri, editorLang, timeoutMs) - const result = readModelMarkers(uri) - // A worker that never came up leaves the markers empty for lack of analysis; report - // it as incomplete rather than let an empty set read as clean. - return settled - ? { status: 'complete', result } - : { status: 'incomplete', result, missing: ['the TypeScript checker'] } + // JavaScript has no workspace-specific declarations; nothing to install or isolate. + if (editorLang !== 'typescript') { + return settleAndRead(uri, editorLang, timeoutMs) + } + + // The resource-type and windmill-client declarations live at global URIs shared with the + // editor. Install this workspace's, settle+read, then restore — all under one lock so a + // concurrent lint of another workspace can neither observe nor clobber them. + return withWorkspaceTypes(async () => { + const restore = snapshotWorkspaceExtraLibs() + try { + await withDeadline( + Promise.all([ + ensureResourceTypeNamespace(req.workspace, req.scriptLang), + ensureCustomWmillTypes(req.workspace) + ]).catch((e) => console.error('headlessLint: extra libs unavailable', e)), + timeoutMs + ) + return await settleAndRead(uri, editorLang, timeoutMs) + } finally { + restore() + } + }) } finally { releaseOwnedModel(ownedUri) } } +async function settleAndRead( + uri: Uri, + editorLang: string, + timeoutMs: number +): Promise { + const settled = await waitForMarkersToSettle(uri, editorLang, timeoutMs) + const result = readModelMarkers(uri) + // A worker that never came up leaves the markers empty for lack of analysis; report it as + // incomplete rather than let an empty set read as clean. + return settled + ? { status: 'complete', result } + : { status: 'incomplete', result, missing: ['the TypeScript checker'] } +} + +// Serializes the install → settle → restore of the global workspace declaration libs, so two +// lints of different workspaces can't interleave and leave the wrong declarations resident. +let workspaceTypesChain: Promise = Promise.resolve() +function withWorkspaceTypes(run: () => Promise): Promise { + const task = workspaceTypesChain.catch(() => {}).then(run) + workspaceTypesChain = task.catch(() => {}) + return task +} + async function acquireTypes(req: HeadlessLintRequest, uriString: string): Promise { const key = `${req.workspace}:${uriString}` let ata = ataByKey.get(key) @@ -186,7 +221,12 @@ async function acquireTypes(req: HeadlessLintRequest, uriString: string): Promis root: await genAtaRoot(req.workspace), scriptPath: editorPathOf(req), modelUri: uriString, - absolutePathExtraLibs + absolutePathExtraLibs, + // An absolute import like "/f/shared" resolves via new URL() to the canonical editor + // URI file:///f/shared.ts — outside the __wmlint__ sandbox. If an editor owns that + // model, ATA must never write the fetched dependency over its (possibly unsaved, + // possibly other-workspace) buffer. Only create genuinely-missing import models. + overwriteLocalModels: () => false }) ataByKey.set(key, ata) const seed = ataSeedImport(req.scriptLang) @@ -213,44 +253,64 @@ async function getWorkerFor(uri: Uri, editorLang: string) { throw lastError ?? new Error('typescript worker unavailable') } -async function countExpectedMarkers(uri: Uri, editorLang: string): Promise { +function normalizeCode(code: unknown): string { + if (code == null) return '' + if (typeof code === 'object') return String((code as { value?: unknown }).value ?? '') + return String(code) +} + +// A diagnostic's identity: where it starts and its code. Counting markers is not enough — +// an edit that swaps one error for another of the same count would otherwise look settled +// while the old marker is still showing. +async function expectedMarkerKeys(uri: Uri, editorLang: string): Promise { try { const worker = await getWorkerFor(uri, editorLang) + const model = meditor.getModel(uri) + if (!model) return undefined const fileName = uri.toString() const [syntactic, semantic, suggestions] = await Promise.all([ worker.getSyntacticDiagnostics(fileName), worker.getSemanticDiagnostics(fileName), worker.getSuggestionDiagnostics(fileName) ]) - return [...syntactic, ...semantic, ...suggestions].filter( - (d) => !TS_DIAGNOSTIC_CODES_TO_IGNORE.includes(d.code) - ).length + return [...syntactic, ...semantic, ...suggestions] + .filter((d) => !TS_DIAGNOSTIC_CODES_TO_IGNORE.includes(d.code)) + .map((d) => { + const pos = model.getPositionAt(d.start ?? 0) + return `${pos.lineNumber}:${pos.column}:${normalizeCode(d.code)}` + }) + .sort() } catch (e) { console.error('headlessLint: could not query the typescript worker', e) return undefined } } -// Ask the worker how many diagnostics it has and wait for that many markers to be -// published. Clean code never fires a marker-change event, so there is no event to wait -// on and a count is the only signal that validation has actually run. -// Returns false when the worker could not be queried at all: the markers then read as -// empty for lack of analysis, not because the code is clean, and the caller must not -// report that as a clean result. +// Wait until the published markers match the worker's current diagnostics by identity, not +// just by count. Clean code never fires a marker-change event, so there is no event to wait +// on and the worker is the only signal that validation has actually run. +// Returns false when the worker could not be queried at all, or when the markers never +// converged before the deadline: in both cases what is on the model is not a trustworthy +// picture of this content's diagnostics, and the caller must not report it as clean. async function waitForMarkersToSettle( uri: Uri, editorLang: string, timeoutMs: number ): Promise { - const expected = await countExpectedMarkers(uri, editorLang) + const expected = await expectedMarkerKeys(uri, editorLang) if (expected === undefined) return false + const expectedJoined = expected.join('\n') const deadline = Date.now() + timeoutMs while (Date.now() < deadline) { - const published = meditor.getModelMarkers({ resource: uri, owner: editorLang }).length - if (published === expected) return true + const published = meditor + .getModelMarkers({ resource: uri, owner: editorLang }) + .map((m) => `${m.startLineNumber}:${m.startColumn}:${normalizeCode(m.code)}`) + .sort() + .join('\n') + if (published === expectedJoined) return true await new Promise((r) => setTimeout(r, 50)) } - return true + return false } const APP_LINTABLE_FILE = /\.(tsx?|jsx?)$/ diff --git a/frontend/src/lib/components/lint/typescriptExtraLibs.ts b/frontend/src/lib/components/lint/typescriptExtraLibs.ts index 6f348d68de..c9b225d89a 100644 --- a/frontend/src/lib/components/lint/typescriptExtraLibs.ts +++ b/frontend/src/lib/components/lint/typescriptExtraLibs.ts @@ -62,3 +62,28 @@ export function applyCustomWmillTypes(data: CustomWmillTypesData): () => void { export async function ensureCustomWmillTypes(workspace: string): Promise { applyCustomWmillTypes(await fetchCustomWmillTypesData(workspace)) } + +// The two libs above are keyed by workspace but installed at these fixed, global URIs — an +// editor and a headless lint of *different* workspaces would otherwise overwrite each other's +// declarations. A headless lint snapshots them, installs its own, and restores afterwards so a +// mounted editor keeps validating against its own workspace's types. +const WORKSPACE_LIB_URIS = ['rt.d.ts', 'file:///custom_wmill_types.d.ts'] as const +const NEUTRAL_LIB = 'export {};' + +export function snapshotWorkspaceExtraLibs(): () => void { + const libs = typescriptDefaults.getExtraLibs() + const saved = WORKSPACE_LIB_URIS.map((uri) => ({ uri, content: libs[uri]?.content })) + return () => { + const now = typescriptDefaults.getExtraLibs() + for (const { uri, content } of saved) { + if (content !== undefined) { + // Restore the prior content (a no-op when the lint left it unchanged). + typescriptDefaults.addExtraLib(content, uri) + } else if (now[uri] !== undefined) { + // The lint installed a lib where none existed; there is no editor declaration to + // restore, so neutralize it rather than leave one workspace's types resident. + typescriptDefaults.addExtraLib(NEUTRAL_LIB, uri) + } + } + } +}