From bc60b9ffe808f422d7563d689a1a8c5cfede6cd6 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Tue, 21 Jul 2026 13:52:15 +0800 Subject: [PATCH] fix(lsp): cancel blocked document opens --- .../2026-07-15-lsp-capability-seam.i18n.yaml | 4 +- .../2026-07-15-lsp-capability-seam.md | 4 +- .../2026-07-15-lsp-capability-seam.zh.md | 4 +- packages/lsp/lsp-local/README.md | 2 +- packages/lsp/lsp-local/src/instance.ts | 13 +++-- .../lsp/lsp-local/tests/fixture-server.ts | 18 ++++++- packages/lsp/lsp-local/tests/instance.spec.ts | 49 +++++++++++++++++++ 7 files changed, 82 insertions(+), 12 deletions(-) diff --git a/.agents/notes/implemented/architecture/2026-07-15-lsp-capability-seam.i18n.yaml b/.agents/notes/implemented/architecture/2026-07-15-lsp-capability-seam.i18n.yaml index 82433d5fff..70b98f0cf1 100644 --- a/.agents/notes/implemented/architecture/2026-07-15-lsp-capability-seam.i18n.yaml +++ b/.agents/notes/implemented/architecture/2026-07-15-lsp-capability-seam.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-15-lsp-capability-seam.md: 91b15e8f9ff044d2c438c87040f9fc19a8dabc6a -2026-07-15-lsp-capability-seam.zh.md: 39e63370241e8bbeb93ea7bb81fbd951fe807b19 +2026-07-15-lsp-capability-seam.md: 7265b04ac9b2f83764bdd13f07b2d3404c4c1708 +2026-07-15-lsp-capability-seam.zh.md: 10e8956005045d0934dd9dada5718b85a34cda3f diff --git a/.agents/notes/implemented/architecture/2026-07-15-lsp-capability-seam.md b/.agents/notes/implemented/architecture/2026-07-15-lsp-capability-seam.md index 91b15e8f9f..7265b04ac9 100644 --- a/.agents/notes/implemented/architecture/2026-07-15-lsp-capability-seam.md +++ b/.agents/notes/implemented/architecture/2026-07-15-lsp-capability-seam.md @@ -119,7 +119,7 @@ The `read` tool is unsuitable source because its output is windowed, numbered, t The local provider uses a compatibility-first transient-open sequence for every query. It accepts legacy `textDocumentSync` `Full` or `Incremental`, or options with `openClose: true`; omitted, `None`, or explicitly incompatible synchronization fails as unsupported before `didOpen`. 1. Canonicalize and validate the host path, then read the current source with Node filesystem APIs. -2. Send `textDocument/didOpen` with version `1`, full text, and the configured language id. +2. Send `textDocument/didOpen` with version `1`, full text, and the configured language id. Its write remains abortable; failure or cancellation invalidates the instance and awaits bounded process termination before the pool can reuse it. 3. Send the requested `textDocument/definition`, `textDocument/references`, `textDocument/implementation`, or `textDocument/hover` request. 4. If `didOpen` succeeded, attempt `textDocument/didClose` in `finally` after the request settles or aborts. A close-write failure does not replace the settled result or error, but invalidates the instance and awaits bounded process termination. @@ -177,7 +177,7 @@ The local provider trusts its configured server and claims no sandbox confinemen - Tool tests pin the four operations, coordinate validation, configured bounds and omission markers, prompt, and ACP presentation. - Registry tests pin atomic reservation/release, order-independent selection, and structured unavailable, disposed, conflict, and unsupported-operation errors. - Fake-stdio tests pin exact initialization capabilities, four protocol mappings, `Location`/`LocationLink` and hover normalization, and `findReferences` mapping to `references.includeDeclaration`. -- Synchronization tests pin UTF-16 negotiation and conversion, supported and rejected `textDocumentSync` forms, balanced transient open/close, close-write failure, and malformed-response rejection. +- Synchronization tests pin UTF-16 negotiation and conversion, supported and rejected `textDocumentSync` forms, blocked and failed open writes, balanced transient open/close, close-write failure, and malformed-response rejection. - Timeout tests pin one `TOOL_TIMEOUT` budget, unclassified upstream cancellation, no hidden seam deadline, and bounded awaited teardown. - Lifecycle tests pin startup single-flight, complete-lifecycle serialization with fresh queued source reads, cross-workspace parallelism, abortable queues, crash replacement without replay, failed-stdin teardown, and quiescent disposal. - Host-filesystem tests pin session-cwd requirements, relative and absolute source containment through symlinks, document validation, file/non-file URI rendering, unformatted source, and no `fs/observed` event. diff --git a/.agents/notes/implemented/architecture/2026-07-15-lsp-capability-seam.zh.md b/.agents/notes/implemented/architecture/2026-07-15-lsp-capability-seam.zh.md index 39e6337024..10e8956005 100644 --- a/.agents/notes/implemented/architecture/2026-07-15-lsp-capability-seam.zh.md +++ b/.agents/notes/implemented/architecture/2026-07-15-lsp-capability-seam.zh.md @@ -119,7 +119,7 @@ ACP 使用 `{ card: 'generic', kind: 'search', title, locations: [{ path: file_p 本地提供方对每次查询都采用兼容优先的临时打开流程。它接受旧式 `textDocumentSync` 的 `Full` 或 `Incremental`，也接受设置了 `openClose: true` 的选项；同步能力缺失、为 `None` 或明确不兼容时，在 `didOpen` 前以不支持错误失败。 1. 规范化并校验主机路径，再使用 Node 文件系统 API 读取当前源文件。 -2. 发送 `textDocument/didOpen`，其中包含版本 `1`、完整文本和配置的语言 id。 +2. 发送 `textDocument/didOpen`，其中包含版本 `1`、完整文本和配置的语言 id。该写入仍可取消；写入失败或遭取消会使实例失效，并等待有界进程终止完成，池才能复用它。 3. 发送所请求的 `textDocument/definition`、`textDocument/references`、`textDocument/implementation` 或 `textDocument/hover` 请求。 4. 如果 `didOpen` 成功，则在请求完成或取消后于 `finally` 中尝试发送 `textDocument/didClose`。关闭写入失败不会覆盖已经确定的结果或错误，但会使实例失效，并等待有界进程终止完成。 @@ -177,7 +177,7 @@ ACP 使用 `{ card: 'generic', kind: 'search', title, locations: [{ path: file_p - 工具测试固定四种操作、坐标校验、配置限制与省略标记、提示词和 ACP 展示。 - 注册表测试固定原子占用/释放、不受顺序影响的选择，以及结构化的不可用、已释放、冲突和不支持操作错误。 - 测试用 stdio server 固定精确的初始化能力、四种协议映射、`Location`/`LocationLink` 与 `hover` 归一化，以及 `findReferences` 到 `references.includeDeclaration` 的映射。 -- 同步测试固定 UTF-16 协商与转换、受支持和被拒绝的 `textDocumentSync` 形式、配对的临时打开/关闭、关闭写入失败和错误响应拒绝。 +- 同步测试固定 UTF-16 协商与转换、受支持和被拒绝的 `textDocumentSync` 形式、打开写入阻塞与失败、配对的临时打开/关闭、关闭写入失败和错误响应拒绝。 - 超时测试固定一个 `TOOL_TIMEOUT` 预算、不对上游取消错误分类、服务边界无隐藏截止时间，以及受限且等待完成的清理。 - 生命周期测试固定启动 single-flight、完整生命周期串行化及排队查询读取最新源文件、跨工作区并行、可取消队列、崩溃后不重放的替换、stdin 失败后的进程拆除，以及释放后完全停稳。 - 主机文件系统测试固定 session cwd 要求、符号链接下相对与绝对源路径的规范 containment、文档校验、file/non-file URI 渲染、无格式源文本和不发送 `fs/observed`。 diff --git a/packages/lsp/lsp-local/README.md b/packages/lsp/lsp-local/README.md index 66568b17d1..85cc7945dd 100644 --- a/packages/lsp/lsp-local/README.md +++ b/packages/lsp/lsp-local/README.md @@ -8,7 +8,7 @@ Namespace plugin (`name` / `inject` / `Config` / `apply`, no default export). - 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. -- 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`. Documents close after each call, so the first version needs no `didChange`, content cache, or document LRU. +- 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. - 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. diff --git a/packages/lsp/lsp-local/src/instance.ts b/packages/lsp/lsp-local/src/instance.ts index 9d327fb795..74381c1483 100644 --- a/packages/lsp/lsp-local/src/instance.ts +++ b/packages/lsp/lsp-local/src/instance.ts @@ -138,9 +138,16 @@ export class LspInstance { try { /* v8 ignore next -- guards an abort landing between the ready wait and didOpen; not deterministically reproducible. */ if (signal?.aborted) throw abortError(signal) - await this.connection.notify('textDocument/didOpen', { - textDocument: { uri, languageId: request.languageId, version: 1, text: source.text }, - }) + try { + await abortable(this.connection.notify('textDocument/didOpen', { + textDocument: { uri, languageId: request.languageId, version: 1, text: source.text }, + }), signal) + } catch (error) { + // A canceled backpressured write or failed stdin leaves the protocol stream unusable before + // `opened` can arm the didClose cleanup. Teardown here makes the pool evict the instance. + await this.startTeardown() + throw error + } opened = true const payload = await this.sendRequest(request.operation, uri, request.position, signal) return this.normalize(request.operation, payload) diff --git a/packages/lsp/lsp-local/tests/fixture-server.ts b/packages/lsp/lsp-local/tests/fixture-server.ts index 8dec856274..c418ada519 100644 --- a/packages/lsp/lsp-local/tests/fixture-server.ts +++ b/packages/lsp/lsp-local/tests/fixture-server.ts @@ -14,6 +14,9 @@ * simulating a server that dies while idle so the pool holds a dead instance (eviction test). * - LSP_FAKE_REPLY_DELAY_MS: delays each textDocument/* response by this many milliseconds. * - 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 fd 0 after the initialized notification. * - LSP_FAKE_CLOSE_STDIN_AFTER_REPLY: "1" closes fd 0 before sending 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). @@ -35,6 +38,9 @@ const crashOnOpen = process.env.LSP_FAKE_CRASH_ON_OPEN === '1' const exitAfterReply = process.env.LSP_FAKE_EXIT_AFTER_REPLY === '1' 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 @@ -137,7 +143,13 @@ function handle(message: { id?: number; method?: string; params?: unknown; resul if (onOpen !== undefined) emitServerRequest(onOpen) return } - if (method === 'textDocument/didClose' || method === 'initialized') return + if (method === 'initialized') { + if (initializedMarker !== undefined) appendFileSync(initializedMarker, 'INITIALIZED\n') + if (pauseStdinAfterInitialized) process.stdin.pause() + if (closeStdinAfterInitialized) closeSync(0) + return + } + if (method === 'textDocument/didClose') return if (method?.startsWith('textDocument/')) { if (hang) return const reply = (): void => { @@ -190,4 +202,6 @@ function send(message: Record): void { // Keep the event loop alive. process.stdin.resume() -if (closeStdinAfterReply) setInterval(() => {}, 1000) +if (pauseStdinAfterInitialized || closeStdinAfterInitialized || closeStdinAfterReply) { + setInterval(() => {}, 1000) +} diff --git a/packages/lsp/lsp-local/tests/instance.spec.ts b/packages/lsp/lsp-local/tests/instance.spec.ts index 6019d870fa..328c8b7313 100644 --- a/packages/lsp/lsp-local/tests/instance.spec.ts +++ b/packages/lsp/lsp-local/tests/instance.spec.ts @@ -178,6 +178,40 @@ describe('LspInstance query and abort', () => { await instance.dispose() }) + it('terminates when abort interrupts a backpressured didOpen write', async () => { + // The fixture consumes initialized, then stops reading. A document larger than the stdio pipe + // keeps didOpen's write callback pending until cancellation forces bounded process teardown. + await writeFile(join(ws, 'a.ts'), 'x'.repeat(2_000_000)) + const marker = join(root, 'initialized.log') + const instance = makeInstance({ + LSP_FAKE_INITIALIZED_MARKER: marker, + LSP_FAKE_PAUSE_STDIN_AFTER_INITIALIZED: '1', + }, { + shutdownTimeoutMs: 100, + killGraceMs: 100, + }) + const controller = new AbortController() + const pending = run(instance, 'goToDefinition', controller.signal) + await waitForFile(marker) + // Let the client enter the large didOpen write after the fixture has paused stdin. + await new Promise(resolve => setTimeout(resolve, 100)) + controller.abort(new Error('didOpen-abort')) + await expect(pending).rejects.toThrow(/didOpen-abort/) + expect(instance.dead).toBe(true) + }) + + it('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' }, { + shutdownTimeoutMs: 100, + killGraceMs: 100, + }) + await expect(run(instance, 'goToDefinition')).rejects.toThrow() + expect(instance.dead).toBe(true) + }) + 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/) @@ -287,3 +321,18 @@ function processAlive(pid: number): boolean { throw error } } + +/** 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() + for (;;) { + try { + await readFile(path) + return + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error + } + if (Date.now() - started > timeoutMs) throw new Error('waitForFile timed out') + await new Promise(resolve => setTimeout(resolve, 10)) + } +}