fix(ai): address headless-lint review (buffer overwrite, settling, workspace isolation)

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) <noreply@anthropic.com>
2026-09-06 08:01:35 +00:00 · 2026-07-21 12:12:52 +02:00
parent f09a96f88d
commit e2b658e90b
2 changed files with 120 additions and 35 deletions
@@ -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<LintOutcome> {
+	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<unknown> = Promise.resolve()
+function withWorkspaceTypes<T>(run: () => Promise<T>): Promise<T> {
+	const task = workspaceTypesChain.catch(() => {}).then(run)
+	workspaceTypesChain = task.catch(() => {})
+	return task
+}
+
 async function acquireTypes(req: HeadlessLintRequest, uriString: string): Promise<void> {
 	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<number | undefined> {
+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<string[] | undefined> {
 	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<boolean> {
-	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?)$/
@@ -62,3 +62,28 @@ export function applyCustomWmillTypes(data: CustomWmillTypesData): () => void {
 export async function ensureCustomWmillTypes(workspace: string): Promise<void> {
 	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)
+			}
+		}
+	}
+}