diff --git a/.agents/notes/implemented/testing/2026-07-22-cross-platform-test-fixtures.i18n.yaml b/.agents/notes/implemented/testing/2026-07-22-cross-platform-test-fixtures.i18n.yaml index d1b133cb72..f5fc9ecef6 100644 --- a/.agents/notes/implemented/testing/2026-07-22-cross-platform-test-fixtures.i18n.yaml +++ b/.agents/notes/implemented/testing/2026-07-22-cross-platform-test-fixtures.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write -2026-07-22-cross-platform-test-fixtures.md: 83af904db5d004366021d4ba6bead656ff813dae -2026-07-22-cross-platform-test-fixtures.zh.md: 3570c393f8d2fc3344aa43ff0eb8291500d07e1c +2026-07-22-cross-platform-test-fixtures.md: 6217aabfdbe8f14f869004c8dafb7e19f4b7443a +2026-07-22-cross-platform-test-fixtures.zh.md: 43942ec0468df822d04b39e318010c2b260c734f diff --git a/.agents/notes/implemented/testing/2026-07-22-cross-platform-test-fixtures.md b/.agents/notes/implemented/testing/2026-07-22-cross-platform-test-fixtures.md index 83af904db5..6217aabfdb 100644 --- a/.agents/notes/implemented/testing/2026-07-22-cross-platform-test-fixtures.md +++ b/.agents/notes/implemented/testing/2026-07-22-cross-platform-test-fixtures.md @@ -6,7 +6,7 @@ English | [中文](2026-07-22-cross-platform-test-fixtures.zh.md) ## Problem -The unit and coverage suites run on Windows, macOS, and Linux, but a platform-neutral behavior can be hidden behind a platform-specific fixture. Literal POSIX paths become drive-relative paths on Windows, a hosted `file:` URI can be a valid UNC path there, and numeric file descriptor `0` is not the sole owner of Node's pipe-backed child stdin. POSIX-only filesystem states such as FIFOs, executable mode bits, and directory search bits have no direct Windows fixture. +The unit and coverage suites run on Windows, macOS, and Linux, but a platform-neutral behavior can be hidden behind a platform-specific fixture. Literal POSIX paths become drive-relative paths on Windows, a hosted `file:` URI can be a valid UNC path there, and child-pipe closure or event-loop scheduling does not settle at the same point on every host. POSIX-only filesystem states such as FIFOs, executable mode bits, and directory search bits have no direct Windows fixture. Treating fixture syntax as product behavior either reports false regressions or encourages production normalization that erases native path semantics. @@ -14,18 +14,20 @@ Treating fixture syntax as product behavior either reports false regressions or Tests of platform-neutral behavior construct absolute paths and `file:` URIs with the host's `node:path` and `node:url` APIs, then assert native absolute output or stable workspace-relative output as the contract requires. Invalid-URI fixtures use encodings rejected by `fileURLToPath()` on every supported platform. -Subprocess fixtures that require the parent write side to fail close both the CRT descriptor and the libuv handle owning child stdin. This pins the connection failure contract across POSIX descriptor-backed and Windows pipe-backed processes while keeping the child alive long enough to distinguish pipe failure from process exit. +Transport-failure tests inject the connection's message writer and deliver the same asynchronous write callback error that a real Node stream would report. The production writer still writes framed messages to child stdin. This keeps a real child alive while the test deterministically distinguishes transport failure from process exit without reaching into platform-specific pipe handles. -Tests for a genuinely POSIX-only primitive use a narrow Windows exclusion on that case. Adjacent cross-platform cases continue to pin non-regular file rejection, unavailable command rejection, and inaccessible working-directory rejection. +Language-server teardown targets the whole descendant tree through a negative process-group id on POSIX and synchronous `taskkill /T /F` on Windows. Windows suppresses only taskkill's already-absent-tree status; command, permission, and other tree-kill failures remain teardown failures. A read-only provider query retries once only when its selected pooled transport fails before or during that query; errors from a still-live server are not replayed. Terminal tests wait for their observable rendered output instead of assuming one event-loop turn is sufficient. + +Tests for a genuinely POSIX-only primitive use a narrow Windows exclusion on that case. Adjacent cross-platform cases continue to pin non-regular file rejection, unavailable command rejection, and inaccessible working-directory rejection. Supported Windows paths remain inside the per-file coverage gate rather than being excluded with their test files. ## Alternatives considered **Normalize all paths and URIs to POSIX strings.** This would make assertions uniform but would change correct Windows behavior: external paths are native absolute paths, UNC file URIs are valid, and configured homes resolve through the host path rules. -**Run POSIX fixtures through a compatibility shell on Windows.** A compatibility environment would test different filesystem and process semantics from the native Node runtime exercised by the product. +**Manipulate child-pipe internals until a write fails.** CRT descriptors and libuv handles have different ownership across hosts and Node versions, so this would test undocumented fixture machinery instead of the connection's write-failure contract. **Skip whole files or packages on Windows.** Broad exclusions would hide supported behavior. Only the individual fixture whose state cannot exist on Windows is excluded; the surrounding contract remains covered. ## Consequences -Portable fixtures are slightly more verbose because expected paths derive from shared native constants. Platform-only exclusions require a neighboring cross-platform assertion for the product behavior they support. Pipe-failure fixtures depend on Node's test-runtime handle shape, but that dependency stays inside the scripted child and proves the real parent-side stream behavior rather than mocking it. +Portable fixtures are slightly more explicit because expected paths derive from shared native constants and transport failures enter through a narrow writer seam. Platform-only exclusions require a neighboring cross-platform assertion for the product behavior they support. Windows teardown depends on the host `taskkill` command after graceful protocol shutdown has failed; a successful synchronous result keeps disposal bounded and makes descendant exit observable before cleanup returns, while a failed tree kill remains visible to the disposer. diff --git a/.agents/notes/implemented/testing/2026-07-22-cross-platform-test-fixtures.zh.md b/.agents/notes/implemented/testing/2026-07-22-cross-platform-test-fixtures.zh.md index 3570c393f8..43942ec046 100644 --- a/.agents/notes/implemented/testing/2026-07-22-cross-platform-test-fixtures.zh.md +++ b/.agents/notes/implemented/testing/2026-07-22-cross-platform-test-fixtures.zh.md @@ -6,7 +6,7 @@ Status: implemented ## 问题 -单元测试与覆盖率测试套件会在 Windows、macOS 和 Linux 上运行,但平台无关行为可能被平台特有的 fixture(测试前置数据)掩盖。字面 POSIX 路径在 Windows 上会变成相对于驱动器的路径;带主机名的 `file:` URI 在 Windows 上可能是有效的 UNC 路径;在 Node 中,编号为 `0` 的文件描述符也不是子进程管道型 stdin 的唯一持有者。FIFO、可执行模式位和目录搜索权限位等仅存在于 POSIX 的文件系统状态,在 Windows 上没有可直接构造的 fixture。 +单元测试与覆盖率测试套件会在 Windows、macOS 和 Linux 上运行,但平台无关行为可能被平台特有的 fixture(测试前置数据)掩盖。字面 POSIX 路径在 Windows 上会变成相对于驱动器的路径;带主机名的 `file:` URI 在 Windows 上可能是有效的 UNC 路径;子进程管道关闭或事件循环调度在不同宿主上的稳定时点也不一致。FIFO、可执行模式位和目录搜索权限位等仅存在于 POSIX 的文件系统状态,在 Windows 上没有可直接构造的 fixture。 把 fixture 语法当成产品行为,要么会误报回归,要么会促使生产代码引入抹去原生路径语义的归一化。 @@ -14,18 +14,20 @@ Status: implemented 测试平台无关行为时,使用宿主的 `node:path` 和 `node:url` API 构造绝对路径与 `file:` URI,再根据契约要求断言原生绝对输出或稳定的工作区相对输出。无效 URI fixture 使用一种在所有受支持平台上都会被 `fileURLToPath()` 拒绝的编码形式。 -需要使父进程写端失败的子进程 fixture 会同时关闭 CRT 文件描述符和持有子进程 stdin 的 libuv 句柄。这种方式在以 POSIX 文件描述符为后端的进程和以 Windows 管道为后端的进程上固定了连接失败契约,同时让子进程存活足够长的时间,以区分管道故障与进程退出。 +传输故障测试会注入连接的消息写入器,并传入与真实 Node 流相同的异步写入回调错误。生产写入器仍会把分帧消息写入子进程 stdin。这种方式让真实子进程保持存活,使测试无需触及平台特有的管道句柄,也能确定性地区分传输故障与进程退出。 -对于真正仅存在于 POSIX 的原语,测试只在该用例上排除 Windows。相邻的跨平台用例仍会固定拒绝非普通文件、不可用命令和无法访问的工作目录的行为。 +语言服务器的资源清理会终止整棵后代进程树:POSIX 使用负数进程组 ID,Windows 同步执行 `taskkill /T /F`。Windows 只会忽略 taskkill 返回的「进程树已经不存在」状态;命令执行失败、权限错误及其他终止进程树的失败仍属于资源清理失败。只读的提供方查询仅在选定的池化传输于该次查询开始前或执行期间失效时重试一次;服务器仍存活时返回的错误不会重放。终端测试会等待可观察的渲染输出,不假设一次事件循环轮转已经足够。 + +对于真正仅存在于 POSIX 的原语,测试只在该用例上排除 Windows。相邻的跨平台用例仍会固定拒绝非普通文件、不可用命令和无法访问的工作目录的行为。Windows 上受支持的路径仍受逐文件覆盖率门禁约束,不会随测试文件一起排除。 ## 曾考虑的替代方案 **将所有路径和 URI 归一化为 POSIX 字符串。**这会使断言保持一致,但也会改变正确的 Windows 行为:外部路径是原生绝对路径,UNC 文件 URI 有效,而且已配置的主目录会按照宿主路径规则解析。 -**在 Windows 上通过兼容性 shell 运行 POSIX fixture。**这种兼容环境测试的文件系统与进程语义不同于产品实际使用的原生 Node 运行时。 +**操纵子进程管道内部状态,直至写入失败。**CRT 描述符与 libuv 句柄在不同宿主和 Node 版本上的所有权不同,因此这种做法测试的是未文档化的 fixture 机制,而非连接的写入失败契约。 **在 Windows 上跳过整个测试文件或包。**过宽的排除会隐藏受支持的行为。只排除无法在 Windows 上构造相应状态的单项 fixture;相关契约仍保持覆盖。 ## 后果 -可移植 fixture 略显冗长,因为预期路径需要从共享的原生常量派生。仅适用于特定平台的排除项必须配有相邻的跨平台断言,以继续覆盖相应的产品行为。管道故障 fixture 依赖 Node 测试运行时的句柄形态,但这种依赖仅存在于脚本化的子进程内;因此,这类 fixture 验证的是真实的父进程侧流行为,而不是对它进行 mock。 +可移植 fixture 需要更显式地构造,因为预期路径要从共享的原生常量派生,传输故障则通过狭窄的写入器 seam 注入。仅适用于特定平台的排除项必须配有相邻的跨平台断言,以继续覆盖相应的产品行为。协议级优雅关停失败后,Windows 上的资源清理依赖宿主的 `taskkill` 命令;命令同步执行成功时,dispose 的完成边界明确,并确保清理返回前即可观察到后代进程退出;若进程树终止失败,dispose 的调用方仍能观察到该失败。 diff --git a/packages/lsp/lsp-local/README.md b/packages/lsp/lsp-local/README.md index 85cc7945dd..7c6c05b7df 100644 --- a/packages/lsp/lsp-local/README.md +++ b/packages/lsp/lsp-local/README.md @@ -7,9 +7,10 @@ Namespace plugin (`name` / `inject` / `Config` / `apply`, no default export). ## What it does - Resolves every server-local setting before registration; an invalid mapping or registration conflict rolls back earlier entries, so a failed load leaves no provider routes. -- Lazily single-flights one server process per `(server id, canonical workspace realpath)`. A crash fails the active query without replay; a later query may replace the process. +- Lazily single-flights one server process per `(server id, canonical workspace realpath)`. A live server error is not replayed; if the selected pooled transport fails before or during a read-only query, the provider awaits its disposal and retries that query once on a fresh process. - Uses a compatibility-first **transient-open** sequence per query: canonicalize and read the source with Node APIs, `textDocument/didOpen` (version 1, full text), the requested request, then `textDocument/didClose` in `finally`. A failed or canceled `didOpen` write terminates the instance before the pool can reuse it. Documents close after each call, so the first version needs no `didChange`, content cache, or document LRU. - Serializes each source-read/open/query/close lifecycle through one abortable per-workspace queue so queued calls read current source only when their turn starts; distinct workspaces run in parallel. +- After protocol shutdown fails, terminates the server's descendant tree through POSIX process-group signaling or synchronous Windows `taskkill /T /F`. Windows suppresses only taskkill's already-absent-tree result; command, permission, and other tree-kill failures remain visible. - Reads sources through Node filesystem APIs in the subprocess's host namespace — NOT `ctx.fs`, and emits no `fs/observed`: only the LSP result is model-visible, so a query does not satisfy read-before-write policy. ## Configuration diff --git a/packages/lsp/lsp-local/src/connection.ts b/packages/lsp/lsp-local/src/connection.ts index ae725b56f7..1103c4dbd2 100644 --- a/packages/lsp/lsp-local/src/connection.ts +++ b/packages/lsp/lsp-local/src/connection.ts @@ -8,7 +8,7 @@ */ import type { ChildProcessByStdio } from 'node:child_process' -import { spawn } from 'node:child_process' +import { spawn, spawnSync } from 'node:child_process' import type { Readable, Writable } from 'node:stream' import { setImmediate as yieldToEventLoop } from 'node:timers/promises' import { encodeMessage, MessageDecoder } from './framing.ts' @@ -36,6 +36,132 @@ interface Pending { reject: (error: Error) => void } +/** + * Write one JSON-RPC message to the child stdin. + * @param stdin - the spawned server stdin. + * @param message - the unencoded JSON-RPC message. + * @param done - callback that reports asynchronous stream settlement. + */ +export type ConnectionWriter = ( + stdin: Writable, + message: unknown, + done: (error?: Error | null) => void, +) => void + +/** Host operations used to signal a detached process tree. */ +export interface ProcessTreeOperations { + /** Signal a POSIX process group. */ + readonly signal: (target: number, signal: NodeJS.Signals) => void + /** Signal the direct child when POSIX group signaling is unavailable. */ + readonly killChild: (signal: NodeJS.Signals) => void + /** Terminate a Windows process tree by root pid. */ + readonly taskkill: (pid: number) => void +} + +/** Narrow taskkill runner result used by the Windows process-tree adapter. */ +export interface TaskkillResult { + /** Process exit status, or null when spawning failed. */ + readonly status: number | null + /** Spawn failure, when the executable could not run. */ + readonly error?: Error +} + +/** Invoke a command synchronously for the Windows taskkill adapter. */ +export type TaskkillRunner = ( + command: string, + args: string[], + options: { stdio: 'ignore' }, +) => TaskkillResult + +/** Invoke the host process-signal primitive for a POSIX process group. */ +export type ProcessSignalRunner = (target: number, signal: NodeJS.Signals) => boolean + +const processSignalRunner: ProcessSignalRunner = process.kill.bind(process) + +/** taskkill status for "process not found": the requested process tree is already absent. */ +const TASKKILL_TREE_NOT_FOUND_STATUS = 128 + +const writeConnectionMessage: ConnectionWriter = (stdin, message, done) => { + stdin.write(encodeMessage(message), done) +} + +/** + * Terminate one Windows process tree and wait for taskkill to finish. + * @param pid - root process id. + * @param run - command runner; tests inject results without requiring Windows. + */ +export function taskkillProcessTree( + pid: number, + run: TaskkillRunner = spawnSync, +): void { + const result = run('taskkill', ['/PID', String(pid), '/T', '/F'], { stdio: 'ignore' }) + if (result.error !== undefined) throw result.error + if (result.status === TASKKILL_TREE_NOT_FOUND_STATUS) return + if (result.status !== 0) throw new Error(`taskkill exited with status ${String(result.status)}`) +} + +/** + * Signal one POSIX process group through an injectable host primitive. + * @param target - negative process-group id. + * @param signal - requested signal. + * @param run - host signal runner; tests inject it without touching real processes. + */ +export function signalProcessGroup( + target: number, + signal: NodeJS.Signals, + run: ProcessSignalRunner = processSignalRunner, +): void { + 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. POSIX falls back to the direct + * child; Windows requires taskkill to reach the full tree. + * @param platform - host platform. + * @param pid - detached root process id. + * @param signal - requested termination signal. + * @param operations - host operations. + */ +export function signalProcessTree( + platform: NodeJS.Platform, + pid: number, + signal: NodeJS.Signals, + operations: ProcessTreeOperations, +): void { + if (platform === 'win32') { + operations.taskkill(pid) + return + } + try { + operations.signal(-pid, signal) + } catch { + try { + operations.killChild(signal) + } catch { + // The direct child already exited; teardown remains idempotent. + } + } +} + /** A live JSON-RPC endpoint bound to one child process. */ export class LspConnection { private readonly child: ChildProcessByStdio @@ -50,14 +176,16 @@ export class LspConnection { /** * @param spec - how to launch the server and answer its config requests. * @param onServerRequest - answers a server→client request; rejects to send an error response. + * @param writer - message writer; tests inject callback failures without relying on OS pipe races. */ constructor( private readonly spec: ConnectionSpec, private readonly onServerRequest: (method: string, params: unknown) => Promise, + private readonly writer: ConnectionWriter = writeConnectionMessage, ) { this.decoder = new MessageDecoder(spec.maxMessageBytes) - // `detached` puts the server in its own process group so teardown can signal the WHOLE group - // (via `process.kill(-pid)`), reaching helper processes a language server spawns (e.g. tsserver). + // `detached` gives teardown a process-tree root: POSIX signals its negative process-group id, + // while Windows passes the root pid to taskkill /T so helpers such as tsserver cannot outlive it. this.child = spawn(spec.command, [...spec.args], { cwd: spec.cwd, env: spec.env, @@ -94,6 +222,20 @@ export class LspConnection { return this.stderr.toString('utf8') } + /** Whether the transport has failed even if the child close event has not arrived yet. */ + get failed(): boolean { + return this.closeReason !== undefined + } + + /** + * Test whether a caught error is this connection's retained fatal transport cause. + * @param error - error caught by the instance or provider. + * @returns `true` only when this connection produced that exact failure. + */ + failedWith(error: unknown): boolean { + return this.closeReason === error + } + /** * Send a request and await its result. * @param method - the JSON-RPC method. @@ -147,50 +289,38 @@ export class LspConnection { return this.nextId } - /** Send SIGTERM to the server's process group (idempotent-safe; a dead group ignores it). */ + /** Request termination of the server's process tree. */ terminate(): void { - this.signalGroup('SIGTERM') + this.signalTree('SIGTERM') } - /** Send SIGKILL to the server's process group. */ + /** Force termination of the server's process tree. */ kill(): void { - this.signalGroup('SIGKILL') + this.signalTree('SIGKILL') } /** - * Wait until the owned process group has no members. + * Wait until the owned process tree has exited. * @param signal - optional bound for the wait. - * @returns `true` when the group exited, or `false` when the signal aborted first. + * @returns `true` when the tree exited, or `false` when the signal aborted first. */ - async waitForProcessGroupExit(signal?: AbortSignal): Promise { - while (this.processGroupAlive()) { - if (signal?.aborted) return false - await yieldToEventLoop() - } - return true + async waitForProcessTreeExit(signal?: AbortSignal): Promise { + return await waitForTreeExit(this.processTreeAlive.bind(this), signal) } - /** - * Signal the whole process group (negative pid) so helper processes are reached; fall back to the - * direct child if the group send fails. Never throws — teardown races process exit. - */ - private signalGroup(sig: NodeJS.Signals): void { + /** Signal the whole process tree. */ + private signalTree(sig: NodeJS.Signals): void { const pid = this.child.pid if (pid === undefined) return - try { - process.kill(-pid, sig) - } catch { - // The group is gone (already exited) or could not be signalled; try the direct child. - try { - this.child.kill(sig) - } catch { - // Already dead; nothing to signal. - } - } + signalProcessTree(process.platform, pid, sig, { + signal: signalProcessGroup, + killChild: this.child.kill.bind(this.child), + taskkill: taskkillProcessTree, + }) } - /** Whether the detached process group still has at least one member. */ - private processGroupAlive(): boolean { + /** Whether the detached tree's root or POSIX process group is still alive. */ + private processTreeAlive(): boolean { const pid = this.child.pid /* v8 ignore next -- only an asynchronous spawn failure omits pid; its close path owns cleanup. */ if (pid === undefined) return false @@ -218,7 +348,7 @@ export class LspConnection { // A framing/JSON failure corrupts the stream position irrecoverably: fail the instance and // SIGKILL the whole group so helper processes don't outlive the leader. this.fail(asError(error)) - this.signalGroup('SIGKILL') + this.signalTree('SIGKILL') return } for (const message of messages) this.dispatch(message) @@ -293,7 +423,7 @@ export class LspConnection { reject(error) } try { - this.child.stdin.write(encodeMessage(message), done) + this.writer(this.child.stdin, message, done) /* v8 ignore start -- Node stream write failures are callback-delivered; this guards a nonconforming Writable implementation throwing synchronously. */ } catch (error) { diff --git a/packages/lsp/lsp-local/src/index.ts b/packages/lsp/lsp-local/src/index.ts index 095d761624..dda3558130 100644 --- a/packages/lsp/lsp-local/src/index.ts +++ b/packages/lsp/lsp-local/src/index.ts @@ -2,9 +2,9 @@ * Generic stdio language-server backend for `ctx.lsp`. One plugin instance configures a named table * of server commands and registers one isolated provider for each entry. Every provider lazily * single-flights one server process per canonical workspace realpath, serves transient-open queries - * through it, and evicts a crashed process so a later query can replace it. Providers read sources - * through Node APIs in the host namespace (not `ctx.fs`) and trust their configured servers — no - * sandbox confinement. + * through it, and replaces a selected transport that fails before or during the next read-only + * query. Providers read sources through Node APIs in the host namespace (not `ctx.fs`) + * and trust their configured servers — no sandbox confinement. * * Namespace plugin (named exports, no default export). Lifecycle is effect-scoped: disposal * unregisters from `ctx.lsp` and tears down every live server. @@ -221,15 +221,23 @@ 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) { + // A selected child can have died while idle or fail during the next write. Queries are + // read-only, so replace that transport once and retry transparently. + if (!instance.isTransportFailure(error)) throw error + await instance.dispose() + this.evictIfCurrent(workspace, instance) + this.assertActive(signal) + instance = this.instanceFor(workspace) + return await instance.query(request, source, signal) } finally { - // Drop a crashed slot only when it still owns this instance; a replacement must survive. - if (instance.dead) this.evictIfCurrent(workspace, instance) + // Reach quiescence before dropping a dead slot; a replacement must survive this ownership check. + if (instance.dead) { + await instance.dispose() + this.evictIfCurrent(workspace, instance) + } } }) } diff --git a/packages/lsp/lsp-local/src/instance.ts b/packages/lsp/lsp-local/src/instance.ts index 74381c1483..266dd3c59f 100644 --- a/packages/lsp/lsp-local/src/instance.ts +++ b/packages/lsp/lsp-local/src/instance.ts @@ -17,7 +17,7 @@ import type { import { deadline } from '@deepseek-ai/dsh-timeout' import { abortable, abortError } from './abort.ts' import { LspConnection } from './connection.ts' -import type { ConnectionSpec } from './connection.ts' +import type { ConnectionSpec, ConnectionWriter } from './connection.ts' import type { HostSource } from './host.ts' import type { WireInitializeResult, WireServerCapabilities } from './protocol.ts' import { @@ -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. @@ -58,9 +67,10 @@ export class LspInstance { /** * @param spec - the launch, initialize, and teardown parameters. + * @param writer - optional connection writer used by transport conformance tests. */ - constructor(private readonly spec: InstanceSpec) { - this.connection = new LspConnection(spec, (method, params) => this.answerServerRequest(method, params)) + constructor(private readonly spec: InstanceSpec, writer?: ConnectionWriter) { + this.connection = new LspConnection(spec, (method, params) => this.answerServerRequest(method, params), writer) this.ready = this.initialize() // A handshake rejection must not surface as an unhandled rejection before the first query awaits // it; queries attach the real handler. @@ -70,7 +80,16 @@ export class LspInstance { /** Synchronous liveness check: true once the process has closed or the instance was disposed. */ get dead(): boolean { - return this.processClosed || this.disposed + return this.processClosed || this.disposed || this.connection.failed + } + + /** + * Test whether a caught query error came from this instance's transport. + * @param error - error caught by the provider. + * @returns `true` only for the connection's retained fatal transport cause. + */ + isTransportFailure(error: unknown): boolean { + return this.connection.failedWith(error) } /** @@ -84,7 +103,12 @@ export class LspInstance { // Serialize behind prior work, but observe abort DURING the queue wait too: if an earlier query // hangs (e.g. a signal-less seam caller), a later tool's timeout must still be able to give up // rather than block on the shared tail forever. - const run = abortable(this.queue, signal).then(() => this.runQuery(request, source, signal)) + const run = abortable(this.queue, signal) + .then(() => this.runQuery(request, source, signal)) + .catch(async (error: unknown) => { + if (this.isTransportFailure(error)) await this.startTeardown() + throw error + }) // Keep the tail alive regardless of this query's outcome so the next caller still serializes. The // tail follows the ACTUAL prior work (this.queue), not the abortable view, so a caller giving up // on the wait does not deserialize the queue. @@ -272,7 +296,7 @@ export class LspInstance { try { await this.gracefulShutdown(shutdownDeadline.signal) } catch { - // Graceful shutdown failed or timed out; process-group cleanup below remains authoritative. + // Graceful shutdown failed or timed out; process-tree cleanup below remains authoritative. } finally { shutdownDeadline[Symbol.dispose]() } @@ -286,20 +310,20 @@ export class LspInstance { await abortable(this.connection.closed, signal) } - /** SIGTERM the group, escalate after `killGraceMs`, then await leader and helper exit. */ + /** Terminate the tree, escalate after `killGraceMs`, then await leader and helper exit. */ private async forceTerminate(): Promise { this.connection.terminate() const graceDeadline = deadline(undefined, this.spec.killGraceMs, 'LSP_KILL_GRACE') - let groupExited: boolean + let treeExited: boolean try { - groupExited = await this.connection.waitForProcessGroupExit(graceDeadline.signal) + treeExited = await this.connection.waitForProcessTreeExit(graceDeadline.signal) } finally { graceDeadline[Symbol.dispose]() } - if (!groupExited) this.connection.kill() + escalateProcessTree(treeExited, this.connection.kill.bind(this.connection)) await Promise.all([ this.connection.closed, - this.connection.waitForProcessGroupExit(), + this.connection.waitForProcessTreeExit(), ]) } } diff --git a/packages/lsp/lsp-local/tests/connection.spec.ts b/packages/lsp/lsp-local/tests/connection.spec.ts index 464848bbf5..aa7e819cb6 100644 --- a/packages/lsp/lsp-local/tests/connection.spec.ts +++ b/packages/lsp/lsp-local/tests/connection.spec.ts @@ -1,6 +1,18 @@ -import { afterEach, describe, expect, it } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import { fileURLToPath } from 'node:url' import { LspConnection } from '@deepseek-ai/dsh-lsp-local' +import { + signalProcessGroup, + signalProcessTree, + taskkillProcessTree, + waitForTreeExit, +} from '@deepseek-ai/dsh-lsp-local/src/connection.ts' +import type { + ConnectionWriter, + ProcessSignalRunner, + ProcessTreeOperations, + TaskkillRunner, +} from '@deepseek-ai/dsh-lsp-local/src/connection.ts' const fixtureServer = fileURLToPath(new URL('./fixture-server.ts', import.meta.url)) @@ -53,6 +65,12 @@ describe('LspConnection', () => { await expect(conn.request('textDocument/hover', {})).rejects.toThrow(/server refused the request/) }) + it('treats signaling an already-closed child as a teardown race', async () => { + const conn = connectScript('') + await conn.closed + expect(() => { conn.kill() }).not.toThrow() + }) + it('answers a server workspace/configuration request from static config', async () => { const seen: SeenRequest[] = [] const conn = connect( @@ -125,7 +143,7 @@ describe('LspConnection', () => { }) /** Spawn a raw connection running an inline node script as the "server". */ -function connectScript(script: string, maxStderrBytes = 100_000): LspConnection { +function connectScript(script: string, maxStderrBytes = 100_000, writer?: ConnectionWriter): LspConnection { const conn = new LspConnection({ command: process.execPath, args: ['-e', script], @@ -134,7 +152,7 @@ function connectScript(script: string, maxStderrBytes = 100_000): LspConnection maxMessageBytes: 16_000_000, maxStderrBytes, configuration: null, - }, () => Promise.resolve(null)) + }, () => Promise.resolve(null), writer) open.push(conn) return conn } @@ -209,13 +227,13 @@ describe('LspConnection edge behavior', () => { await expect(conn.request('initialize', {})).rejects.toThrow(/exited|closed/) }) - it.skipIf(process.platform === 'win32')('rejects a pending request when child stdin closes but the process stays alive', async () => { - const conn = connectScript('const stdin=process.stdin; require("node:fs").closeSync(0); stdin._handle?.close(); setInterval(()=>{}, 1000)') - await new Promise(resolve => setTimeout(resolve, 100)) - const timeout = new Promise((_resolve, reject) => { - setTimeout(() => { reject(new Error('request timed out')) }, 1000) - }) - await expect(Promise.race([conn.request('initialize', {}), timeout])).rejects.not.toThrow(/timed out/) + it('rejects a pending request when child stdin fails but the process stays alive', async () => { + const failure = new Error('fixture stdin failure') + const writer: ConnectionWriter = (_stdin, _message, done) => { + queueMicrotask(() => { done(failure) }) + } + const conn = connectScript('setInterval(()=>{}, 1000)', 100_000, writer) + await expect(conn.request('initialize', {})).rejects.toThrow(/fixture stdin failure/) }) it('ignores a frame that is neither a valid request nor a numeric-id response', async () => { @@ -230,6 +248,72 @@ describe('LspConnection edge behavior', () => { }) }) +describe('process-tree signaling', () => { + it('forwards POSIX process-group signals through the host runner', () => { + const run: ProcessSignalRunner = vi.fn(() => true) + signalProcessGroup(-42, 'SIGKILL', run) + 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) + expect(operations.taskkill).toHaveBeenCalledWith(42) + expect(operations.signal).not.toHaveBeenCalled() + + signalProcessTree('linux', 42, 'SIGKILL', operations) + expect(operations.signal).toHaveBeenCalledWith(-42, 'SIGKILL') + }) + + it('surfaces a Windows taskkill failure without downgrading to the direct child', () => { + const fallback = fakeProcessTreeOperations() + vi.mocked(fallback.taskkill).mockImplementation(() => { throw new Error('taskkill unavailable') }) + expect(() => { signalProcessTree('win32', 42, 'SIGTERM', fallback) }).toThrow(/taskkill unavailable/) + expect(fallback.killChild).not.toHaveBeenCalled() + }) + + it('tolerates a POSIX tree-signaling race after the direct child is already gone', () => { + const posixGone = fakeProcessTreeOperations() + vi.mocked(posixGone.signal).mockImplementation(() => { throw new Error('group gone') }) + vi.mocked(posixGone.killChild).mockImplementation(() => { throw new Error('child gone') }) + expect(() => { signalProcessTree('linux', 42, 'SIGKILL', posixGone) }).not.toThrow() + }) + + it('runs taskkill for the full tree, accepts an absent tree, and rejects command failures', () => { + const success: TaskkillRunner = vi.fn(() => ({ status: 0 })) + taskkillProcessTree(42, success) + expect(success).toHaveBeenCalledWith('taskkill', ['/PID', '42', '/T', '/F'], { stdio: 'ignore' }) + + expect(() => { taskkillProcessTree(42, () => ({ status: 128 })) }).not.toThrow() + + const spawnFailure = new Error('cannot spawn taskkill') + expect(() => { taskkillProcessTree(42, () => ({ status: null, error: spawnFailure })) }).toThrow(spawnFailure) + expect(() => { taskkillProcessTree(42, () => ({ status: 1 })) }).toThrow(/status 1/) + }) +}) + +/** Create observable process-tree operations without touching host processes. */ +function fakeProcessTreeOperations(): ProcessTreeOperations { + return { + signal: vi.fn(), + killChild: vi.fn(), + taskkill: vi.fn(), + } +} + /** Poll a predicate until it holds or a deadline elapses. */ async function waitFor(predicate: () => boolean, timeoutMs = 3000): Promise { const start = Date.now() diff --git a/packages/lsp/lsp-local/tests/fixture-server.ts b/packages/lsp/lsp-local/tests/fixture-server.ts index 2b7fb76b2f..1a30ed5628 100644 --- a/packages/lsp/lsp-local/tests/fixture-server.ts +++ b/packages/lsp/lsp-local/tests/fixture-server.ts @@ -16,8 +16,6 @@ * - LSP_FAKE_OPEN_MARKER: appends each didOpen document text as one JSON line to this path. * - LSP_FAKE_INITIALIZED_MARKER: records when the initialized notification is received. * - LSP_FAKE_PAUSE_STDIN_AFTER_INITIALIZED: "1" stops consuming stdin after initialized. - * - LSP_FAKE_CLOSE_STDIN_AFTER_INITIALIZED: "1" closes the stdin pipe after initialization. - * - LSP_FAKE_CLOSE_STDIN_AFTER_REPLY: "1" closes the stdin pipe before the first query response. * - LSP_FAKE_EXIT_DELAY_MS / LSP_FAKE_EXIT_MARKER: delay protocol exit and record exit/termination. * - LSP_FAKE_NO_SHUTDOWN: "1" ignores the shutdown request (forces kill escalation). * - LSP_FAKE_ON_OPEN: server→client request to emit when a didOpen arrives, one of @@ -28,7 +26,7 @@ * Run: node fixture-server.ts (Node's erasable TypeScript syntax support). */ -import { appendFileSync, closeSync } from 'node:fs' +import { appendFileSync } from 'node:fs' const enc = process.env.LSP_FAKE_ENCODING ?? 'utf-16' const sync: unknown = process.env.LSP_FAKE_SYNC !== undefined ? JSON.parse(process.env.LSP_FAKE_SYNC) : 1 @@ -40,8 +38,6 @@ const replyDelayMs = Number(process.env.LSP_FAKE_REPLY_DELAY_MS ?? 0) const openMarker = process.env.LSP_FAKE_OPEN_MARKER const initializedMarker = process.env.LSP_FAKE_INITIALIZED_MARKER const pauseStdinAfterInitialized = process.env.LSP_FAKE_PAUSE_STDIN_AFTER_INITIALIZED === '1' -const closeStdinAfterInitialized = process.env.LSP_FAKE_CLOSE_STDIN_AFTER_INITIALIZED === '1' -const closeStdinAfterReply = process.env.LSP_FAKE_CLOSE_STDIN_AFTER_REPLY === '1' const exitDelayMs = Number(process.env.LSP_FAKE_EXIT_DELAY_MS ?? 0) const exitMarker = process.env.LSP_FAKE_EXIT_MARKER const noShutdown = process.env.LSP_FAKE_NO_SHUTDOWN === '1' @@ -146,14 +142,12 @@ function handle(message: { id?: number; method?: string; params?: unknown; resul if (method === 'initialized') { if (initializedMarker !== undefined) appendFileSync(initializedMarker, 'INITIALIZED\n') if (pauseStdinAfterInitialized) process.stdin.pause() - if (closeStdinAfterInitialized) closeStdinPipe() return } if (method === 'textDocument/didClose') return if (method?.startsWith('textDocument/')) { if (hang) return const reply = (): void => { - if (closeStdinAfterReply) closeStdinPipe() if (errorReply) { send({ id, error: { code: -32000, message: 'server refused the request' } }) } else { @@ -171,13 +165,6 @@ function handle(message: { id?: number; method?: string; params?: unknown; resul if (id !== undefined) send({ id, result: null }) } -/** Close both the CRT descriptor and libuv handle that can own a platform's child-stdin pipe. */ -function closeStdinPipe(): void { - const stdin = process.stdin as NodeJS.ReadStream & { _handle?: { close(): void } } - closeSync(0) - stdin._handle?.close() -} - /** Append one teardown event when the fixture is configured to expose process ordering. */ function markExit(event: string): void { if (exitMarker !== undefined) appendFileSync(exitMarker, `${event}\n`) @@ -209,6 +196,6 @@ function send(message: Record): void { // Keep the event loop alive. process.stdin.resume() -if (pauseStdinAfterInitialized || closeStdinAfterInitialized || closeStdinAfterReply) { +if (pauseStdinAfterInitialized) { setInterval(() => {}, 1000) } diff --git a/packages/lsp/lsp-local/tests/instance.spec.ts b/packages/lsp/lsp-local/tests/instance.spec.ts index 343233c4f5..9f246e602a 100644 --- a/packages/lsp/lsp-local/tests/instance.spec.ts +++ b/packages/lsp/lsp-local/tests/instance.spec.ts @@ -1,9 +1,12 @@ -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' 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' @@ -26,7 +29,11 @@ afterEach(async () => { await rm(root, { recursive: true, force: true }) }) -function makeInstance(env: Record = {}, overrides: Partial = {}): LspInstance { +function makeInstance( + env: Record = {}, + overrides: Partial = {}, + writer?: ConnectionWriter, +): LspInstance { const instance = new LspInstance({ command: process.execPath, args: [fixtureServer], @@ -39,7 +46,7 @@ function makeInstance(env: Record = {}, overrides: Partial { expect(instance.dead).toBe(true) }) - it.skipIf(process.platform === 'win32')('terminates when stdin fails during the didOpen write', async () => { - // Closing stdin after initialized makes a large didOpen fail before `opened` can arm didClose; - // the instance must still become dead so its provider can replace it. - await writeFile(join(ws, 'a.ts'), 'x'.repeat(2_000_000)) - const instance = makeInstance({ LSP_FAKE_CLOSE_STDIN_AFTER_INITIALIZED: '1' }, { + it('terminates when stdin fails during the didOpen write', async () => { + const instance = makeInstance({}, { shutdownTimeoutMs: 100, killGraceMs: 100, - }) + }, failingWriter('textDocument/didOpen')) await expect(run(instance, 'goToDefinition')).rejects.toThrow() expect(instance.dead).toBe(true) }) + it('awaits process exit before rejecting a request write failure', async () => { + const instance = makeInstance({}, { + shutdownTimeoutMs: 100, + killGraceMs: 100, + }, failingWriter('textDocument/definition')) + // The pid is observed only to prove the owned subprocess reached quiescence before rejection. + const pid = (instance as unknown as { connection: { pid: number } }).connection.pid + await expect(run(instance, 'goToDefinition')).rejects.toThrow(/fixture textDocument\/definition failure/) + expect(processAlive(pid)).toBe(false) + }) + it('rejects when the server lacks the operation capability', async () => { const instance = makeInstance({ LSP_FAKE_CAPS: JSON.stringify({ definitionProvider: false }), LSP_FAKE_DEF: 'null' }) await expect(run(instance, 'goToDefinition')).rejects.toThrow(/does not support goToDefinition/) @@ -225,11 +240,10 @@ describe('LspInstance query and abort', () => { await expect(run(instance, 'goToDefinition', controller.signal)).rejects.toThrow(/server refused/) }) - it.skipIf(process.platform === 'win32')('keeps a settled result but awaits teardown when didClose cannot be written', async () => { + it('keeps a settled result but awaits teardown when didClose cannot be written', async () => { const instance = makeInstance({ LSP_FAKE_DEF: 'null', - LSP_FAKE_CLOSE_STDIN_AFTER_REPLY: '1', - }, { shutdownTimeoutMs: 100, killGraceMs: 100 }) + }, { shutdownTimeoutMs: 100, killGraceMs: 100 }, failingWriter('textDocument/didClose')) await expect(run(instance, 'goToDefinition')).resolves.toEqual({ kind: 'locations', locations: [], @@ -240,6 +254,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({ @@ -281,7 +303,7 @@ describe('LspInstance disposal', () => { await expect(instance.dispose()).resolves.toBeUndefined() }) - it.skipIf(process.platform === 'win32')('awaits a surviving process-group helper on every concurrent dispose', async () => { + it('awaits a surviving process-tree helper on every concurrent dispose', async () => { const marker = join(root, 'helper.pid') const helper = 'process.on("SIGTERM",()=>{});setInterval(()=>{},1000);' const script = 'const{spawn}=require("node:child_process");const{writeFileSync}=require("node:fs");' @@ -298,6 +320,7 @@ describe('LspInstance disposal', () => { await first } finally { if (processAlive(helperPid)) process.kill(helperPid, 'SIGKILL') + await waitForProcessExit(helperPid) } }) @@ -322,6 +345,26 @@ function processAlive(pid: number): boolean { } } +/** Wait until a process id disappears so temporary-workspace cleanup cannot race handle release. */ +async function waitForProcessExit(pid: number, timeoutMs = 3_000): Promise { + const started = Date.now() + while (processAlive(pid)) { + if (Date.now() - started > timeoutMs) throw new Error(`process ${pid} did not exit`) + await new Promise(resolve => setTimeout(resolve, 10)) + } +} + +/** Write normally except for one method whose callback receives a deterministic transport error. */ +function failingWriter(method: string): ConnectionWriter { + return (stdin, message, done) => { + if ((message as { method?: unknown }).method === method) { + queueMicrotask(() => { done(new Error(`fixture ${method} failure`)) }) + return + } + stdin.write(encodeMessage(message), done) + } +} + /** Wait until a fixture marker exists, bounded so a broken handshake cannot hang the test. */ async function waitForFile(path: string, timeoutMs = 3000): Promise { const started = Date.now() diff --git a/packages/lsp/lsp-local/tests/lifecycle.spec.ts b/packages/lsp/lsp-local/tests/lifecycle.spec.ts index 7ba76d03de..47b8d78bb4 100644 --- a/packages/lsp/lsp-local/tests/lifecycle.spec.ts +++ b/packages/lsp/lsp-local/tests/lifecycle.spec.ts @@ -122,9 +122,15 @@ describe('lsp-local end to end over a fake server', () => { await ctx.fiber.dispose() }) - it('rejects a non-utf-16 position encoding at initialize', async () => { - const ctx = await mount({ LSP_FAKE_ENCODING: 'utf-8', LSP_FAKE_DEF: 'null' }) + it('rejects a non-utf-16 position encoding at initialize without retrying', async () => { + const marker = join(root, 'initialize-rejection-exit.log') + const ctx = await mount({ + LSP_FAKE_ENCODING: 'utf-8', + LSP_FAKE_DEF: 'null', + LSP_FAKE_EXIT_MARKER: marker, + }) await expect(ctx.lsp.query(query('goToDefinition'))).rejects.toThrow(/unsupported position encoding/) + expect(await readFile(marker, 'utf8')).toBe('EXIT\nCLEAN\n') await ctx.fiber.dispose() }) @@ -237,7 +243,7 @@ describe('lsp-local end to end over a fake server', () => { await ctx.fiber.dispose() }) - it.skipIf(process.platform === 'win32')('evicts a pooled server that died while idle and serves the next query from a fresh one', async () => { + it('evicts a pooled server that died while idle and serves the next query from a fresh one', async () => { // The first query succeeds, then the server exits before the second arrives, leaving a dead // instance in the pool. The next query must evict-and-replace it and still succeed, rather than // failing once on the closed connection first. diff --git a/packages/lsp/lsp-local/tests/provider.spec.ts b/packages/lsp/lsp-local/tests/provider.spec.ts index 7a969781f3..829a84264b 100644 --- a/packages/lsp/lsp-local/tests/provider.spec.ts +++ b/packages/lsp/lsp-local/tests/provider.spec.ts @@ -1,7 +1,7 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest' import { chmod, mkdtemp, mkdir, rm, writeFile, realpath } from 'node:fs/promises' import { tmpdir } from 'node:os' -import { join } from 'node:path' +import { delimiter, join } from 'node:path' import { Context } from 'cordis' import Lsp, { type LspQueryRequest } from '@deepseek-ai/dsh-lsp' import * as LspLocal from '@deepseek-ai/dsh-lsp-local' @@ -57,7 +57,7 @@ describe('lsp-local provider resolution', () => { await expect(ctx.plugin(LspLocal, config('nope', { command: 'fake-lsp', args: [], - env: { PATH: `::${join(root, 'empty')}` }, + env: { PATH: `${delimiter}${delimiter}${join(root, 'empty')}` }, extensionToLanguage: { '.ts': 'typescript' }, }))).rejects.toThrow(/was not found on PATH/) await ctx.fiber.dispose() diff --git a/packages/ui/tui/tests/tui.spec.ts b/packages/ui/tui/tests/tui.spec.ts index 7cee8fa3ae..5cbbd8ed62 100644 --- a/packages/ui/tui/tests/tui.spec.ts +++ b/packages/ui/tui/tests/tui.spec.ts @@ -198,7 +198,7 @@ describe('pi-tui chat lifecycle and transcript', () => { await dispose(result) }) - it.skipIf(process.platform === 'win32')('renders its header, footer, replay, streaming answer, todos, and status', async () => { + it('renders its header, footer, replay, streaming answer, todos, and status', async () => { let now = 0 const result = await setup({ contextWindow: 100, @@ -320,7 +320,9 @@ describe('pi-tui chat lifecycle and transcript', () => { { inputTokens: 500, outputTokens: 8 }, { turn: 3, step: 1 }, ) - await tick() + await vi.waitFor(() => { + expect(result.terminal.output).toContain('final live answer') + }) expect(result.terminal.output).toContain('◒ Working · 8s') expect(result.terminal.output).toContain('esc interrupt') @@ -328,7 +330,6 @@ describe('pi-tui chat lifecycle and transcript', () => { expect(result.terminal.output).toContain('user context') expect(result.terminal.output).toContain('Prompt blocked') expect(result.terminal.output).toContain('Turn cancelled') - expect(result.terminal.output).toContain('final live answer') expect(result.terminal.progress).toContain(true) result.session.append('assistant/chunk', { diff --git a/vitest.config.ts b/vitest.config.ts index 8baa7c7b32..c5b5c06d11 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -11,17 +11,6 @@ const windowsUnsupportedPackages = process.platform === 'win32' ] : [] -// These files retain 100% per-file coverage on POSIX, where their process-pipe and terminal timing -// tests are deterministic; Windows skips those cases and must not fail solely on their uncovered paths. -const windowsCoverageExclusions = process.platform === 'win32' - ? [ - 'packages/lsp/lsp-local/src/connection.ts', - 'packages/lsp/lsp-local/src/index.ts', - 'packages/lsp/lsp-local/src/instance.ts', - 'packages/ui/tui/src/index.ts', - ] - : [] - export default defineConfig({ // Native path resolution reads each package's nearest tsconfig, but only the root defines // workspace paths. Keep this plugin pinned to the root map so unbuilt bare package imports resolve @@ -44,7 +33,6 @@ export default defineConfig({ 'packages/*/*/src/bin.ts', 'packages/*/*/src/worker.ts', ...windowsUnsupportedPackages.map(path => `${path}/src/**/*.ts`), - ...windowsCoverageExclusions, ], // 100% or it doesn't merge (docs/testing.md: excessive tests are welcome). // Per-file so a well-covered big file can't subsidize a bare one.