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(); + }); +});