From 141793ac78d46f417ff813bff4dc5366ffa270ce Mon Sep 17 00:00:00 2001 From: fi3ework Date: Tue, 25 Aug 2026 15:32:20 +0800 Subject: [PATCH] fix(vscode): adopt a same-key pending lint runtime instead of restarting it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit At activation the register-time reconcile, a detection pass and didOpen routinely land inside one lint worker startup window. reconcile() swept the document's pending runtime uses before knowing what the new pass would resolve to, so the second reconcile tore down the not-yet-bound runtime mid-LSP-initialize (SIGTERM), vscode-languageclient force-notified "Server initialization failed" / "couldn't create connection to server", and an identical runtime was then started from scratch. - RuntimeManager.reconcile now resolves the document's core first (planDocumentCore) and the pending-use sweep keeps a pending entry whose key matches the plan — the reconcile adopts the in-flight start. A key change still cancels the pending start immediately, and a superseded operation leaves the entry's fate to the newest reconcile's sweep. - Rslint.close() gives a still-Starting client one bounded chance (2s) to settle before tearing down the transport, so legitimate mid-start closes (document closed during start, core key changed) stop cleanly instead of triggering the same forced toasts; planned aborts are no longer logged as errors. The pre-teardown wait subsumes the old post-dispose start wait. Both are ahead-of-upstream fixes, recorded in AGENTS.md to be offered back on the next sync. Regression-locked in tests/stacks/lint/runtimeManager.test.ts. --- packages/vscode/AGENTS.md | 1 + packages/vscode/src/stacks/lint/Rslint.ts | 51 ++-- .../vscode/src/stacks/lint/RuntimeManager.ts | 87 +++++-- .../tests/stacks/lint/runtimeManager.test.ts | 233 ++++++++++++++++++ 4 files changed, 335 insertions(+), 37 deletions(-) create mode 100644 packages/vscode/tests/stacks/lint/runtimeManager.test.ts diff --git a/packages/vscode/AGENTS.md b/packages/vscode/AGENTS.md index a31e0b0..a5e778b 100644 --- a/packages/vscode/AGENTS.md +++ b/packages/vscode/AGENTS.md @@ -7,6 +7,7 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten - `stacks/lint` and `stacks/test` are deliberate near-verbatim copies of the upstream extensions, kept close to upstream so changes can be synced by diffing. Do NOT deduplicate or refactor across the two stacks — the duplication is the point; consolidation is a later, explicit phase. - The copies diverge from upstream in exactly nine ways (the "adaptations" below). When syncing upstream, preserve them. A tenth divergence is either a bug or must be added to this list. - **Tracked upstream state.** `stacks/lint` is synced to web-infra-dev/rslint `packages/vscode-extension` at **39536fd6** (#1617 — per-document core resolution, `CoreResolver` + `RuntimeManager`, `corePath`, PnP removed) and **892482e0** (#1630 — `configPath` on `rslint/configRefresh`). `CoreResolver.ts` / `RuntimeManager.ts` / `WorkspaceDocumentRouter.ts` / `Rslint.ts` are the files to diff when syncing further; record the new commits here when you do. +- **Ahead of upstream — offer these back when syncing** (bug fixes, not adaptations): (1) `RuntimeManager.reconcile` resolves the document's core **before** sweeping pending uses (`planDocumentCore`), so a reconcile landing on the key a pending start is already producing adopts that start instead of tearing it down mid-`initialize` — the teardown made vscode-languageclient force-notify ("couldn't create connection to server") whenever the register-time pass, a detection change and `didOpen` landed inside one worker startup window (`tests/stacks/lint/runtimeManager.test.ts`). (2) `Rslint.close()` gives a still-Starting language client a bounded chance to settle before tearing down its transport, so a legitimate mid-start close (document closed during start, core key changed) stops cleanly instead of triggering the same force-notified toasts. ## The nine adaptations diff --git a/packages/vscode/src/stacks/lint/Rslint.ts b/packages/vscode/src/stacks/lint/Rslint.ts index 18f1498..df993d4 100644 --- a/packages/vscode/src/stacks/lint/Rslint.ts +++ b/packages/vscode/src/stacks/lint/Rslint.ts @@ -48,6 +48,13 @@ import { type WorkspaceDocumentRouter, } from './WorkspaceDocumentRouter'; +/** + * Bound for each wait inside `close()`. Covers a warm worker boot (~1s), and + * is what a changed-key replacement or deactivation pays, worst case, for a + * hung start (the superseded close is awaited on the per-document tail). + */ +const CLOSE_SETTLEMENT_TIMEOUT_MS = 2_000; + const LOCKFILE_NAMES = [ 'package-lock.json', 'pnpm-lock.yaml', @@ -362,13 +369,14 @@ export class Rslint implements Disposable { await this.startPromise; } + private isPlannedStartAbort(error: unknown): boolean { + return ( + this.closing || (error instanceof Error && error.name === 'AbortError') + ); + } + private reportStartFailure(error: unknown): void { - if ( - this.closing || - (error instanceof Error && error.name === 'AbortError') - ) { - return; - } + if (this.isPlannedStartAbort(error)) return; this.report(statusForRslintStartFailure(error)); } @@ -519,7 +527,11 @@ export class Rslint implements Disposable { this.logger.info('Rslint language client started successfully'); this.reportRunning(); } catch (error: unknown) { - this.logger.error('Failed to start Rslint language client', error); + // A close or supersede during start is a planned abort, not a failure; + // logging it as an error made every teardown race look like a crash. + if (!this.isPlannedStartAbort(error)) { + this.logger.error('Failed to start Rslint language client', error); + } throw error; } } @@ -654,6 +666,18 @@ export class Rslint implements Disposable { this.client = undefined; const clientStartPromise = this.clientStartPromise; this.clientStartPromise = undefined; + // Severing the transport under an in-flight initialize makes + // vscode-languageclient force-notify ("couldn't create connection to + // server" / "Server initialization failed"), so a Starting client gets a + // bounded chance to settle first — a successful start then stops cleanly, + // a hung one falls through to the hard teardown. + if (client?.state === State.Starting && clientStartPromise) { + await waitForPromiseSettlement( + clientStartPromise, + CLOSE_SETTLEMENT_TIMEOUT_MS, + 'language client start before teardown', + ).catch(() => undefined); + } const clientStopped = client?.state === State.Starting ? observeClientStopped(client) @@ -677,22 +701,11 @@ export class Rslint implements Disposable { } catch (error) { clientErrors.push(error); } - if (clientStartPromise) { - try { - await waitForPromiseSettlement( - clientStartPromise, - 2_000, - 'language client start', - ); - } catch (error) { - clientErrors.push(error); - } - } if (clientStopped) { try { await waitForPromiseSettlement( clientStopped.promise, - 2_000, + CLOSE_SETTLEMENT_TIMEOUT_MS, 'language client terminal state', ); } catch (error) { diff --git a/packages/vscode/src/stacks/lint/RuntimeManager.ts b/packages/vscode/src/stacks/lint/RuntimeManager.ts index ebaff49..5b94fd8 100644 --- a/packages/vscode/src/stacks/lint/RuntimeManager.ts +++ b/packages/vscode/src/stacks/lint/RuntimeManager.ts @@ -16,6 +16,8 @@ // - The extra hooks (`onDocumentFailure` / `onDocumentSettled` / // `onRuntimeClosed`) exist only so the controller can keep its per-folder // status fold in step; they carry no lifecycle decisions. +// - One ahead-of-upstream fix: `reconcile` resolves before sweeping pending +// uses (`planDocumentCore`) — see AGENTS.md ("Ahead of upstream"). import { workspace, type TextDocument, type WorkspaceFolder } from 'vscode'; import type { @@ -77,6 +79,21 @@ export interface RuntimeManagerOptions { readonly onRuntimeClosed?: (resolved: ResolvedCoreRuntime) => void; } +/** What one reconcile decided for its document, shared by the pending-use + * sweep and the queued operation (see `planDocumentCore`). */ +type DocumentCorePlan = + | { readonly action: 'detach' } + | { + readonly action: 'bind'; + readonly workspaceFolder: WorkspaceFolder; + readonly resolved: ResolvedCoreRuntime; + } + | { + readonly action: 'report'; + readonly workspaceFolder: WorkspaceFolder; + readonly error: unknown; + }; + interface RuntimeEntry { readonly resolved: ResolvedCoreRuntime; readonly runtime: ManagedRslintRuntime; @@ -159,10 +176,23 @@ export class RuntimeManager { if (this.closing) return; const key = documentKey(document); const epoch = this.nextDocumentEpoch(key); - this.releasePendingDocumentUses(key); + // Resolve before touching pending uses: a reconcile landing on the key a + // pending start is already producing must adopt that start, not tear it + // down mid-initialize — vscode-languageclient force-notifies ("couldn't + // create connection to server") when its in-flight initialize is severed, + // and the register-time pass, a detection change and `onDidOpen` routinely + // land inside one worker startup window. A key change still cancels the + // pending start immediately, so a hung start cannot block the tail + // (assumes the resolver settles — a bounded fs walk). + const plan = await this.planDocumentCore(document); + if (!this.isCurrentDocument(document, epoch)) return; + this.releasePendingDocumentUses( + key, + plan.action === 'bind' ? plan.resolved.key : undefined, + ); await this.enqueueDocument(key, async () => { if (!this.isCurrentDocument(document, epoch)) return; - await this.reconcileCurrentDocument(document, epoch); + await this.reconcileCurrentDocument(document, epoch, plan); }); } @@ -186,12 +216,14 @@ export class RuntimeManager { await (this.closePromise ??= this.closeImpl()); } - private async reconcileCurrentDocument( + /** + * The document's target, decided before the per-document tail is entered so + * `reconcile` can spare a pending same-key start from the pending-use sweep; + * the queued operation consumes the same plan so the two never disagree. + */ + private async planDocumentCore( document: TextDocument, - epoch: number, - ): Promise { - const key = documentKey(document); - const existing = this.bindings.get(key); + ): Promise { const workspaceFolder = workspace.getWorkspaceFolder(document.uri); const mode = workspaceFolder ? this.options.folderMode(workspaceFolder) @@ -201,27 +233,39 @@ export class RuntimeManager { !workspaceFolder || mode === undefined ) { - await this.detachDocument(document); - return; + return { action: 'detach' }; } const configuration = workspace.getConfiguration( 'rstack.rslint', document.uri, ); - - let resolved: ResolvedCoreRuntime; try { - resolved = await this.resolver.resolve(document, workspaceFolder, { + const resolved = await this.resolver.resolve(document, workspaceFolder, { mode, corePath: configuration.get('corePath'), }); + return { action: 'bind', workspaceFolder, resolved }; } catch (error) { - if (this.isCurrentDocument(document, epoch)) { - this.reportFailure(document, workspaceFolder, error, existing); - } + return { action: 'report', workspaceFolder, error }; + } + } + + private async reconcileCurrentDocument( + document: TextDocument, + epoch: number, + plan: DocumentCorePlan, + ): Promise { + const key = documentKey(document); + const existing = this.bindings.get(key); + if (plan.action === 'detach') { + await this.detachDocument(document); return; } - if (!this.isCurrentDocument(document, epoch)) return; + if (plan.action === 'report') { + this.reportFailure(document, plan.workspaceFolder, plan.error, existing); + return; + } + const { workspaceFolder, resolved } = plan; if (existing?.resolved.key === resolved.key) { this.options.onDocumentSettled?.(document); return; @@ -233,7 +277,9 @@ export class RuntimeManager { replacement = this.acquireRuntime(resolved, key); await replacement.startPromise; if (!this.isCurrentDocument(document, epoch)) { - await this.releaseRuntimeAfterFailure(replacement, key); + // A newer reconcile owns this document; its sweep already decided this + // entry's fate (kept for same-key adoption, released otherwise). + // Releasing again would tear down what the successor is binding. return; } await this.router.assign(document, resolved.key); @@ -382,10 +428,15 @@ export class RuntimeManager { } } - private releasePendingDocumentUses(documentUri: string): void { + private releasePendingDocumentUses( + documentUri: string, + keepKey?: string, + ): void { const bound = this.bindings.get(documentUri); for (const entry of [...this.entries.values()]) { if (entry === bound || !entry.users.has(documentUri)) continue; + // Same key as the sweep's plan: adopt the pending start, don't restart. + if (entry.resolved.key === keepKey) continue; void this.releaseRuntime(entry, documentUri).catch((error: unknown) => { this.logger.error( `Failed to cancel pending Rslint core ${entry.resolved.installation.packageDirectory}`, diff --git a/packages/vscode/tests/stacks/lint/runtimeManager.test.ts b/packages/vscode/tests/stacks/lint/runtimeManager.test.ts new file mode 100644 index 0000000..233b80c --- /dev/null +++ b/packages/vscode/tests/stacks/lint/runtimeManager.test.ts @@ -0,0 +1,233 @@ +/** + * The reconcile-during-start race (upstream lifecycle, rslint #1617 port): + * a second `reconcile` of the same document while the first runtime start is + * still in flight must not tear down and relaunch a runtime that resolves to + * the very same key. Tearing it down mid-`initialize` is what surfaced + * vscode-languageclient's force-notified "Server initialization failed" / + * "couldn't create connection to server" toasts at activation, when the + * register-time reconcile, a detection pass and `onDidOpenTextDocument` all + * land within the worker's startup window. + */ +import { describe, expect, it, rs } from '@rstest/core'; +import type { TextDocument, WorkspaceFolder } from 'vscode'; + +rs.mock('vscode', () => { + const folder = { + name: 'fixture', + index: 0, + uri: { toString: () => 'file:///project', fsPath: '/project' }, + }; + return { + RelativePattern: class {}, + Uri: { parse: (value: string) => ({ toString: () => value }) }, + workspace: { + textDocuments: [] as unknown[], + getWorkspaceFolder: () => folder, + getConfiguration: () => ({ get: () => undefined }), + }, + }; +}); + +import { + RuntimeManager, + type ManagedRslintRuntime, +} from '../../../src/stacks/lint/RuntimeManager'; +import type { ResolvedCoreRuntime } from '../../../src/stacks/lint/CoreResolver'; +import type { WorkspaceDocumentRouter } from '../../../src/stacks/lint/WorkspaceDocumentRouter'; + +const folder = { + name: 'fixture', + index: 0, + uri: { toString: () => 'file:///project', fsPath: '/project' }, +} as unknown as WorkspaceFolder; + +function documentOf(path: string): TextDocument { + return { + languageId: 'typescript', + uri: { + scheme: 'file', + fsPath: path, + toString: () => `file://${path}`, + }, + } as unknown as TextDocument; +} + +function resolvedCore(key: string): ResolvedCoreRuntime { + return { + key, + workspaceFolder: folder, + installation: { packageDirectory: '/project/node_modules/@rslint/core' }, + } as unknown as ResolvedCoreRuntime; +} + +interface FakeRuntime { + readonly runtime: ManagedRslintRuntime; + releaseStart(): void; + closes: number; + aborted: boolean; +} + +function fakeRuntime(): FakeRuntime { + let releaseStart!: () => void; + const startGate = new Promise((resolve) => { + releaseStart = resolve; + }); + const fake: FakeRuntime = { + releaseStart, + closes: 0, + aborted: false, + runtime: { + rootKey: 'core-key', + workspaceFolder: folder, + sendDocumentOpen: async () => undefined, + sendDocumentClose: async () => undefined, + clearDocumentDiagnostics: () => undefined, + // Mirrors Rslint.start: the returned promise rejects on abort even + // while the underlying startup work is still pending (raceWithAbort). + async start(signal) { + await new Promise((resolve, reject) => { + signal.addEventListener( + 'abort', + () => { + fake.aborted = true; + reject( + signal.reason instanceof Error + ? signal.reason + : new Error('aborted'), + ); + }, + { once: true }, + ); + void startGate.then(resolve); + }); + }, + async close() { + fake.closes += 1; + }, + }, + }; + return fake; +} + +const routerStub = { + assign: async () => undefined, + activate: async () => undefined, + deactivate: async () => undefined, + closeAll: async () => undefined, +} as unknown as WorkspaceDocumentRouter; + +const silentLogger = { + debug: () => undefined, + info: () => undefined, + error: () => undefined, +}; + +const WAIT = { timeout: 5_000, interval: 5 }; + +interface Harness { + readonly manager: RuntimeManager; + readonly runtimes: FakeRuntime[]; + readonly failures: unknown[]; + readonly state: { key: string; documentOpen: boolean }; +} + +function createHarness(): Harness { + const runtimes: FakeRuntime[] = []; + const failures: unknown[] = []; + const state = { key: 'core-key', documentOpen: true }; + const manager = new RuntimeManager( + routerStub, + { + clear: () => undefined, + resolve: async () => resolvedCore(state.key), + }, + () => { + const entry = fakeRuntime(); + runtimes.push(entry); + return entry.runtime; + }, + silentLogger, + { + folderMode: () => 'bridged', + documentIsOpen: () => state.documentOpen, + onDocumentFailure: (failure) => failures.push(failure.error), + }, + ); + return { manager, runtimes, failures, state }; +} + +/** Release start gates (from index `from` on) until `pending` settles. */ +async function settleWithStartsReleased( + harness: Harness, + pending: Promise, + from = 0, +): Promise { + let settled = false; + void pending.then(() => { + settled = true; + }); + await rs.waitUntil(() => { + for (const entry of harness.runtimes.slice(from)) entry.releaseStart(); + return settled; + }, WAIT); +} + +describe('RuntimeManager reconcile-during-start', () => { + it('keeps the pending runtime when a second reconcile resolves to the same key', async () => { + const harness = createHarness(); + const document = documentOf('/project/src/index.ts'); + const first = harness.manager.reconcile(document); + await rs.waitUntil(() => harness.runtimes.length === 1, WAIT); + + // A detection pass / onDidOpen landing inside the startup window. + const second = harness.manager.reconcile(document); + await settleWithStartsReleased(harness, Promise.all([first, second])); + + expect(harness.failures).toEqual([]); + expect(harness.runtimes.length).toBe(1); + expect(harness.runtimes[0].closes).toBe(0); + expect(harness.runtimes[0].aborted).toBe(false); + + await harness.manager.close(); + }); + + it('cancels a pending start immediately when the resolution moves to another key', async () => { + const harness = createHarness(); + const document = documentOf('/project/src/index.ts'); + const first = harness.manager.reconcile(document); + await rs.waitUntil(() => harness.runtimes.length === 1, WAIT); + + // The core changed under a start that never settles (hung worker): the + // superseding reconcile must abort it rather than queue behind it. Only + // the replacement's gate is released; the first start stays hung. + harness.state.key = 'other-core-key'; + const second = harness.manager.reconcile(document); + await rs.waitUntil(() => harness.runtimes[0].aborted, WAIT); + await settleWithStartsReleased(harness, Promise.all([first, second]), 1); + + expect(harness.failures).toEqual([]); + expect(harness.runtimes.length).toBe(2); + expect(harness.runtimes[0].closes).toBe(1); + expect(harness.runtimes[1].aborted).toBe(false); + expect(harness.runtimes[1].closes).toBe(0); + + await harness.manager.close(); + }); + + it('releases the pending runtime when the document closes during start', async () => { + const harness = createHarness(); + const document = documentOf('/project/src/index.ts'); + const first = harness.manager.reconcile(document); + await rs.waitUntil(() => harness.runtimes.length === 1, WAIT); + + harness.state.documentOpen = false; + harness.manager.documentClosed(document); + await rs.waitUntil(() => harness.runtimes[0].closes === 1, WAIT); + await first; + + expect(harness.failures).toEqual([]); + expect(harness.runtimes.length).toBe(1); + + await harness.manager.close(); + }); +});