From 7b6b544243b1109d23a21e7e10d9db1cd62d2389 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Wed, 22 Jul 2026 14:34:26 +0800 Subject: [PATCH] test(windows): cover teardown decisions deterministically --- packages/lsp/lsp-local/src/connection.ts | 25 +++++++++++++++---- packages/lsp/lsp-local/src/index.ts | 4 --- packages/lsp/lsp-local/src/instance.ts | 11 +++++++- .../lsp/lsp-local/tests/connection.spec.ts | 14 +++++++++++ packages/lsp/lsp-local/tests/instance.spec.ts | 11 +++++++- 5 files changed, 54 insertions(+), 11 deletions(-) diff --git a/packages/lsp/lsp-local/src/connection.ts b/packages/lsp/lsp-local/src/connection.ts index 201cd87bdd..7ed3e09054 100644 --- a/packages/lsp/lsp-local/src/connection.ts +++ b/packages/lsp/lsp-local/src/connection.ts @@ -110,6 +110,25 @@ export function signalProcessGroup( run(target, signal) } +/** + * Wait until a process-tree liveness probe reports exit. + * @param isAlive - process-tree liveness probe. + * @param signal - optional bound for the wait. + * @param yieldNow - event-loop yield primitive. + * @returns `true` when the tree exited, or `false` when the signal aborted first. + */ +export async function waitForTreeExit( + isAlive: () => boolean, + signal?: AbortSignal, + yieldNow: () => Promise = yieldToEventLoop, +): Promise { + while (isAlive()) { + if (signal?.aborted) return false + await yieldNow() + } + return true +} + /** * Signal a detached process tree with platform-correct semantics and a direct-child fallback. * @param platform - host platform. @@ -269,11 +288,7 @@ export class LspConnection { * @returns `true` when the tree exited, or `false` when the signal aborted first. */ async waitForProcessTreeExit(signal?: AbortSignal): Promise { - while (this.processTreeAlive()) { - if (signal?.aborted) return false - await yieldToEventLoop() - } - return true + return await waitForTreeExit(this.processTreeAlive.bind(this), signal) } /** diff --git a/packages/lsp/lsp-local/src/index.ts b/packages/lsp/lsp-local/src/index.ts index 340d5a4df0..7a4b1b6b6d 100644 --- a/packages/lsp/lsp-local/src/index.ts +++ b/packages/lsp/lsp-local/src/index.ts @@ -221,10 +221,6 @@ class LocalLspProvider implements LspProvider { // synchronous get-or-create so every spawned process remains owned by teardown. this.assertActive(signal) let instance = this.instanceFor(workspace) - if (instance.dead) { - this.evictIfCurrent(workspace, instance) - instance = this.instanceFor(workspace) - } try { return await instance.query(request, source, signal) } catch (error) { diff --git a/packages/lsp/lsp-local/src/instance.ts b/packages/lsp/lsp-local/src/instance.ts index 02bdd160d4..eecc5d4e5e 100644 --- a/packages/lsp/lsp-local/src/instance.ts +++ b/packages/lsp/lsp-local/src/instance.ts @@ -39,6 +39,15 @@ export interface InstanceSpec extends ConnectionSpec { readonly killGraceMs: number } +/** + * Force-kill a process tree only when graceful termination did not make it exit. + * @param treeExited - whether the tree exited within its grace period. + * @param forceKill - forceful process-tree termination primitive. + */ +export function escalateProcessTree(treeExited: boolean, forceKill: () => void): void { + if (!treeExited) forceKill() +} + /** * A single initialized server process. Not exported as a provider — the provider single-flights and * pools these. `query()` serializes; `dispose()` rejects queued work and tears the process down. @@ -297,7 +306,7 @@ export class LspInstance { } finally { graceDeadline[Symbol.dispose]() } - if (!treeExited) this.connection.kill() + escalateProcessTree(treeExited, this.connection.kill.bind(this.connection)) await Promise.all([ this.connection.closed, this.connection.waitForProcessTreeExit(), diff --git a/packages/lsp/lsp-local/tests/connection.spec.ts b/packages/lsp/lsp-local/tests/connection.spec.ts index 18cb7a1bce..9fea82b43f 100644 --- a/packages/lsp/lsp-local/tests/connection.spec.ts +++ b/packages/lsp/lsp-local/tests/connection.spec.ts @@ -5,6 +5,7 @@ import { signalProcessGroup, signalProcessTree, taskkillProcessTree, + waitForTreeExit, } from '@deepseek-ai/dsh-lsp-local/src/connection.ts' import type { ConnectionWriter, @@ -248,6 +249,19 @@ describe('process-tree signaling', () => { expect(run).toHaveBeenCalledWith(-42, 'SIGKILL') }) + it('waits for tree exit and stops when its bound aborts', async () => { + const isAlive = vi.fn() + .mockReturnValueOnce(true) + .mockReturnValue(false) + const yieldNow = vi.fn(() => Promise.resolve()) + await expect(waitForTreeExit(isAlive, undefined, yieldNow)).resolves.toBe(true) + expect(yieldNow).toHaveBeenCalledOnce() + + const controller = new AbortController() + controller.abort() + await expect(waitForTreeExit(() => true, controller.signal, yieldNow)).resolves.toBe(false) + }) + it('uses taskkill for a Windows tree and a negative pid for a POSIX group', () => { const operations = fakeProcessTreeOperations() signalProcessTree('win32', 42, 'SIGTERM', operations) diff --git a/packages/lsp/lsp-local/tests/instance.spec.ts b/packages/lsp/lsp-local/tests/instance.spec.ts index cfa8120dbc..d08a431192 100644 --- a/packages/lsp/lsp-local/tests/instance.spec.ts +++ b/packages/lsp/lsp-local/tests/instance.spec.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { mkdtemp, mkdir, readFile, rm, writeFile, realpath } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -6,6 +6,7 @@ import { pathToFileURL, fileURLToPath } from 'node:url' import { LspInstance, readHostSource } from '@deepseek-ai/dsh-lsp-local' import { encodeMessage } from '@deepseek-ai/dsh-lsp-local' import type { ConnectionWriter } from '@deepseek-ai/dsh-lsp-local/src/connection.ts' +import { escalateProcessTree } from '@deepseek-ai/dsh-lsp-local/src/instance.ts' import type { InstanceSpec } from '@deepseek-ai/dsh-lsp-local/src/instance.ts' import type { LspProviderQuery, LspQueryResult } from '@deepseek-ai/dsh-lsp' @@ -242,6 +243,14 @@ describe('LspInstance query and abort', () => { }) describe('LspInstance disposal', () => { + it('escalates only when the process tree survives its grace period', () => { + const forceKill = vi.fn() + escalateProcessTree(false, forceKill) + expect(forceKill).toHaveBeenCalledOnce() + escalateProcessTree(true, forceKill) + expect(forceKill).toHaveBeenCalledOnce() + }) + it('lets a server finish protocol exit before signal escalation', async () => { const marker = join(root, 'graceful-exit.log') const instance = makeInstance({