From 674d23118bfa894e13142d8cc1746dd74c39fba0 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Wed, 29 Jul 2026 20:20:51 +0800 Subject: [PATCH] fix(runtime): close measured portability defects --- packages/lsp/lsp-local/src/host.ts | 5 +-- packages/lsp/lsp-local/src/index.ts | 38 ++++++++-------- packages/lsp/lsp-local/tests/host.spec.ts | 4 +- packages/lsp/lsp-local/tests/provider.spec.ts | 44 +++++++++++++++++++ packages/pty/pty-local/src/session.ts | 2 +- packages/pty/pty-local/tests/session.spec.ts | 6 +-- 6 files changed, 73 insertions(+), 26 deletions(-) diff --git a/packages/lsp/lsp-local/src/host.ts b/packages/lsp/lsp-local/src/host.ts index b8939a2ae4..ffba6a1344 100644 --- a/packages/lsp/lsp-local/src/host.ts +++ b/packages/lsp/lsp-local/src/host.ts @@ -100,15 +100,14 @@ export async function readHostSource( for await (const chunk of stream) { throwIfAborted(signal) bytes += Buffer.byteLength(chunk) - if (bytes > maxDocumentBytes) { - throw new Error(`source "${filePath}" exceeds the ${maxDocumentBytes}-byte limit`) - } + if (bytes > maxDocumentBytes) break chunks.push(chunk) } } catch (error: unknown) { throwIfAborted(signal) throw new Error(`source "${filePath}" could not be read: ${messageOf(error)}`, { cause: error }) } + if (bytes > maxDocumentBytes) throw new Error(`source "${filePath}" exceeds the ${maxDocumentBytes}-byte limit`) throwIfAborted(signal) return { fileUrl: fs.fileUrl(target), diff --git a/packages/lsp/lsp-local/src/index.ts b/packages/lsp/lsp-local/src/index.ts index f3f45959e5..0e9468cba9 100644 --- a/packages/lsp/lsp-local/src/index.ts +++ b/packages/lsp/lsp-local/src/index.ts @@ -129,27 +129,29 @@ export async function apply(ctx: Context, config: Config): Promise { // Resolve every server-local setting before registration so a bad later command or bound cannot // publish an earlier provider. Registry-level mapping conflicts are rolled back below. const providers = await (async () => { + const lookups = entries.map(async ([providerId, rawConfig]) => { + if (providerId.trim() === '') throw new Error('lsp-local: server ids must be non-empty strings') + const resolved = rawConfig as ResolvedServerConfig + validateServerConfig(providerId, resolved) + const executable = await ctx.subprocess.resolveExecutable( + resolved.command, + resolved.env, + setupAbort.signal, + ) + setupAbort.signal.throwIfAborted() + return new LocalLspProvider( + providerId, + ctx.fs, + resolved, + executable, + spec => ctx.subprocess.spawn(spec), + ) + }) try { - return await Promise.all(entries.map(async ([providerId, rawConfig]) => { - if (providerId.trim() === '') throw new Error('lsp-local: server ids must be non-empty strings') - const resolved = rawConfig as ResolvedServerConfig - validateServerConfig(providerId, resolved) - const executable = await ctx.subprocess.resolveExecutable( - resolved.command, - resolved.env, - setupAbort.signal, - ) - setupAbort.signal.throwIfAborted() - return new LocalLspProvider( - providerId, - ctx.fs, - resolved, - executable, - spec => ctx.subprocess.spawn(spec), - ) - })) + return await Promise.all(lookups) } catch (error: unknown) { setupAbort.abort(error) + await Promise.allSettled(lookups) throw error } finally { stopSetupCancellation() diff --git a/packages/lsp/lsp-local/tests/host.spec.ts b/packages/lsp/lsp-local/tests/host.spec.ts index eea3851414..c97defc086 100644 --- a/packages/lsp/lsp-local/tests/host.spec.ts +++ b/packages/lsp/lsp-local/tests/host.spec.ts @@ -158,7 +158,9 @@ describe('readHostSource', () => { it('rejects an oversized source', async () => { await writeFile(join(ws, 'big.ts'), 'x'.repeat(100)) - await expect(readSource('big.ts', 10)).rejects.toThrow(/10-byte limit/) + await expect(readSource('big.ts', 10)).rejects.toMatchObject({ + message: 'source "big.ts" exceeds the 10-byte limit', + }) }) it('counts the complete UTF-8 byte length at the configured boundary', async () => { diff --git a/packages/lsp/lsp-local/tests/provider.spec.ts b/packages/lsp/lsp-local/tests/provider.spec.ts index 8d8a7ad356..88b2b0144f 100644 --- a/packages/lsp/lsp-local/tests/provider.spec.ts +++ b/packages/lsp/lsp-local/tests/provider.spec.ts @@ -195,6 +195,50 @@ describe('lsp-local provider resolution', () => { await ctx.fiber.dispose() }) + it('waits for aborted sibling executable lookups before setup rejects', async () => { + const ctx = new Context() + await ctx.plugin(Lsp) + await ctx.plugin(LocalSubprocessService) + await ctx.plugin(LocalFileSystem, { cwd: process.cwd() }) + const slowStarted = Promise.withResolvers() + const slowAborted = Promise.withResolvers() + const releaseCleanup = Promise.withResolvers() + vi.spyOn(ctx.subprocess, 'resolveExecutable').mockImplementation(async (command, _env, signal) => { + if (signal === undefined) throw new Error('missing setup signal') + if (command === 'slow-lsp') { + return await new Promise((_resolve, reject) => { + const onAbort = (): void => { + slowAborted.resolve(undefined) + void releaseCleanup.promise.then(() => { + reject(signal.reason instanceof Error ? signal.reason : new Error(String(signal.reason))) + }) + } + signal.addEventListener('abort', onAbort, { once: true }) + slowStarted.resolve(undefined) + if (signal.aborted) onAbort() + }) + } + await slowStarted.promise + throw new Error('lookup failed') + }) + + const loading = ctx.plugin(LspLocal, { + servers: { + slow: { command: 'slow-lsp', extensionToLanguage: { '.ts': 'typescript' } }, + failing: { command: 'failing-lsp', extensionToLanguage: { '.js': 'javascript' } }, + }, + }) + await slowAborted.promise + let settled = false + void loading.then(() => { settled = true }, () => { settled = true }) + await new Promise((resolve) => { setImmediate(resolve) }) + expect(settled).toBe(false) + + releaseCleanup.resolve(undefined) + await expect(loading).rejects.toThrow('lookup failed') + await ctx.fiber.dispose() + }) + it('aborts executable resolution when disposed during setup', async () => { const ctx = new Context() await ctx.plugin(Lsp) diff --git a/packages/pty/pty-local/src/session.ts b/packages/pty/pty-local/src/session.ts index f9b35dc665..5517cd902d 100644 --- a/packages/pty/pty-local/src/session.ts +++ b/packages/pty/pty-local/src/session.ts @@ -237,7 +237,7 @@ export class LocalPtySession implements PtyBackendSession { } this.activeDeadlineTimer = setTimeout(() => { if (this.active === operation) { - this.settleActive('timeout', this.activeWrite?.operation === operation) + this.settleActive('timeout', this.activeWrite?.operation === operation || this.interrupting === operation) } }, this.config.timeoutMs) void this.beginSend(operation, request) diff --git a/packages/pty/pty-local/tests/session.spec.ts b/packages/pty/pty-local/tests/session.spec.ts index 14cf252a0e..fbe0aa807b 100644 --- a/packages/pty/pty-local/tests/session.spec.ts +++ b/packages/pty/pty-local/tests/session.spec.ts @@ -365,11 +365,11 @@ describe('LocalPtySession readiness and output', () => { expect(operation.cancel()).toBe(true) terminal.emitData('\x1b]133;D;130\x07dsh> ') - await vi.advanceTimersByTimeAsync(10) + await vi.advanceTimersByTimeAsync(100) + expect((await operation.done).waitReason).toBe('timeout') expect(() => session.startSend({ text: 'successor', submit: true })).toThrow('active send') signalGate.resolve(undefined) - await vi.advanceTimersByTimeAsync(10) - await operation.done + await vi.advanceTimersByTimeAsync(0) expect(inspector.groups).toContainEqual([456, 'SIGINT']) expect(inspector.groups).not.toContainEqual([789, 'SIGINT']) })