From 2fd995bf3c838197b838b592cdc1133d3396f8ae Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Tue, 21 Jul 2026 13:29:40 +0800 Subject: [PATCH] fix(lsp): align operations and harden lifecycle --- .../2026-07-15-lsp-capability-seam.i18n.yaml | 4 +- .../2026-07-15-lsp-capability-seam.md | 26 ++--- .../2026-07-15-lsp-capability-seam.zh.md | 28 +++--- AGENTS.md | 1 + docs/config-catalog.md | 10 +- docs/core-data-structures/lsp.md | 17 ++-- docs/module-graph.md | 3 +- docs/tool-catalog.md | 10 +- .../acp-agent/tests/lsp.cordis.snapshot.yml | 2 + examples/acp-agent/tests/lsp.cordis.yml | 2 + .../snapshots/lsp-definition/session.jsonl | 8 +- .../lsp-definition/stdout.expected.jsonl | 2 +- .../lsp-definition/system-prompt.expected.md | 2 +- .../lsp-definition/tool-schemas.expected.json | 10 +- knip.json | 2 +- packages/lsp/README.md | 2 +- packages/lsp/lsp-local/README.md | 10 +- packages/lsp/lsp-local/package.json | 2 +- packages/lsp/lsp-local/src/abort.ts | 48 +++++++++ packages/lsp/lsp-local/src/connection.ts | 66 ++++++++----- packages/lsp/lsp-local/src/host.ts | 28 +++++- packages/lsp/lsp-local/src/index.ts | 90 +++++++++++------ packages/lsp/lsp-local/src/instance.ts | 82 ++++++--------- packages/lsp/lsp-local/src/translate.ts | 67 +++++++++---- packages/lsp/lsp-local/tests/built-lib.e2e.ts | 8 +- .../lsp/lsp-local/tests/connection.spec.ts | 21 ++-- .../lsp/lsp-local/tests/fixture-server.ts | 33 +++++-- packages/lsp/lsp-local/tests/host.spec.ts | 18 ++++ packages/lsp/lsp-local/tests/instance.spec.ts | 63 +++++++----- .../lsp/lsp-local/tests/lifecycle.spec.ts | 99 +++++++++++++------ packages/lsp/lsp-local/tests/provider.spec.ts | 27 ++++- .../lsp/lsp-local/tests/translate.spec.ts | 44 ++++++--- .../lsp-local/tests/typescript-server.e2e.ts | 6 +- packages/lsp/lsp/README.md | 4 +- packages/lsp/lsp/src/index.ts | 8 +- packages/lsp/lsp/src/types.ts | 13 +-- packages/lsp/lsp/tests/lsp.spec.ts | 2 +- packages/lsp/tool-lsp/README.md | 10 +- packages/lsp/tool-lsp/package.json | 4 +- packages/lsp/tool-lsp/src/index.ts | 48 +++++---- packages/lsp/tool-lsp/src/render.ts | 40 +++++--- .../lsp/tool-lsp/tests/integration.spec.ts | 4 +- packages/lsp/tool-lsp/tests/render.spec.ts | 37 ++++--- packages/lsp/tool-lsp/tests/tool-lsp.spec.ts | 29 ++++-- packages/lsp/tool-lsp/tsconfig.json | 3 + pnpm-lock.yaml | 3 + 46 files changed, 680 insertions(+), 366 deletions(-) create mode 100644 packages/lsp/lsp-local/src/abort.ts 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 097eb8134e..82433d5fff 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: 6a71858bb6c8ca0ad422042a22ee9085387db46d -2026-07-15-lsp-capability-seam.zh.md: 19a00a762d4f096f02386cf4e4c09e62daee5d6d +2026-07-15-lsp-capability-seam.md: 91b15e8f9ff044d2c438c87040f9fc19a8dabc6a +2026-07-15-lsp-capability-seam.zh.md: 39e63370241e8bbeb93ea7bb81fbd951fe807b19 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 6a71858bb6..91b15e8f9f 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 @@ -22,7 +22,7 @@ Add LSP as a three-package capability seam with one read-only model tool and one `dsh-lsp-local` is a generic host, not a language-server catalog or installer. Deployments explicitly configure commands and mappings; future presets belong in composition plugins or `cordis.yml` overlays. -The model and seam expose exactly `definition`, `references`, `implementation`, and `hover`; no arbitrary JSON-RPC method escapes through `ctx.lsp`. +The model and seam expose exactly `goToDefinition`, `findReferences`, `goToImplementation`, and `hover`; no arbitrary JSON-RPC method escapes through `ctx.lsp`. These operation literals match Claude Code's familiar camelCase names while the tool name and `file_path` field remain harness-owned. The prompt positions LSP as a precision aid: `Use search/read for ordinary navigation. Use lsp when textual matches are ambiguous or before a change requires precise definitions, implementations, or references.` @@ -30,14 +30,14 @@ The prompt positions LSP as a precision aid: `Use search/read for ordinary navig `dsh-lsp` registers providers by branded id and extension-to-language-id mapping. `registerProvider()` atomically reserves the id and every normalized extension: invalid input or any conflict publishes nothing, and its disposer releases all reservations. Provider plugins register through `ctx.effect()`. Selection is per query and order-independent; no match returns a structured unavailable error. The first version has no glob, language-id, or explicit route selector and no statically declared operation capabilities. -The seam exposes one `query(request, signal?)` operation because no fields need implementation defaulting: `workspaceRoot` is required, `languageId` comes from the registration, and consumers own timeouts and result limits. `query()` validates, selects, and derives without hidden `??` fallbacks, leaving no executable spec to resolve. `dsh-tool-lsp` passes only `exec.signal` as a bare `AbortSignal`, matching web and keeping `dsh-lsp` independent of `dsh-tools`. Removal before selection fails as unavailable; later disposal follows the selected provider's cancellation lifecycle without rerouting. +The seam exposes one `query(request, signal?)` operation because no fields need implementation defaulting: `workspaceRoot` is required, `languageId` comes from the registration, and consumers own timeouts and result limits. `query()` selects and derives without hidden `??` fallbacks, leaving no executable spec to resolve. `dsh-tool-lsp` validates model arguments and passes only `exec.signal` as a bare `AbortSignal`, matching web and keeping `dsh-lsp` independent of `dsh-tools`. Removal before selection fails as unavailable; later disposal follows the selected provider's cancellation lifecycle without rerouting. The intended contract shape is: ```ts import type { Branded } from '@deepseek-ai/dsh-brand' -type LspOperation = 'definition' | 'references' | 'implementation' | 'hover' +type LspOperation = 'goToDefinition' | 'findReferences' | 'goToImplementation' | 'hover' type LspProviderId = Branded<'LspProviderId'> interface LspPosition { @@ -77,7 +77,7 @@ interface LspService { } ``` -Mapping keys normalize to lowercase, leading-dot extensions selected from `filePath`'s final extension; language ids only synchronize documents. Seam positions and ranges are zero-based UTF-16. `references` always includes declarations: providers enforce this internally, the local mapping sets `context.includeDeclaration: true`, and callers get no flag. Closed result unions normalize navigation to locations and hover to content or `null`; navigation results carry the provider's resolved workspace root so consumers relativize file URIs in the same canonical namespace. The seam exposes no protocol types, process or document controls, or generic request escape hatch. +Mapping keys normalize to lowercase, leading-dot extensions selected from `filePath`'s final extension; language ids only synchronize documents. Seam positions and ranges are zero-based UTF-16. `findReferences` always includes declarations: providers enforce this internally, the local mapping sets `context.includeDeclaration: true`, and callers get no flag. Closed result unions normalize navigation to locations and hover to content or `null`; navigation results carry the provider's resolved workspace root so consumers relativize file URIs in the same canonical namespace. The seam exposes no protocol types, process or document controls, or generic request escape hatch. `dsh-lsp-local` owns host files, server configuration, JSON-RPC, process and transient-document state, and protocol translation; it depends on `dsh-lsp` and Node APIs, not `dsh-fs`. The server-table key is its provider id. The plugin resolves every server-local setting before registration, rolls back earlier registrations if a later mapping is invalid or conflicts, and retains an independent process pool per provider. `dsh-tool-lsp` runtime-injects only `tools`, `lsp`, and `systemPrompt`, obtains the workspace from `exec.agent?.session.header.cwd` through a package-local `sessionCwd(exec)` helper matching the filesystem tools' lookup, and imports no provider. @@ -87,18 +87,18 @@ The single `lsp` tool accepts: ```ts interface LspToolInput { - readonly operation: 'definition' | 'references' | 'implementation' | 'hover' + readonly operation: 'goToDefinition' | 'findReferences' | 'goToImplementation' | 'hover' readonly file_path: string readonly line: number readonly character: number } ``` -`line` and `character` are positive, one-based UTF-16 cursor coordinates; the tool converts them to the seam's zero-based `LspPosition` and converts rendered locations back. `references` includes declarations so impact analysis does not omit the defining site. Provider, language id, workspace root, limits, timeout, initialization, and executable remain outside model input. +`line` and `character` are positive, one-based UTF-16 cursor coordinates; the tool converts them to the seam's zero-based `LspPosition` and converts rendered locations back. `findReferences` includes declarations so impact analysis does not omit the defining site. Provider, language id, workspace root, limits, timeout, initialization, and executable remain outside model input. The tool requires `workspaceRoot` from session `header.cwd`, with no fallback; absence fails as `LSP_WORKSPACE_REQUIRED` before querying or startup. The local provider resolves relative paths against that root and accepts absolute paths directly; both forms are canonicalized and rejected before startup when the target is outside the canonical workspace. -Locations render as stable, file-grouped `path:line:character` entries. A `file:` URI accepted by Node `fileURLToPath()` becomes a relative path inside the workspace or an absolute path outside it; other URIs remain verbatim. `maxLocations` defaults to `100`, and `maxHoverChars` defaults to `16_000` after hover normalization; both report omissions. Empty locations and `null` hover are successful no-result responses; malformed payloads remain structured errors. +Locations render as stable, file-grouped `path:line:character` entries. A `file:` URI accepted by Node `fileURLToPath()` becomes a relative path inside the workspace or an absolute path outside it; other URIs remain verbatim. `maxLocations` defaults to `100` and reports omitted items; `maxResultChars` defaults to `16_000` and bounds every complete rendered result, including its truncation metadata. Empty locations and `null` hover are successful no-result responses; missing or malformed server payloads fail with structured `LSP_MALFORMED_RESPONSE` errors. ACP uses `{ card: 'generic', kind: 'search', title, locations: [{ path: file_path, line }] }` with an args-derived operation/cursor `title`. Because `FileLocation` has no character, follow-along focuses the input line while the title preserves the cursor; presentation remains pure. @@ -108,11 +108,11 @@ ACP uses `{ card: 'generic', kind: 'search', title, locations: [{ path: file_pat The seam and provider add no startup or request deadline. Non-tool callers therefore receive no hidden timeout and must supply an `AbortSignal`, using `deadline()` when they need a budget. -Provider disposal occurs outside tool execution, so `dsh-lsp-local` keeps `shutdownTimeoutMs` (default `5_000`) for `shutdown`/`exit` and `killGraceMs` (default `2_000`) before hard kill; the same bounds govern failed-instance cleanup. It uses `deadline()` and `timeoutOf()` but owns request cancellation, process signals, and awaiting close because timeout notification does not terminate work. +Provider disposal occurs outside tool execution, so `dsh-lsp-local` keeps `shutdownTimeoutMs` (default `5_000`) for `shutdown`/`exit` and `killGraceMs` (default `2_000`) for both request-cancel grace and SIGTERM-to-SIGKILL escalation; the same bounds govern failed-instance cleanup. Timer values above Node's `2_147_483_647` ms scheduling range fail at load. The provider uses `deadline()` and `timeoutOf()` but owns request cancellation, process signals, and awaiting close because timeout notification does not terminate work. ## Workspace, filesystem, and document synchronization -`dsh-lsp-local` canonicalizes and reads through Node APIs in the subprocess's host namespace. It rejects missing, non-regular, non-UTF-8, oversized, or canonical out-of-workspace sources and keeps one handle through validation and reading. It does not consume `ctx.fs` or emit `fs/observed`: only the LSP result is model-visible, so the query does not satisfy read-before-write policy. +`dsh-lsp-local` canonicalizes and reads through Node APIs in the subprocess's host namespace. It rejects missing, non-regular, non-UTF-8, oversized, or canonical out-of-workspace sources and keeps one `O_NOFOLLOW | O_NONBLOCK` handle through validation and reading, so a FIFO with no writer cannot block before the regular-file check. It observes caller cancellation around each filesystem operation. It does not consume `ctx.fs` or emit `fs/observed`: only the LSP result is model-visible, so the query does not satisfy read-before-write policy. The `read` tool is unsuitable source because its output is windowed, numbered, transcript-visible, and observed. Reading in `tool-lsp` would also assign provider-specific synchronization to the consumer and preclude non-local providers. @@ -123,7 +123,7 @@ The local provider uses a compatibility-first transient-open sequence for every 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. -Documents close after each call, so the first version needs no `didChange`, `didSave`, content cache, mutation listener, or document LRU. One abortable per-instance queue serializes complete lifecycles; distinct instances may run in parallel. The server's workspace index remains responsible for closed files reached from the source. +Documents close after each call, so the first version needs no `didChange`, `didSave`, content cache, mutation listener, or document LRU. One abortable per-workspace provider queue serializes source-read/open/query/close lifecycles, so a waiting query reads current bytes only when its turn starts; the instance also keeps protocol lifecycles serialized. Distinct workspaces may run in parallel. The server's workspace index remains responsible for closed files reached from the source. The canonical workspace `realpath` must be a directory and supplies process cwd, `rootUri`, the sole `workspaceFolders` entry, and pool identity; symlink aliases therefore share an instance. Result locations may be external, but an external path cannot become a query source. Remote, virtual, or independently sandboxed filesystems require another provider. @@ -133,7 +133,7 @@ The canonical workspace `realpath` must be a directory and supplies process cwd, Initialization advertises `general.positionEncodings: ['utf-16']`, `workspace: { workspaceFolders: true, configuration: true }`, `textDocument.hover.contentFormat: ['markdown', 'plaintext']`, and `linkSupport: true` for definition and implementation, with no dynamic registration. Returned operation and synchronization capabilities are authoritative. An omitted server `positionEncoding` defaults to `utf-16`; any other value is a protocol error. Configuration may supply initialization options and `workspace/configuration` responses, but the client rejects `workspace/applyEdit` and never executes commands or edits. -Navigation maps `Location` directly and `LocationLink` from `targetUri` plus `targetSelectionRange`. Hover normalization takes `MarkupContent.value`, preserves string `MarkedString` values, renders language-tagged values as fenced code, joins arrays with one blank line, and applies `maxHoverChars` last. +Navigation maps `Location` directly and `LocationLink` from `targetUri` plus `targetSelectionRange`. Positions must be nonnegative integers. Hover normalization accepts only valid `MarkupContent` and `MarkedString` shapes, preserves string values, renders language-tagged values as fenced code, and joins arrays with one blank line. The model-facing tool applies `maxResultChars` after rendering. Abort reaches every query phase and sends `$/cancelRequest` once an id exists. An unresponsive server is terminated and awaited without collateral active work because the instance is serialized. Disposal rejects and cancels work, attempts graceful shutdown, escalates through bounded termination, and awaits quiescence. @@ -176,10 +176,10 @@ The local provider trusts its configured server and claims no sandbox confinemen - Package tests pin the three-package dependency direction, runtime injections, and `ctx.lsp`-only communication. - 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 `references.includeDeclaration`. +- 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. - Timeout tests pin one `TOOL_TIMEOUT` budget, unclassified upstream cancellation, no hidden seam deadline, and bounded awaited teardown. -- Lifecycle tests pin startup single-flight, per-instance serialization, cross-instance parallelism, abortable queues, crash replacement without replay, and quiescent disposal. +- 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. - A keyless pinned TypeScript real-server e2e exercises all four operations; runnable configuration uses the same explicit provider mapping. - Snapshots cover model-visible schema, prompt, results, omissions, and ACP rendering; a built-artifact smoke test covers framing and cleanup. 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 19a00a762d..39e6337024 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 @@ -22,22 +22,22 @@ harness 已具备文本搜索与文件读取能力，但二者都无法识别程 `dsh-lsp-local` 是通用 host，不是语言服务器目录或安装器。部署显式配置命令与映射；未来 preset 属于组合插件或 `cordis.yml` overlay。 -模型与服务边界仅公开 `definition`、`references`、`implementation` 和 `hover`；`ctx.lsp` 不提供任意 JSON-RPC 方法。 +模型与服务边界仅公开 `goToDefinition`、`findReferences`、`goToImplementation` 和 `hover`；`ctx.lsp` 不提供任意 JSON-RPC 方法。这些操作字面量与 Claude Code 熟悉的 camelCase 命名一致，而工具名与 `file_path` 字段仍由 harness 自行定义。 提示词将 LSP 定位为精确查询手段：`Use search/read for ordinary navigation. Use lsp when textual matches are ambiguous or before a change requires precise definitions, implementations, or references.` -## Package 与职责边界 +## 包与职责边界 `dsh-lsp` 按带品牌类型的 id 和扩展名到语言 id 的映射注册提供方。`registerProvider()` 以原子方式占用 id 与所有规范化扩展名：输入无效或存在冲突时不发布任何状态，清理函数释放全部占用。提供方插件通过 `ctx.effect()` 注册。系统按查询且不受顺序影响地选择提供方；没有匹配项时返回结构化不可用错误。第一版不提供 glob、language-id 或显式路由选择器，也不静态声明操作能力。 -服务边界只公开 `query(request, signal?)`，因为没有字段需要实现层填充默认值：`workspaceRoot` 是必填项，`languageId` 来自注册映射，超时与结果限制由消费方负责。`query()` 执行校验、选择与推导时不使用隐藏的 `??` 后备逻辑，因此没有需要 resolve 的可执行 spec。`dsh-tool-lsp` 只把 `exec.signal` 作为裸 `AbortSignal` 传递，与 web 一致，并使 `dsh-lsp` 不依赖 `dsh-tools`。提供方在选择前被移除时按不可用失败；之后的释放遵循已选提供方的取消生命周期，不改路由。 +服务边界只公开 `query(request, signal?)`，因为没有字段需要实现层填充默认值：`workspaceRoot` 是必填项，`languageId` 来自注册映射，超时与结果限制由消费方负责。`query()` 执行选择与推导时不使用隐藏的 `??` 后备逻辑，因此没有需要 resolve 的可执行 spec。`dsh-tool-lsp` 校验模型参数，并只把 `exec.signal` 作为裸 `AbortSignal` 传递，与 web 一致，并使 `dsh-lsp` 不依赖 `dsh-tools`。提供方在选择前被移除时按不可用失败；之后的释放遵循已选提供方的取消生命周期，不改路由。 预期契约如下： ```ts import type { Branded } from '@deepseek-ai/dsh-brand' -type LspOperation = 'definition' | 'references' | 'implementation' | 'hover' +type LspOperation = 'goToDefinition' | 'findReferences' | 'goToImplementation' | 'hover' type LspProviderId = Branded<'LspProviderId'> interface LspPosition { @@ -77,7 +77,7 @@ interface LspService { } ``` -映射键规范化为带前导点的小写扩展名，并按 `filePath` 的最后一个扩展名选择；语言 id 仅用于文档同步。服务边界中的位置和范围从零开始按 UTF-16 计数。`references` 始终包含声明：提供方在内部执行该约束，本地映射设置 `context.includeDeclaration: true`，调用方不能配置。封闭结果联合将导航统一为位置，将 `hover` 统一为内容或 `null`；导航结果携带提供方解析后的工作区根目录，使消费方依据同一规范化根目录相对化文件 URI。服务边界不公开协议类型、进程或文档控制，也不提供通用请求逃生口。 +映射键规范化为带前导点的小写扩展名，并按 `filePath` 的最后一个扩展名选择；语言 id 仅用于文档同步。服务边界中的位置和范围从零开始按 UTF-16 计数。`findReferences` 始终包含声明：提供方在内部执行该约束，本地映射设置 `context.includeDeclaration: true`，调用方不能配置。封闭结果联合将导航统一为位置，将 `hover` 统一为内容或 `null`；导航结果携带提供方解析后的工作区根目录，使消费方依据同一规范化根目录相对化文件 URI。服务边界不公开协议类型、进程或文档控制，也不提供通用请求逃生口。 `dsh-lsp-local` 负责主机文件、服务器配置、JSON-RPC、进程与临时文档状态和协议转换；它依赖 `dsh-lsp` 与 Node API，不依赖 `dsh-fs`。服务器表的键是提供方 id。插件在注册前解析每个服务器的本地设置；如果后续映射无效或发生冲突，插件会撤销此前的注册，并为每个提供方保留独立进程池。`dsh-tool-lsp` 在运行时只注入 `tools`、`lsp` 和 `systemPrompt`，通过包内的 `sessionCwd(exec)` 辅助函数从 `exec.agent?.session.header.cwd` 取得工作区，其取值方式与文件系统工具一致，也不导入提供方。 @@ -87,18 +87,18 @@ interface LspService { ```ts interface LspToolInput { - readonly operation: 'definition' | 'references' | 'implementation' | 'hover' + readonly operation: 'goToDefinition' | 'findReferences' | 'goToImplementation' | 'hover' readonly file_path: string readonly line: number readonly character: number } ``` -`line` 和 `character` 是从一开始计数的正数 UTF-16 光标坐标；工具将其转换为服务边界中从零开始的 `LspPosition`，并将渲染位置转回。`references` 包含声明，避免影响分析漏掉定义位置。提供方、语言 id、工作区根目录、限制、超时、初始化和可执行文件均不进入模型输入。 +`line` 和 `character` 是从一开始计数的正数 UTF-16 光标坐标；工具将其转换为服务边界中从零开始的 `LspPosition`，并将渲染位置转回。`findReferences` 包含声明，避免影响分析漏掉定义位置。提供方、语言 id、工作区根目录、限制、超时、初始化和可执行文件均不进入模型输入。 工具必须从会话 `header.cwd` 取得 `workspaceRoot`，没有后备值；缺失时在查询或启动前以 `LSP_WORKSPACE_REQUIRED` 失败。本地提供方基于根目录解析相对路径并直接接受绝对路径；两种路径都会进行规范化，如果目标位于规范工作区外，则在启动前拒绝。 -位置按文件稳定分组并渲染为 `path:line:character`。Node `fileURLToPath()` 可接受的 `file:` URI 在工作区内转换为相对路径，在工作区外转换为绝对路径；其他 URI 保持原样。`maxLocations` 默认值为 `100`，`maxHoverChars` 在 `hover` 归一化后应用，默认值为 `16_000`；两者都会报告省略数量。空位置与 `null` hover 是成功的无结果响应；格式错误的载荷保持为结构化错误。 +位置按文件稳定分组并渲染为 `path:line:character`。Node `fileURLToPath()` 可接受的 `file:` URI 在工作区内转换为相对路径，在工作区外转换为绝对路径；其他 URI 保持原样。`maxLocations` 默认值为 `100`，并报告省略的条目；`maxResultChars` 默认值为 `16_000`，并限制每个完整渲染结果，其中包括截断元数据。空位置与 `null` hover 是成功的无结果响应；服务器载荷缺失或格式错误时，以结构化 `LSP_MALFORMED_RESPONSE` 错误失败。 ACP 使用 `{ card: 'generic', kind: 'search', title, locations: [{ path: file_path, line }] }`，`title` 由参数推导并标明操作与光标。由于 `FileLocation` 没有 character，跟随位置聚焦输入行，标题保留完整光标；展示保持纯函数。 @@ -108,11 +108,11 @@ ACP 使用 `{ card: 'generic', kind: 'search', title, locations: [{ path: file_p 服务边界和提供方不增加启动或请求截止时间。非工具调用方不会获得隐藏超时，必须自行提供 `AbortSignal`，并在需要预算时使用 `deadline()`。 -提供方释放发生在工具执行之外，因此 `dsh-lsp-local` 保留 `shutdownTimeoutMs`（默认 `5_000`）限制 `shutdown`/`exit`，以及 `killGraceMs`（默认 `2_000`）限制强制终止前的宽限期；失败实例的清理也使用相同边界。它使用 `deadline()` 和 `timeoutOf()`，但仍负责请求取消、进程信号和等待关闭，因为超时通知不会终止工作。 +提供方释放发生在工具执行之外，因此 `dsh-lsp-local` 保留 `shutdownTimeoutMs`（默认 `5_000`）限制 `shutdown`/`exit`，以及 `killGraceMs`（默认 `2_000`），同时用于限制请求取消宽限期和从 SIGTERM 升级到 SIGKILL 的宽限期；失败实例的清理也使用相同边界。定时器值超过 Node `2_147_483_647` ms 的调度范围时，插件加载失败。提供方使用 `deadline()` 和 `timeoutOf()`，但仍负责请求取消、进程信号和等待关闭，因为超时通知不会终止工作。 ## 工作区、文件系统与文档同步 -`dsh-lsp-local` 通过 Node API 在子进程所在的主机命名空间中规范化并读取文件。它拒绝缺失、非普通、非 UTF-8、超大或规范路径越出工作区的源文件，并在校验与读取期间保持同一句柄。它不使用 `ctx.fs` 或发送 `fs/observed`：只有 LSP 结果对模型可见，因此查询不满足写前读取策略。 +`dsh-lsp-local` 通过 Node API 在子进程所在的主机命名空间中规范化并读取文件。它拒绝缺失、非普通、非 UTF-8、超大或规范路径越出工作区的源文件，并在校验与读取期间保持同一个 `O_NOFOLLOW | O_NONBLOCK` 句柄，因此没有写入方的 FIFO 不会在普通文件校验前造成阻塞。它在每项文件系统操作前后检查调用方是否取消。它不使用 `ctx.fs` 或发送 `fs/observed`：只有 LSP 结果对模型可见，因此查询不满足写前读取策略。 `read` 工具的输出带窗口与行号，进入 transcript（文本记录）且已被观察，不适合作为源文件。在 `tool-lsp` 内读取还会把提供方专用同步职责交给消费方，并排除非本地提供方。 @@ -123,7 +123,7 @@ ACP 使用 `{ card: 'generic', kind: 'search', title, locations: [{ path: file_p 3. 发送所请求的 `textDocument/definition`、`textDocument/references`、`textDocument/implementation` 或 `textDocument/hover` 请求。 4. 如果 `didOpen` 成功，则在请求完成或取消后于 `finally` 中尝试发送 `textDocument/didClose`。关闭写入失败不会覆盖已经确定的结果或错误，但会使实例失效，并等待有界进程终止完成。 -每次调用后都关闭文档，因此第一版不需要 `didChange`、`didSave`、内容缓存、变更监听器或文档 LRU。每个实例使用一个可取消队列串行执行完整生命周期；不同实例可以并行。服务器工作区索引仍负责从源文件跳转到的已关闭文件。 +每次调用后都关闭文档，因此第一版不需要 `didChange`、`didSave`、内容缓存、变更监听器或文档 LRU。每个工作区的提供方队列可取消，并串行执行源文件读取、打开、查询和关闭的完整生命周期，因此等待中的查询只在轮到它时才读取当前字节；实例也会串行执行协议生命周期。不同工作区可以并行。服务器工作区索引仍负责从源文件跳转到的已关闭文件。 规范工作区 `realpath` 必须是目录，并用于进程 cwd、`rootUri`、唯一的 `workspaceFolders` 条目和进程池 identity；符号链接别名因此共享实例。结果位置可以在工作区外，但外部路径不能成为查询源。远程、虚拟或独立沙箱化文件系统需要另一种提供方。 @@ -133,7 +133,7 @@ ACP 使用 `{ card: 'generic', kind: 'search', title, locations: [{ path: file_p 初始化声明 `general.positionEncodings: ['utf-16']`、`workspace: { workspaceFolders: true, configuration: true }`、`textDocument.hover.contentFormat: ['markdown', 'plaintext']`，以及 definition 与 implementation 的 `linkSupport: true`，但不支持动态注册。服务器返回的操作与同步能力均为真源。服务器省略 `positionEncoding` 时默认为 `utf-16`；其他值均属于协议错误。配置可以提供初始化选项和 `workspace/configuration` 响应，但客户端拒绝 `workspace/applyEdit`，绝不执行命令或编辑。 -导航结果直接映射 `Location`，并将 `LocationLink` 的 `targetUri` 与 `targetSelectionRange` 映射为统一位置。`hover` 归一化直接采用 `MarkupContent.value`，保留字符串 `MarkedString`，把带语言标签的值渲染为围栏代码块，以一个空行连接数组，并在最后应用 `maxHoverChars`。 +导航结果直接映射 `Location`，并将 `LocationLink` 的 `targetUri` 与 `targetSelectionRange` 映射为统一位置。位置必须是非负整数。`hover` 归一化只接受有效的 `MarkupContent` 和 `MarkedString` 结构，保留字符串值，把带语言标签的值渲染为围栏代码块，并以一个空行连接数组。面向模型的工具在渲染后应用 `maxResultChars`。 取消信号传递到查询的所有阶段，请求 id 创建后还会发送 `$/cancelRequest`。无响应的服务器会被终止并等待关闭；实例串行化保证没有其他正在执行的工作被连带中断。资源释放会拒绝并取消工作、尝试优雅关闭、通过有界终止流程升级处理，并等待完全停稳。 @@ -176,10 +176,10 @@ ACP 使用 `{ card: 'generic', kind: 'search', title, locations: [{ path: file_p - 包测试固定三个包的依赖方向、运行时注入和仅通过 `ctx.lsp` 通信的边界。 - 工具测试固定四种操作、坐标校验、配置限制与省略标记、提示词和 ACP 展示。 - 注册表测试固定原子占用/释放、不受顺序影响的选择，以及结构化的不可用、已释放、冲突和不支持操作错误。 -- 测试用 stdio server 固定精确的初始化能力、四种协议映射、`Location`/`LocationLink` 与 `hover` 归一化，以及 `references.includeDeclaration`。 +- 测试用 stdio server 固定精确的初始化能力、四种协议映射、`Location`/`LocationLink` 与 `hover` 归一化，以及 `findReferences` 到 `references.includeDeclaration` 的映射。 - 同步测试固定 UTF-16 协商与转换、受支持和被拒绝的 `textDocumentSync` 形式、配对的临时打开/关闭、关闭写入失败和错误响应拒绝。 - 超时测试固定一个 `TOOL_TIMEOUT` 预算、不对上游取消错误分类、服务边界无隐藏截止时间，以及受限且等待完成的清理。 -- 生命周期测试固定启动 single-flight、实例内串行、跨实例并行、可取消队列、崩溃后不重放的替换，以及释放后完全停稳。 +- 生命周期测试固定启动 single-flight、完整生命周期串行化及排队查询读取最新源文件、跨工作区并行、可取消队列、崩溃后不重放的替换、stdin 失败后的进程拆除，以及释放后完全停稳。 - 主机文件系统测试固定 session cwd 要求、符号链接下相对与绝对源路径的规范 containment、文档校验、file/non-file URI 渲染、无格式源文本和不发送 `fs/observed`。 - 无密钥且固定版本的 TypeScript 真实服务器 e2e 覆盖四种操作；可运行配置使用同一项显式提供方映射。 - 快照覆盖模型可见 schema、提示词、结果、省略提示和 ACP 渲染；构建产物冒烟测试覆盖分帧与清理。 diff --git a/AGENTS.md b/AGENTS.md index 6a0e4ca13b..5835149048 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,6 +16,7 @@ packages/ @deepseek-ai/dsh- workspaces at packages/// llm/ LLM seam + the DeepSeek adapters (hand-rolled + pi-ai design twin) bash/ bash executor seam + local impl + model-facing bash tools fs/ filesystem seam + local impl + policy gate + read/write/edit tools + lsp/ language-server seam + local stdio provider + model-facing lsp tool skill/ skill provider registry + local impl + catalog/loader tool web/ web seam + search/fetch providers + model-facing web tools compact/ compaction seam + basic backend diff --git a/docs/config-catalog.md b/docs/config-catalog.md index 8c7abf7d14..1d7930a84e 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -650,12 +650,12 @@ export interface LspLocalServerConfig { maxDocumentBytes?: number /** Graceful `shutdown`/`exit` budget before escalation (ms). Default 5000. */ shutdownTimeoutMs?: number - /** SIGTERM→SIGKILL grace after graceful shutdown fails (ms). Default 2000. */ + /** Request-cancel and SIGTERM→SIGKILL grace (ms). Default 2000. */ killGraceMs?: number } ``` -Source: [`packages/lsp/lsp-local/src/index.ts:83`](../packages/lsp/lsp-local/src/index.ts) +Source: [`packages/lsp/lsp-local/src/index.ts:85`](../packages/lsp/lsp-local/src/index.ts) ## `@deepseek-ai/dsh-mcp-client` @@ -1188,14 +1188,14 @@ Requires: `tools` · `lsp` · `systemPrompt` export interface Config { /** Largest number of rendered locations before an omission marker (default 100). */ maxLocations?: number - /** Largest hover length in characters after normalization (default 16000). */ - maxHoverChars?: number + /** Largest complete rendered result in characters, including truncation metadata (default 16000). */ + maxResultChars?: number /** Tool-call timeout budget in ms (default 60000). */ timeoutMs?: number } ``` -Source: [`packages/lsp/tool-lsp/src/index.ts:56`](../packages/lsp/tool-lsp/src/index.ts) +Source: [`packages/lsp/tool-lsp/src/index.ts:58`](../packages/lsp/tool-lsp/src/index.ts) ## `@deepseek-ai/dsh-tool-ralph` diff --git a/docs/core-data-structures/lsp.md b/docs/core-data-structures/lsp.md index 6bf63c20c2..eb370f6e38 100644 --- a/docs/core-data-structures/lsp.md +++ b/docs/core-data-structures/lsp.md @@ -14,7 +14,7 @@ The seam and model expose exactly four semantic queries; the union is closed, so * compile-enforced change across the seam, providers, and the tool. Symbols and call hierarchy are * deliberately deferred (they need different schemas). */ -type LspOperation = 'definition' | 'references' | 'implementation' | 'hover' +type LspOperation = 'goToDefinition' | 'findReferences' | 'goToImplementation' | 'hover' ``` ```ts type-equiv @@ -71,7 +71,7 @@ interface LspProviderQuery extends LspQueryRequest { ## Result -A CLOSED discriminated union: navigation operations normalize to `locations`, `hover` to content or `null`. Consumers `switch` on `kind` to exhaustiveness so a new arm breaks compilation until handled. `references` always includes declarations — the provider enforces this internally, so callers get no flag. The `locations` variant carries `resolvedWorkspaceRoot`: the provider's canonical form of the request's `workspaceRoot` and the root its `file:` URIs are relative to, so a caller relativizing display paths uses it rather than the possibly-symlinked request root. +A CLOSED discriminated union: navigation operations normalize to `locations`, `hover` to content or `null`. Consumers `switch` on `kind` to exhaustiveness so a new arm breaks compilation until handled. `findReferences` always includes declarations — the provider enforces this internally, so callers get no flag. The `locations` variant carries `resolvedWorkspaceRoot`: the provider's canonical form of the request's `workspaceRoot` and the root its `file:` URIs are relative to, so a caller relativizing display paths uses it rather than the possibly-symlinked request root. ```ts type-equiv /** One resolved location: a document URI and the range within it. */ @@ -95,9 +95,9 @@ interface LspHover { ```ts type-equiv /** - * The closed result union. Navigation operations (`definition`, `references`, `implementation`) - * normalize to `locations`; `hover` normalizes to content or `null`. Consumers `switch` on `kind` - * to exhaustiveness so a new arm breaks compilation until handled. + * The closed result union. Navigation operations (`goToDefinition`, `findReferences`, + * `goToImplementation`) normalize to `locations`; `hover` normalizes to content or `null`. + * Consumers `switch` on `kind` to exhaustiveness so a new arm breaks compilation until handled. * * The `locations` variant carries `resolvedWorkspaceRoot`: the provider's canonical form of the * request's `workspaceRoot`, and the root its `file:` location URIs are relative to. A caller that @@ -116,8 +116,9 @@ A provider owns a stable branded `id` and an exclusive lowercase leading-dot ext ```ts type-equiv /** * A language-server backend registered on `ctx.lsp`. Each provider owns a stable {@link - * LspProviderId} and an extension-to-language-id map (lowercase, leading-dot keys). `references` - * always includes declarations — the provider enforces this internally; callers get no flag. + * LspProviderId} and an extension-to-language-id map (lowercase, leading-dot keys). + * `findReferences` always includes declarations — the provider enforces this internally; callers + * get no flag. */ interface LspProvider { /** Stable provider identity, reserved atomically with the extension mappings. */ @@ -159,4 +160,4 @@ interface LspService { } ``` -`LspProviderId` is the seam's branded id (`Branded<'LspProviderId'>` from [dsh-brand](../../packages/util/brand)); `LspError` extends `HarnessError` with a stable `code` (`LSP_INVALID_PROVIDER`, `LSP_CONFLICT`, `LSP_UNAVAILABLE`, `LSP_UNSUPPORTED_OPERATION`) callers route on instead of parsing `message`. +`LspProviderId` is the seam's branded id (`Branded<'LspProviderId'>` from [dsh-brand](../../packages/util/brand)); `LspError` extends `HarnessError` with stable codes such as `LSP_INVALID_PROVIDER`, `LSP_CONFLICT`, `LSP_UNAVAILABLE`, `LSP_DISPOSED`, `LSP_UNSUPPORTED_OPERATION`, and `LSP_MALFORMED_RESPONSE`, which callers route on instead of parsing `message`. diff --git a/docs/module-graph.md b/docs/module-graph.md index 0bbfe9c93d..715ac67732 100644 --- a/docs/module-graph.md +++ b/docs/module-graph.md @@ -404,6 +404,7 @@ flowchart TD pkg_tool_lsp --> pkg_llm pkg_tool_lsp --> pkg_lsp pkg_tool_lsp --> pkg_system_prompt + pkg_tool_lsp --> pkg_timeout pkg_tool_lsp --> pkg_tools pkg_mcp_client --> pkg_llm pkg_mcp_client --> pkg_tools @@ -610,7 +611,7 @@ flowchart TD | [`tool-ask-user`](../packages/ui/tool-ask-user) | `ui` | [`agent`](../packages/core/agent), [`tools`](../packages/core/tools), [`user-interaction`](../packages/ui/user-interaction) | | [`workspace-context`](../packages/context/workspace-context) | `context` | [`agent`](../packages/core/agent), [`fs`](../packages/fs/fs), [`llm`](../packages/llm/llm), [`paths`](../packages/util/paths), [`session`](../packages/core/session), [`tools`](../packages/core/tools) | | [`repeat-tool-guard`](../packages/guard/repeat-tool-guard) | `guard` | [`agent`](../packages/core/agent), [`tools`](../packages/core/tools) | -| [`tool-lsp`](../packages/lsp/tool-lsp) | `lsp` | [`llm`](../packages/llm/llm), [`lsp`](../packages/lsp/lsp), [`system-prompt`](../packages/core/system-prompt), [`tools`](../packages/core/tools) | +| [`tool-lsp`](../packages/lsp/tool-lsp) | `lsp` | [`llm`](../packages/llm/llm), [`lsp`](../packages/lsp/lsp), [`system-prompt`](../packages/core/system-prompt), [`timeout`](../packages/util/timeout), [`tools`](../packages/core/tools) | | [`mcp-client`](../packages/mcp/mcp-client) | `mcp` | [`llm`](../packages/llm/llm), [`tools`](../packages/core/tools) | | [`tool-tasks`](../packages/tasks/tool-tasks) | `tasks` | [`agent`](../packages/core/agent), [`system-prompt`](../packages/core/system-prompt), [`tasks`](../packages/tasks/tasks), [`tools`](../packages/core/tools) | | [`tool-workflow`](../packages/workflow/tool-workflow) | `workflow` | [`agent`](../packages/core/agent), [`llm`](../packages/llm/llm), [`system-prompt`](../packages/core/system-prompt), [`tools`](../packages/core/tools), [`workflow`](../packages/workflow/workflow) | diff --git a/docs/tool-catalog.md b/docs/tool-catalog.md index 326f6c02a7..8273203616 100644 --- a/docs/tool-catalog.md +++ b/docs/tool-catalog.md @@ -492,7 +492,7 @@ create, edit, pause, and resume require direct-human root authority; complete an ### `lsp` -Query a language server for precise code navigation. operation is one of definition, references, implementation, hover. line and character are one-based UTF-16 cursor coordinates. references includes the declaration. +Query a language server for precise code navigation. operation is one of goToDefinition, findReferences, goToImplementation, hover. line and character are one-based UTF-16 cursor coordinates. findReferences includes the declaration. ```json { @@ -500,11 +500,11 @@ Query a language server for precise code navigation. operation is one of definit "properties": { "operation": { "type": "string", - "description": "definition, references, implementation, or hover.", + "description": "goToDefinition, findReferences, goToImplementation, or hover.", "enum": [ - "definition", - "references", - "implementation", + "goToDefinition", + "findReferences", + "goToImplementation", "hover" ] }, diff --git a/examples/acp-agent/tests/lsp.cordis.snapshot.yml b/examples/acp-agent/tests/lsp.cordis.snapshot.yml index 4d24ead86d..dc672376b5 100644 --- a/examples/acp-agent/tests/lsp.cordis.snapshot.yml +++ b/examples/acp-agent/tests/lsp.cordis.snapshot.yml @@ -19,6 +19,8 @@ args: ['./lsp-server.mjs'] extensionToLanguage: '.ts': typescript + - id: timeout-policy + name: '@deepseek-ai/dsh-timeout-policy' - id: tool-lsp name: '@deepseek-ai/dsh-tool-lsp' config: diff --git a/examples/acp-agent/tests/lsp.cordis.yml b/examples/acp-agent/tests/lsp.cordis.yml index f1eefa99af..49c9099d65 100644 --- a/examples/acp-agent/tests/lsp.cordis.yml +++ b/examples/acp-agent/tests/lsp.cordis.yml @@ -17,6 +17,8 @@ args: ['./lsp-server.mjs'] extensionToLanguage: '.ts': typescript + - id: timeout-policy + name: '@deepseek-ai/dsh-timeout-policy' - id: tool-lsp name: '@deepseek-ai/dsh-tool-lsp' config: diff --git a/examples/acp-agent/tests/snapshots/lsp-definition/session.jsonl b/examples/acp-agent/tests/snapshots/lsp-definition/session.jsonl index 36fb9ad8f2..00780e7787 100644 --- a/examples/acp-agent/tests/snapshots/lsp-definition/session.jsonl +++ b/examples/acp-agent/tests/snapshots/lsp-definition/session.jsonl @@ -4,12 +4,12 @@ {"type":"step/start","seq":2,"time":0,"data":{"turn":1,"step":1}} {"type":"request/header","seq":3,"time":0,"data":{"header":{"config":{"provider":"deepseek","model":"deepseek-v4-flash"},"system":"{{system}}","tools":"{{tools}}"},"reason":"initial"}} {"type":"assistant/chunk","seq":4,"time":0,"data":{"turn":1,"step":1,"chunk":{"type":"block-start","index":0,"blockType":"tool-call"}}} -{"type":"assistant/chunk","seq":5,"time":0,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":0,"id":"call_lsp_definition","name":"lsp","argumentsDelta":"{\"operation\":\"definition\",\"file_path\":\"subject.ts\",\"line\":1,\"character\":7}"}}} -{"type":"assistant/chunk","seq":6,"time":0,"data":{"turn":1,"step":1,"chunk":{"type":"block-end","index":0,"block":{"type":"tool-call","id":"call_lsp_definition","name":"lsp","arguments":"{\"operation\":\"definition\",\"file_path\":\"subject.ts\",\"line\":1,\"character\":7}"}}}} +{"type":"assistant/chunk","seq":5,"time":0,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":0,"id":"call_lsp_definition","name":"lsp","argumentsDelta":"{\"operation\":\"goToDefinition\",\"file_path\":\"subject.ts\",\"line\":1,\"character\":7}"}}} +{"type":"assistant/chunk","seq":6,"time":0,"data":{"turn":1,"step":1,"chunk":{"type":"block-end","index":0,"block":{"type":"tool-call","id":"call_lsp_definition","name":"lsp","arguments":"{\"operation\":\"goToDefinition\",\"file_path\":\"subject.ts\",\"line\":1,\"character\":7}"}}}} {"type":"assistant/chunk","seq":7,"time":0,"data":{"turn":1,"step":1,"chunk":{"type":"usage","usage":{"inputTokens":10,"outputTokens":5}}}} {"type":"assistant/chunk","seq":8,"time":0,"data":{"turn":1,"step":1,"chunk":{"type":"finish","reason":{"kind":"tool-calls"}}}} -{"type":"assistant/message","seq":9,"time":0,"data":{"turn":1,"step":1,"content":[{"type":"tool-call","id":"call_lsp_definition","name":"lsp","arguments":"{\"operation\":\"definition\",\"file_path\":\"subject.ts\",\"line\":1,\"character\":7}"}],"provenance":{"provider":"deepseek","model":"deepseek-v4-flash"},"usage":{"inputTokens":10,"outputTokens":5}},"sourceEventSeqs":[4,5,6,7,8],"surfaceOp":"append"} -{"type":"tool/call","seq":10,"time":0,"data":{"turn":1,"step":1,"callId":"call_lsp_definition","name":"lsp","arguments":"{\"operation\":\"definition\",\"file_path\":\"subject.ts\",\"line\":1,\"character\":7}"}} +{"type":"assistant/message","seq":9,"time":0,"data":{"turn":1,"step":1,"content":[{"type":"tool-call","id":"call_lsp_definition","name":"lsp","arguments":"{\"operation\":\"goToDefinition\",\"file_path\":\"subject.ts\",\"line\":1,\"character\":7}"}],"provenance":{"provider":"deepseek","model":"deepseek-v4-flash"},"usage":{"inputTokens":10,"outputTokens":5}},"sourceEventSeqs":[4,5,6,7,8],"surfaceOp":"append"} +{"type":"tool/call","seq":10,"time":0,"data":{"turn":1,"step":1,"callId":"call_lsp_definition","name":"lsp","arguments":"{\"operation\":\"goToDefinition\",\"file_path\":\"subject.ts\",\"line\":1,\"character\":7}"}} {"type":"tool/result","seq":11,"time":0,"data":{"turn":1,"step":1,"callId":"call_lsp_definition","content":[{"type":"text","text":"subject.ts:1:7\n… 1 more location omitted (limit 1)."}],"isError":false},"sourceEventSeqs":[10],"surfaceOp":"append"} {"type":"step/end","seq":12,"time":0,"data":{"turn":1,"step":1}} {"type":"step/start","seq":13,"time":0,"data":{"turn":1,"step":2}} diff --git a/examples/acp-agent/tests/snapshots/lsp-definition/stdout.expected.jsonl b/examples/acp-agent/tests/snapshots/lsp-definition/stdout.expected.jsonl index 9e527ea2c5..ce84702157 100644 --- a/examples/acp-agent/tests/snapshots/lsp-definition/stdout.expected.jsonl +++ b/examples/acp-agent/tests/snapshots/lsp-definition/stdout.expected.jsonl @@ -1,7 +1,7 @@ {"jsonrpc":"2.0","id":1,"result":{"protocolVersion":1,"agentInfo":{"name":"deepseek-harness-acp","version":"0.0.1"},"agentCapabilities":{"loadSession":true,"promptCapabilities":{"image":false,"audio":false,"embeddedContext":false}},"authMethods":[]}} {"jsonrpc":"2.0","id":2,"result":{"sessionId":"{{sessionId}}","configOptions":[{"id":"permission","name":"Permissions","description":"The session permission preset: each choice bundles a sandbox mode and an approval policy.","category":"mode","type":"select","currentValue":"danger-full-access","options":[{"value":"workspace-write","name":"workspace-write","description":"Write inside the workspace and permitted temporary directories; wider retries require approval."},{"value":"danger-full-access","name":"danger-full-access","description":"Full file access without approval prompts."}]}]}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"available_commands_update","availableCommands":[{"name":"goal","description":"set or view the goal for a long-running task","input":{"hint":"[|clear|edit |pause|resume]"}}]}}} -{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call","toolCallId":"call_lsp_definition","title":"LSP definition subject.ts:1:7","kind":"search","status":"in_progress","locations":[{"path":"subject.ts","line":1}]}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call","toolCallId":"call_lsp_definition","title":"LSP goToDefinition subject.ts:1:7","kind":"search","status":"in_progress","locations":[{"path":"subject.ts","line":1}]}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call_update","toolCallId":"call_lsp_definition","status":"completed","content":[{"type":"content","content":{"type":"text","text":"subject.ts:1:7\n… 1 more location omitted (limit 1)."}}]}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_message_chunk","content":{"type":"text","text":"DONE"}}}} {"jsonrpc":"2.0","id":3,"result":{"stopReason":"end_turn"}} diff --git a/examples/acp-agent/tests/snapshots/lsp-definition/system-prompt.expected.md b/examples/acp-agent/tests/snapshots/lsp-definition/system-prompt.expected.md index ad82d538b7..8e49c2dce5 100644 --- a/examples/acp-agent/tests/snapshots/lsp-definition/system-prompt.expected.md +++ b/examples/acp-agent/tests/snapshots/lsp-definition/system-prompt.expected.md @@ -15,7 +15,7 @@ Check the [exit code: N] marker on every bash result; investigate failures befor Track every background task id you start. You are notified in-session when a task finishes — do not busy-poll or sleep on one; keep working on independent steps and do not duplicate a running task's work. Before giving a final answer, collect every still-relevant task with task_output (set wait: true only when you are genuinely blocked on it), and task_kill tasks that stopped mattering. -Use search/read for ordinary navigation. Use lsp when textual matches are ambiguous or before a change requires precise definitions, implementations, or references. Positions are one-based line and character (UTF-16) at the cursor; an off-symbol position may return no results. references always includes the declaration. +Use search/read for ordinary navigation. Use lsp when textual matches are ambiguous or before a change requires precise definitions, implementations, or references. Positions are one-based line and character (UTF-16) at the cursor; an off-symbol position may return no results. findReferences always includes the declaration. Use goal tools for one long-running completion objective in the current session. create_goal may infer goal intent from a direct human request in any language; do not create a goal for routine single-turn work. Call get_goal before update_goal and copy its exact goal_id and revision. After session resume or fork, an active goal is disarmed: when a human asks to continue or resume in any wording or language, use update_goal action resume to rearm it. Mark complete only when the objective is actually achieved. Mark blocked only after the same blocking condition persists for at least 3 consecutive goal rounds, and report that concrete condition in blocked_reason; difficulty, uncertainty, or useful remaining work is not blocked. diff --git a/examples/acp-agent/tests/snapshots/lsp-definition/tool-schemas.expected.json b/examples/acp-agent/tests/snapshots/lsp-definition/tool-schemas.expected.json index 1105e030c6..4fa5010b72 100644 --- a/examples/acp-agent/tests/snapshots/lsp-definition/tool-schemas.expected.json +++ b/examples/acp-agent/tests/snapshots/lsp-definition/tool-schemas.expected.json @@ -117,17 +117,17 @@ }, { "name": "lsp", - "description": "Query a language server for precise code navigation. operation is one of definition, references, implementation, hover. line and character are one-based UTF-16 cursor coordinates. references includes the declaration.", + "description": "Query a language server for precise code navigation. operation is one of goToDefinition, findReferences, goToImplementation, hover. line and character are one-based UTF-16 cursor coordinates. findReferences includes the declaration.", "parameters": { "type": "object", "properties": { "operation": { "type": "string", - "description": "definition, references, implementation, or hover.", + "description": "goToDefinition, findReferences, goToImplementation, or hover.", "enum": [ - "definition", - "references", - "implementation", + "goToDefinition", + "findReferences", + "goToImplementation", "hover" ] }, diff --git a/knip.json b/knip.json index 8565e5b462..e80eb596e5 100644 --- a/knip.json +++ b/knip.json @@ -1,7 +1,6 @@ { "$schema": "https://unpkg.com/knip@5/schema.json", "exclude": ["duplicates"], - "ignore": ["examples/*/tests/snapshots/*/workspace/**/*"], "ignoreBinaries": ["bwrap", "python3", "sandbox-exec"], "ignoreWorkspaces": ["vendor/*", "python/sdk-runtime"], "workspaces": { @@ -14,6 +13,7 @@ "headless-agent/tests/fixtures/goal-domain/seed-goal.ts", "headless-agent/tests/fixtures/time-context-driver.ts", "headless-agent/tests/fixtures/time-context-mock-llm.ts", + "acp-agent/tests/snapshots/lsp-definition/workspace/subject.ts", "tui-agent/tests/fixtures/tui-scripted-llm.ts", "*/tests/**/*.e2e.ts", "*/tests/**/*.snapshot.ts" diff --git a/packages/lsp/README.md b/packages/lsp/README.md index 12e8722671..147888a259 100644 --- a/packages/lsp/README.md +++ b/packages/lsp/README.md @@ -8,6 +8,6 @@ The language-server capability seam: an abstract LSP interface, a generic stdio | `lsp-local/` | Generic multi-server local backend (spawn, JSON-RPC, transient-open queries) | (registers providers on `ctx.lsp`) | | `tool-lsp/` | Model-facing `lsp` tool (four operations, one-based UTF-16 cursor coordinates) | (registers on `ctx.tools`) | -The interface lives at `lsp/lsp/`. The seam exposes exactly four semantic operations — `definition`, `references`, `implementation`, `hover` — and no generic JSON-RPC escape hatch, so a provider swap does not change how the model asks for navigation and no protocol payload or unreviewed mutation reaches the model contract. Providers register **capabilities**, not tools; `tool-lsp` is the only owner of the model-facing name, schema, prompt guidance, and presentation. +The interface lives at `lsp/lsp/`. The seam exposes exactly four semantic operations — `goToDefinition`, `findReferences`, `goToImplementation`, `hover` — and no generic JSON-RPC escape hatch, so a provider swap does not change how the model asks for navigation and no protocol payload or unreviewed mutation reaches the model contract. Providers register **capabilities**, not tools; `tool-lsp` is the only owner of the model-facing name, schema, prompt guidance, and presentation. See the [LSP capability seam Agent Note](../../.agents/notes/implemented/architecture/2026-07-15-lsp-capability-seam.md) for the design rationale, including why documents open transiently per query, why the local host reads through Node APIs rather than `ctx.fs`, and why extension ownership is exclusive within one runtime. diff --git a/packages/lsp/lsp-local/README.md b/packages/lsp/lsp-local/README.md index 35f109620a..66568b17d1 100644 --- a/packages/lsp/lsp-local/README.md +++ b/packages/lsp/lsp-local/README.md @@ -9,7 +9,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. -- Serializes queries through one abortable per-instance queue so a cancellation that fails to stop the server can terminate it without killing unrelated work; distinct instances run in parallel. +- 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. ## Configuration @@ -28,13 +28,13 @@ The `servers` record key is the stable provider id reserved on `ctx.lsp`; each v | `maxStderrBytes` | `1000000` | Largest stderr tail retained for diagnostics. | | `maxDocumentBytes` | `4000000` | Largest source file this host will open. | | `shutdownTimeoutMs` | `5000` | Graceful `shutdown`/`exit` budget before escalation. | -| `killGraceMs` | `2000` | SIGTERM→SIGKILL grace after graceful shutdown fails. | +| `killGraceMs` | `2000` | Grace for request cancellation and for SIGTERM→SIGKILL escalation. | -`servers` must contain at least one entry, and every id must be non-empty. All executables resolve at load after credential scrubbing; a bad later entry prevents every provider from registering. Processes launch lazily on the first matching query. +`servers` must contain at least one entry, and every id must be non-empty. Timer budgets must be positive integers no greater than Node's `2_147_483_647` ms timer limit. All executables resolve at load after credential scrubbing; a bad later entry prevents every provider from registering. Processes launch lazily on the first matching query. ## Protocol behavior -Initialization advertises `general.positionEncodings: ['utf-16']`, `workspace: { workspaceFolders: true, configuration: true }`, `textDocument.hover.contentFormat: ['markdown', 'plaintext']`, and `linkSupport: true` for definition and implementation, with no dynamic registration. The server's returned capabilities are authoritative: an unsupported operation, or synchronization without transient open/close, fails the query. An omitted server `positionEncoding` defaults to `utf-16`; any other value is a protocol error. The client answers `workspace/configuration` from static config, accepts lifecycle bookkeeping requests, and rejects `workspace/applyEdit` — it never applies edits or runs commands. Navigation maps `Location` directly and `LocationLink` from `targetUri` + `targetSelectionRange`; hover normalization takes `MarkupContent.value`, preserves string `MarkedString`s, renders language-tagged values as fenced code, and joins arrays with one blank line. +Initialization advertises `general.positionEncodings: ['utf-16']`, `workspace: { workspaceFolders: true, configuration: true }`, `textDocument.hover.contentFormat: ['markdown', 'plaintext']`, and `linkSupport: true` for definition and implementation, with no dynamic registration. The server's returned capabilities are authoritative: an unsupported operation, or synchronization without transient open/close, fails the query. An omitted server `positionEncoding` defaults to `utf-16`; any other value is a protocol error. The client answers `workspace/configuration` from static config, accepts lifecycle bookkeeping requests, and rejects `workspace/applyEdit` — it never applies edits or runs commands. Navigation maps `Location` directly and `LocationLink` from `targetUri` + `targetSelectionRange`; hover normalization takes valid `MarkupContent.value`, preserves string `MarkedString`s, renders language-tagged values as fenced code, and joins arrays with one blank line. Missing results, malformed ranges or positions, and malformed hover encodings fail as structured `LSP_MALFORMED_RESPONSE` errors. ## Security boundary @@ -50,6 +50,6 @@ No direct invalidation; `dsh-tool-lsp` owns request-prefix changes. ## Known Limitations and Deferred Work -- **Trusted host-local only** — no sandbox confinement, no private cache/temp write contract; supporting untrusted binaries or restricted/remote/virtual workspaces requires a later process/filesystem contract and a different provider ([seam Agent Note](../../../.agents/notes/implemented/architecture/2026-07-15-lsp-capability-seam.md)). Containment resolves `realpath`, then opens the source through one handle with `O_NOFOLLOW` (final-component symlink guard) and a bounded read; a concurrent mutator that swaps an *ancestor* directory for a symlink between the resolve and the open is an accepted residual TOCTOU under this trusted-deployment model, not closed with non-portable `openat` segment walks. +- **Trusted host-local only** — no sandbox confinement, no private cache/temp write contract; supporting untrusted binaries or restricted/remote/virtual workspaces requires a later process/filesystem contract and a different provider ([seam Agent Note](../../../.agents/notes/implemented/architecture/2026-07-15-lsp-capability-seam.md)). Containment resolves `realpath`, then opens the source through one handle with `O_NOFOLLOW | O_NONBLOCK` (final-component symlink guard plus nonblocking rejection of FIFOs) and a bounded read; a concurrent mutator that swaps an *ancestor* directory for a symlink between the resolve and the open is an accepted residual TOCTOU under this trusted-deployment model, not closed with non-portable `openat` segment walks. - **Transient-open compatibility floor** — servers whose synchronization omits open/close (or advertise `None`) are unsupported even if closed-document queries would work; the pinned TypeScript e2e establishes one compatibility floor, not a cross-language claim. - **Per-server/workspace serialization latency** — parallel agents sharing one server and workspace queue behind one process; long-lived workspace processes consume memory until disposal. diff --git a/packages/lsp/lsp-local/package.json b/packages/lsp/lsp-local/package.json index d437e91109..68e6cf7764 100644 --- a/packages/lsp/lsp-local/package.json +++ b/packages/lsp/lsp-local/package.json @@ -1,6 +1,6 @@ { "name": "@deepseek-ai/dsh-lsp-local", - "description": "Generic stdio language-server provider for the DeepSeek Harness LSP capability seam (ctx.lsp) — spawns configured servers, translates JSON-RPC, and serves transient-open definition/references/implementation/hover queries in the host filesystem namespace", + "description": "Generic stdio language-server provider for the DeepSeek Harness LSP capability seam (ctx.lsp) — spawns configured servers, translates JSON-RPC, and serves transient-open goToDefinition/findReferences/goToImplementation/hover queries in the host filesystem namespace", "version": "0.0.1", "private": true, "type": "module", diff --git a/packages/lsp/lsp-local/src/abort.ts b/packages/lsp/lsp-local/src/abort.ts new file mode 100644 index 0000000000..7790069e44 --- /dev/null +++ b/packages/lsp/lsp-local/src/abort.ts @@ -0,0 +1,48 @@ +/** + * Shared cancellation helpers for the local LSP provider's host-I/O, queue, and protocol phases. + * @module @deepseek-ai/dsh-lsp-local/abort + */ + +import { timeoutOf } from '@deepseek-ai/dsh-timeout' + +/** + * Build an abort Error carrying the signal's reason and preserving timeout classification. + * @param signal - the aborted signal whose reason to surface. + * @returns the timeout reason if present, else the Error reason, else a generic aborted Error. + */ +export function abortError(signal: AbortSignal): Error { + const timeout = timeoutOf(signal) + if (timeout !== undefined) return timeout + const reason: unknown = signal.reason + if (reason instanceof Error) return reason + return new Error('LSP query aborted') +} + +/** + * Throw the signal's classified abort error when it has already fired. + * @param signal - the optional query cancellation signal. + */ +export function throwIfAborted(signal?: AbortSignal): void { + if (signal?.aborted) throw abortError(signal) +} + +/** + * Await work while allowing a query signal to abandon its wait; the underlying work keeps its own + * handlers and continues to its owner-defined quiescence boundary. + * @param work - the owned asynchronous work. + * @param signal - optional query cancellation. + * @returns the work result, or a rejection carrying the classified abort reason. + */ +export function abortable(work: Promise, signal?: AbortSignal): Promise { + if (signal === undefined) return work + if (signal.aborted) return Promise.reject(abortError(signal)) + const canceled = Promise.withResolvers() + const onAbort = (): void => { canceled.reject(abortError(signal)) } + signal.addEventListener('abort', onAbort, { once: true }) + const normalized = work.catch((error: unknown) => { + /* v8 ignore next -- owned LSP promises reject with Error; coercion defends the generic helper. */ + throw error instanceof Error ? error : new Error(String(error)) + }) + return Promise.race([normalized, canceled.promise]) + .finally(() => { signal.removeEventListener('abort', onAbort) }) +} diff --git a/packages/lsp/lsp-local/src/connection.ts b/packages/lsp/lsp-local/src/connection.ts index 6acb321e90..ae725b56f7 100644 --- a/packages/lsp/lsp-local/src/connection.ts +++ b/packages/lsp/lsp-local/src/connection.ts @@ -75,10 +75,10 @@ export class LspConnection { }) }) this.child.on('error', (error) => { this.fail(error) }) - // A write to the child's stdin after it exits emits an async 'error'; swallow it so an EPIPE - // during teardown does not crash the process. Pending requests fail via the 'close' handler. - /* v8 ignore next -- the handler only fires on an async stdin write error during teardown. */ - this.child.stdin.on('error', () => { /* swallow */ }) + // Child stdin can fail while the process itself remains alive (for example, a server closes fd + // 0). Treat that as a fatal connection error so pending requests reject immediately instead of + // waiting for a process-close event that may never arrive. + this.child.stdin.on('error', (error) => { this.fail(error) }) this.child.stdout.on('data', (chunk: Buffer) => { this.onStdout(chunk) }) this.child.stderr.on('data', (chunk: Buffer) => { this.onStderr(chunk) }) } @@ -108,15 +108,9 @@ export class LspConnection { return } this.pending.set(id, { resolve, reject }) - try { - this.write({ jsonrpc: '2.0', id, method, params }) - } catch (error) { - /* v8 ignore start -- a stdin write failure surfaces asynchronously via the swallowed - 'error' listener, so this synchronous catch is a defensive guard. */ - this.pending.delete(id) - reject(asError(error)) - /* v8 ignore stop */ - } + // `write()` records either synchronous or callback-delivered failures on the connection and + // rejects every pending request. This handler only consumes the write promise itself. + void this.write({ jsonrpc: '2.0', id, method, params }).catch(() => {}) }) // A caller that stops awaiting (e.g. an aborted query) can leave this promise to reject later // when the process closes; a benign no-op handler keeps that from surfacing as an unhandled @@ -129,9 +123,10 @@ export class LspConnection { * Send a notification (no id, no response). * @param method - the JSON-RPC method. * @param params - the notification params. + * @returns a promise that settles when the framed notification has been written. */ - notify(method: string, params: unknown): void { - this.write({ jsonrpc: '2.0', method, params }) + notify(method: string, params: unknown): Promise { + return this.write({ jsonrpc: '2.0', method, params }) } /** @@ -139,11 +134,9 @@ export class LspConnection { * @param requestId - the numeric id of the request to cancel. */ cancel(requestId: number): void { - try { - this.write({ jsonrpc: '2.0', method: '$/cancelRequest', params: { id: requestId } }) - } catch { - // The server is already gone or unwritable; the pending request will fail on close. - } + // The server is already gone or unwritable when this rejects; `write()` has recorded the fatal + // connection failure and rejected the pending request, so cancellation remains best-effort. + void this.write({ jsonrpc: '2.0', method: '$/cancelRequest', params: { id: requestId } }).catch(() => {}) } /** @@ -253,7 +246,10 @@ export class LspConnection { const id = frame.id const method = frame.method if (typeof method === 'string' && (typeof id === 'number' || typeof id === 'string')) { - void this.handleServerRequest(id, method, frame.params) + // A response-write failure has already invalidated the connection in `write()`. + /* v8 ignore next -- protocol tests exercise response writes; only a simultaneous connection + failure makes this consumption handler run. */ + void this.handleServerRequest(id, method, frame.params).catch(() => {}) return } if (typeof method === 'string') { @@ -266,9 +262,9 @@ export class LspConnection { private async handleServerRequest(id: number | string, method: string, params: unknown): Promise { try { const result = await this.onServerRequest(method, params) - this.write({ jsonrpc: '2.0', id, result }) + await this.write({ jsonrpc: '2.0', id, result }) } catch (error) { - this.write({ jsonrpc: '2.0', id, error: { code: -32601, message: asError(error).message } }) + await this.write({ jsonrpc: '2.0', id, error: { code: -32601, message: asError(error).message } }) } } @@ -285,8 +281,28 @@ export class LspConnection { pending.resolve(frame.result) } - private write(message: unknown): void { - this.child.stdin.write(encodeMessage(message)) + private write(message: unknown): Promise { + if (this.closeReason !== undefined) return Promise.reject(this.closeReason) + return new Promise((resolve, reject) => { + const done = (error?: Error | null): void => { + if (error === undefined || error === null) { + resolve() + return + } + this.fail(error) + reject(error) + } + try { + this.child.stdin.write(encodeMessage(message), done) + /* v8 ignore start -- Node stream write failures are callback-delivered; this guards a + nonconforming Writable implementation throwing synchronously. */ + } catch (error) { + const failure = asError(error) + this.fail(failure) + reject(failure) + } + /* v8 ignore stop */ + }) } /** The exit-close error message, appending the retained stderr tail when the server wrote any. */ diff --git a/packages/lsp/lsp-local/src/host.ts b/packages/lsp/lsp-local/src/host.ts index 949a1b838b..11996908ac 100644 --- a/packages/lsp/lsp-local/src/host.ts +++ b/packages/lsp/lsp-local/src/host.ts @@ -13,6 +13,7 @@ import { constants } from 'node:fs' import { open, realpath, stat } from 'node:fs/promises' import type { FileHandle } from 'node:fs/promises' import { isAbsolute, resolve as resolvePath, sep } from 'node:path' +import { throwIfAborted } from './abort.ts' /** A validated source: its canonical absolute path and current UTF-8 text. */ export interface HostSource { @@ -27,17 +28,21 @@ export interface HostSource { * process cwd, `rootUri`, the sole `workspaceFolders` entry, and pool identity, so symlinked roots * collapse to one instance. * @param workspaceRoot - the caller's workspace root (absolute). + * @param signal - optional cancellation observed around each filesystem operation. * @returns the canonical directory path. * @throws Error when the path is missing or not a directory. */ -export async function canonicalizeWorkspace(workspaceRoot: string): Promise { +export async function canonicalizeWorkspace(workspaceRoot: string, signal?: AbortSignal): Promise { + throwIfAborted(signal) let canonical: string try { canonical = await realpath(workspaceRoot) } catch (error) { throw new Error(`workspace root "${workspaceRoot}" cannot be resolved: ${messageOf(error)}`) } + throwIfAborted(signal) const info = await stat(canonical) + throwIfAborted(signal) if (!info.isDirectory()) { throw new Error(`workspace root "${workspaceRoot}" is not a directory`) } @@ -52,6 +57,7 @@ export async function canonicalizeWorkspace(workspaceRoot: string): Promise { + throwIfAborted(signal) const requested = isAbsolute(filePath) ? filePath : resolvePath(canonicalWorkspace, filePath) let canonicalPath: string try { @@ -67,6 +75,7 @@ export async function readHostSource( } catch (error) { throw new Error(`source "${filePath}" cannot be resolved: ${messageOf(error)}`) } + throwIfAborted(signal) if (!isInside(canonicalWorkspace, canonicalPath)) { throw new Error(`source "${filePath}" resolves outside the workspace`) } @@ -74,9 +83,12 @@ export async function readHostSource( // realpath and read cannot swap the target, so the regular-file and size checks bind the bytes we // actually read (no path-based TOCTOU). O_NOFOLLOW rejects the final component being swapped for a // symlink between realpath and open (which would otherwise escape the workspace). - const handle = await open(canonicalPath, constants.O_RDONLY | constants.O_NOFOLLOW) + // O_NONBLOCK prevents a FIFO with no writer from hanging before fstat can reject it as nonregular. + const handle = await open(canonicalPath, constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK) try { + throwIfAborted(signal) const info = await handle.stat() + throwIfAborted(signal) if (!info.isFile()) { throw new Error(`source "${filePath}" is not a regular file`) } @@ -85,8 +97,9 @@ export async function readHostSource( } // Bound the read to the cap even if the file grew after stat: read one extra byte and reject on // overflow, so a concurrent grow cannot defeat the memory bound. - const buffer = await readCapped(handle, maxDocumentBytes, filePath) + const buffer = await readCapped(handle, maxDocumentBytes, filePath, signal) const text = decodeUtf8Strict(buffer, filePath) + throwIfAborted(signal) return { canonicalPath, text } } finally { await handle.close() @@ -94,12 +107,19 @@ export async function readHostSource( } /** Read at most `maxBytes` from the handle, rejecting when the source overflows the cap. */ -async function readCapped(handle: FileHandle, maxBytes: number, filePath: string): Promise { +async function readCapped( + handle: FileHandle, + maxBytes: number, + filePath: string, + signal?: AbortSignal, +): Promise { const limit = maxBytes + 1 const chunk = Buffer.allocUnsafe(limit) let total = 0 for (;;) { + throwIfAborted(signal) const { bytesRead } = await handle.read(chunk, total, limit - total, total) + throwIfAborted(signal) if (bytesRead === 0) break total += bytesRead /* v8 ignore next 3 -- overflow requires the file to grow past the cap between stat and read (a concurrent mutation); defensive. */ diff --git a/packages/lsp/lsp-local/src/index.ts b/packages/lsp/lsp-local/src/index.ts index 67e86da11c..095d761624 100644 --- a/packages/lsp/lsp-local/src/index.ts +++ b/packages/lsp/lsp-local/src/index.ts @@ -15,14 +15,16 @@ import { accessSync, constants, statSync } from 'node:fs' import { delimiter, isAbsolute, join } from 'node:path' import type { Context } from 'cordis' import z from 'schemastery' -import { LspProviderId } from '@deepseek-ai/dsh-lsp' +import { LspError, LspProviderId } from '@deepseek-ai/dsh-lsp' import type { LspProvider, LspProviderQuery, LspQueryResult, } from '@deepseek-ai/dsh-lsp' +import { MAX_TIMER_DELAY_MS } from '@deepseek-ai/dsh-timeout' +import { abortable, abortError } from './abort.ts' import { canonicalizeWorkspace, readHostSource } from './host.ts' -import { abortError, LspInstance } from './instance.ts' +import { LspInstance } from './instance.ts' import type { InstanceSpec } from './instance.ts' export { canonicalizeWorkspace, readHostSource } from './host.ts' @@ -75,7 +77,7 @@ export interface LspLocalServerConfig { maxDocumentBytes?: number /** Graceful `shutdown`/`exit` budget before escalation (ms). Default 5000. */ shutdownTimeoutMs?: number - /** SIGTERM→SIGKILL grace after graceful shutdown fails (ms). Default 2000. */ + /** Request-cancel and SIGTERM→SIGKILL grace (ms). Default 2000. */ killGraceMs?: number } @@ -98,8 +100,8 @@ const LspLocalServerConfig: z = z.object({ maxMessageBytes: z.number().default(DEFAULT_MAX_MESSAGE_BYTES), maxStderrBytes: z.number().default(DEFAULT_MAX_STDERR_BYTES), maxDocumentBytes: z.number().default(DEFAULT_MAX_DOCUMENT_BYTES), - shutdownTimeoutMs: z.number().default(DEFAULT_SHUTDOWN_TIMEOUT_MS), - killGraceMs: z.number().default(DEFAULT_KILL_GRACE_MS), + shutdownTimeoutMs: z.number().max(MAX_TIMER_DELAY_MS).default(DEFAULT_SHUTDOWN_TIMEOUT_MS), + killGraceMs: z.number().max(MAX_TIMER_DELAY_MS).default(DEFAULT_KILL_GRACE_MS), }) export const Config: z = z.object({ @@ -148,8 +150,8 @@ export function apply(ctx: Context, config: Config): void { function validateServerConfig(providerId: string, resolved: ResolvedServerConfig): void { // Teardown budgets feed `deadline()`, whose `<= 0` is the internal no-timeout sentinel; a // nonpositive value would let a server that ignores shutdown hang disposal forever. Fail at load. - assertPositiveInteger(providerId, 'shutdownTimeoutMs', resolved.shutdownTimeoutMs) - assertPositiveInteger(providerId, 'killGraceMs', resolved.killGraceMs) + assertTimer(providerId, 'shutdownTimeoutMs', resolved.shutdownTimeoutMs) + assertTimer(providerId, 'killGraceMs', resolved.killGraceMs) // Byte caps must be positive: a nonpositive stderr cap defeats the retained-tail bound // (`slice(-0)` keeps everything), `maxMessageBytes: 0` makes every response fatal, and a bad // document cap fails later in the read path instead of at load. @@ -158,6 +160,13 @@ function validateServerConfig(providerId: string, resolved: ResolvedServerConfig assertPositiveInteger(providerId, 'maxDocumentBytes', resolved.maxDocumentBytes) } +/** Reject a timer value Node would clamp instead of scheduling as configured. */ +function assertTimer(providerId: string, name: string, value: number): void { + if (!Number.isInteger(value) || value < 1 || value > MAX_TIMER_DELAY_MS) { + throw new Error(`lsp-local: servers.${providerId}.${name} must be a positive integer no greater than ${MAX_TIMER_DELAY_MS}`) + } +} + /** Reject a nonpositive or non-integer config value at load, so misconfiguration fails loud. */ function assertPositiveInteger(providerId: string, name: string, value: number): void { if (!Number.isInteger(value) || value < 1) { @@ -171,6 +180,8 @@ class LocalLspProvider implements LspProvider { readonly extensionToLanguage: Readonly> /** One live instance per canonical workspace realpath. */ private readonly instances = new Map() + /** One complete source-read→open→query→close serialization tail per canonical workspace. */ + private readonly queues = new Map>() private disposed = false constructor( @@ -192,35 +203,49 @@ class LocalLspProvider implements LspProvider { private assertActive(signal?: AbortSignal): void { /* v8 ignore next -- the seam unregisters this provider before disposal; direct in-flight calls exercise the post-await check instead. */ - if (this.isDisposed()) throw new Error('lsp-local provider is disposed') + if (this.isDisposed()) throw new LspError('lsp-local provider is disposed', 'LSP_DISPOSED') if (signal?.aborted) throw abortError(signal) } async query(request: LspProviderQuery, signal?: AbortSignal): Promise { // Honor an already-aborted signal before host I/O so a canceled request never starts a server. this.assertActive(signal) - const workspace = await canonicalizeWorkspace(request.workspaceRoot) - // Validate and read the source BEFORE spawning a server: a missing/external/non-regular/oversized - // source must fail without leaving an idle process pooled (the pre-start rejection contract), and - // the single-handle read preserves the containment/size checks against a mid-read swap. - const source = await readHostSource(request.filePath, workspace, this.config.maxDocumentBytes) - // Re-check disposal after the awaits: disposeAll() may have snapshotted the instance map while we - // were canonicalizing/reading, so creating a server now would leave it unowned by teardown. + const workspace = await canonicalizeWorkspace(request.workspaceRoot, signal) this.assertActive(signal) - let instance = this.instanceFor(workspace) - // Eviction and replacement are synchronous so disposal cannot snapshot the pool between them and - // miss a newly spawned process. - if (instance.dead) { - this.evictIfCurrent(workspace, instance) - instance = this.instanceFor(workspace) - } - try { - return await instance.query(request, source, signal) - } finally { - // A crashed/closed process must not be reused: drop its slot so the next query starts fresh, - // but only if the slot still holds THIS instance (a concurrent replacement must survive). - if (instance.dead) this.evictIfCurrent(workspace, instance) - } + return this.enqueue(workspace, signal, async () => { + this.assertActive(signal) + // Read inside the workspace queue but before spawning: a queued query sees current bytes when + // its turn starts, while an invalid source still cannot leave an idle process pooled. + const source = await readHostSource(request.filePath, workspace, this.config.maxDocumentBytes, signal) + // Disposal may have snapshotted the instance map while host I/O was pending. Re-check before a + // 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) + } finally { + // Drop a crashed slot only when it still owns this instance; a replacement must survive. + if (instance.dead) this.evictIfCurrent(workspace, instance) + } + }) + } + + /** Serialize one complete query lifecycle for a canonical workspace. */ + private enqueue(workspace: string, signal: AbortSignal | undefined, run: () => Promise): Promise { + const previous = this.queues.get(workspace) ?? Promise.resolve() + const result = abortable(previous, signal).then(run) + // The tail follows the actual prior work even when this caller aborts its wait. It never rejects, + // so later callers serialize without inheriting an earlier query's outcome. + const tail = previous.then(() => result).then(() => undefined, () => undefined) + this.queues.set(workspace, tail) + void tail.then(() => { + if (this.queues.get(workspace) === tail) this.queues.delete(workspace) + }) + return result } /** Return or synchronously publish the one instance for a canonical workspace. */ @@ -259,8 +284,13 @@ class LocalLspProvider implements LspProvider { async disposeAll(): Promise { this.disposed = true const live = [...this.instances.values()] + const draining = [...this.queues.values()] this.instances.clear() - await Promise.all(live.map(instance => instance.dispose())) + await Promise.all([ + ...live.map(instance => instance.dispose()), + ...draining, + ]) + this.queues.clear() } } diff --git a/packages/lsp/lsp-local/src/instance.ts b/packages/lsp/lsp-local/src/instance.ts index fb67f6e777..9d327fb795 100644 --- a/packages/lsp/lsp-local/src/instance.ts +++ b/packages/lsp/lsp-local/src/instance.ts @@ -14,7 +14,8 @@ import type { LspProviderQuery, LspQueryResult, } from '@deepseek-ai/dsh-lsp' -import { deadline, timeoutOf } from '@deepseek-ai/dsh-timeout' +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 { HostSource } from './host.ts' @@ -83,7 +84,7 @@ 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 = this.abortable(this.queue, signal).then(() => this.runQuery(request, source, signal)) + const run = abortable(this.queue, signal).then(() => this.runQuery(request, source, signal)) // 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. @@ -103,11 +104,11 @@ export class LspInstance { // An omitted encoding defaults to utf-16; any other value is a protocol error we reject here. negotiatePositionEncoding(capabilities.positionEncoding) this.capabilities = capabilities - this.connection.notify('initialized', {}) + await this.connection.notify('initialized', {}) } private async runQuery(request: LspProviderQuery, source: HostSource, signal?: AbortSignal): Promise { - if (this.disposed) throw new Error('LSP instance was disposed') + if (this.disposed) throw new LspError('LSP instance was disposed', 'LSP_DISPOSED') /* v8 ignore next -- the abortable queue wait rejects a pre-aborted signal before runQuery; this is a belt-and-suspenders guard. */ if (signal?.aborted) throw abortError(signal) // Observe abort during the handshake wait, and never pool a poisoned instance: if the wait ends @@ -115,11 +116,10 @@ export class LspInstance { // negotiation, malformed result) without the process exiting — tear the instance down so a // permanently-rejecting/pending `ready` can't make every later query for this workspace fail. try { - await this.abortable(this.ready, signal) + await abortable(this.ready, signal) } catch (error) { if (!this.dead) { - /* v8 ignore next -- ready rejects with an Error (abort reason or initialize failure); the String() fallback is defensive. */ - await this.startTeardown(error instanceof Error ? error : new Error(String(error))) + await this.startTeardown() } throw error } @@ -138,7 +138,7 @@ 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) - this.connection.notify('textDocument/didOpen', { + await this.connection.notify('textDocument/didOpen', { textDocument: { uri, languageId: request.languageId, version: 1, text: source.text }, }) opened = true @@ -150,34 +150,21 @@ export class LspInstance { // the next queued query's document lifecycle overlap the still-active request. if (opened && !this.dead) { try { - this.connection.notify('textDocument/didClose', { textDocument: { uri } }) - } catch (error) { - /* v8 ignore start -- stdin write errors surface asynchronously via the swallowed 'error' - listener, so a synchronous didClose write failure is a defensive path. */ + await this.connection.notify('textDocument/didClose', { textDocument: { uri } }) + } catch { // A close-write failure does not replace the settled result/error, but the instance can no // longer be trusted: invalidate it and await bounded process termination. - void this.startTeardown(error instanceof Error ? error : new Error(String(error))) - /* v8 ignore stop */ + try { + await this.startTeardown() + } catch { + /* v8 ignore next -- teardown owns all expected process races; this only preserves the + already-settled query outcome if an unexpected cleanup primitive itself rejects. */ + } } } } } - /** - * Await `work`, but reject as soon as `signal` aborts. The underlying `work` promise keeps its own - * handlers, so an orphaned rejection after abort is not unhandled. - */ - private abortable(work: Promise, signal: AbortSignal | undefined): Promise { - if (signal === undefined) return work - /* v8 ignore next -- callers either pre-check the signal or pass a freshly armed teardown deadline. */ - if (signal.aborted) return Promise.reject(abortError(signal)) - return new Promise((resolve, reject) => { - const onAbort = (): void => { reject(abortError(signal)) } - signal.addEventListener('abort', onAbort, { once: true }) - work.then(resolve, reject).finally(() => { signal.removeEventListener('abort', onAbort) }) - }) - } - private async sendRequest( operation: LspOperation, uri: string, @@ -187,9 +174,9 @@ export class LspInstance { const params = { textDocument: { uri }, position: { line: position.line, character: position.character }, - // references always includes declarations: the caller gets no flag and impact analysis never - // omits the defining site. - ...(operation === 'references' ? { context: { includeDeclaration: true } } : {}), + // findReferences always includes declarations: the caller gets no flag and impact analysis + // never omits the defining site. + ...(operation === 'findReferences' ? { context: { includeDeclaration: true } } : {}), } const requestId = this.connection.peekNextId() const send = this.connection.request(requestMethod(operation), params) @@ -204,7 +191,7 @@ export class LspInstance { */ private async raceAbort(send: Promise, requestId: number, signal: AbortSignal): Promise { try { - return await this.abortable(send, signal) + return await abortable(send, signal) } catch (error) { if (!signal.aborted) throw error this.connection.cancel(requestId) @@ -221,7 +208,7 @@ export class LspInstance { grace.signal.addEventListener('abort', () => { resolve(false) }, { once: true }) }), ]) - if (!settled) await this.startTeardown(abortError(signal)) + if (!settled) await this.startTeardown() } finally { grace[Symbol.dispose]() } @@ -263,17 +250,17 @@ export class LspInstance { * process close so nothing outlives disposal. */ async dispose(): Promise { - await this.startTeardown(new Error('LSP instance disposed')) + await this.startTeardown() } /** Publish disposal once and make every caller await the same quiescence boundary. */ - private startTeardown(reason: Error): Promise { + private startTeardown(): Promise { this.disposed = true - this.teardownPromise ??= this.tearDown(reason) + this.teardownPromise ??= this.tearDown() return this.teardownPromise } - private async tearDown(_reason: Error): Promise { + private async tearDown(): Promise { const shutdownDeadline = deadline(undefined, this.spec.shutdownTimeoutMs, 'LSP_SHUTDOWN') try { await this.gracefulShutdown(shutdownDeadline.signal) @@ -287,9 +274,9 @@ export class LspInstance { /** Best-effort LSP `shutdown`/`exit`, including process close, bounded by `signal`. */ private async gracefulShutdown(signal: AbortSignal): Promise { - await this.abortable(this.connection.request('shutdown', null), signal) - this.connection.notify('exit', null) - await this.abortable(this.connection.closed, signal) + await abortable(this.connection.request('shutdown', null), signal) + await this.connection.notify('exit', null) + await abortable(this.connection.closed, signal) } /** SIGTERM the group, escalate after `killGraceMs`, then await leader and helper exit. */ @@ -322,19 +309,6 @@ function markSettled(): boolean { return true } -/** - * Build an abort Error carrying the signal's reason (preserving a timeout classification). - * @param signal - the aborted signal whose reason to surface. - * @returns the timeout reason if the signal carries one, else the signal's Error reason, else a generic aborted Error. - */ -export function abortError(signal: AbortSignal): Error { - const timeout = timeoutOf(signal) - if (timeout !== undefined) return timeout - const reason: unknown = signal.reason - if (reason instanceof Error) return reason - return new Error('LSP query aborted') -} - /** * The client capabilities advertised at `initialize`: UTF-16 positions, workspace folders and * configuration, markdown/plaintext hover, and link support for definition/implementation. No diff --git a/packages/lsp/lsp-local/src/translate.ts b/packages/lsp/lsp-local/src/translate.ts index a208c3bedf..a49246c8bb 100644 --- a/packages/lsp/lsp-local/src/translate.ts +++ b/packages/lsp/lsp-local/src/translate.ts @@ -11,6 +11,7 @@ import type { LspOperation, LspRange, } from '@deepseek-ai/dsh-lsp' +import { LspError } from '@deepseek-ai/dsh-lsp' import { assertNever } from '@deepseek-ai/dsh-llm' import type { WireHover, @@ -30,9 +31,9 @@ import type { */ export function requestMethod(operation: LspOperation): string { switch (operation) { - case 'definition': return 'textDocument/definition' - case 'references': return 'textDocument/references' - case 'implementation': return 'textDocument/implementation' + case 'goToDefinition': return 'textDocument/definition' + case 'findReferences': return 'textDocument/references' + case 'goToImplementation': return 'textDocument/implementation' case 'hover': return 'textDocument/hover' /* v8 ignore next -- exhaustive over the closed LspOperation union; unreachable. */ default: return assertNever(operation, 'requestMethod') @@ -42,9 +43,9 @@ export function requestMethod(operation: LspOperation): string { /** The `ServerCapabilities` provider field backing each operation. */ function capabilityValue(capabilities: WireServerCapabilities, operation: LspOperation): WireProviderCapability { switch (operation) { - case 'definition': return capabilities.definitionProvider - case 'references': return capabilities.referencesProvider - case 'implementation': return capabilities.implementationProvider + case 'goToDefinition': return capabilities.definitionProvider + case 'findReferences': return capabilities.referencesProvider + case 'goToImplementation': return capabilities.implementationProvider case 'hover': return capabilities.hoverProvider /* v8 ignore next -- exhaustive over the closed LspOperation union; unreachable. */ default: return assertNever(operation, 'capabilityValue') @@ -127,7 +128,12 @@ function isRange(value: unknown): value is WireRange { function isPosition(value: unknown): boolean { if (value === null || typeof value !== 'object') return false const position = value as Record - return typeof position.line === 'number' && typeof position.character === 'number' + return isProtocolCoordinate(position.line) && isProtocolCoordinate(position.character) +} + +/** Whether a wire coordinate is a valid nonnegative integer. */ +function isProtocolCoordinate(value: unknown): value is number { + return typeof value === 'number' && Number.isInteger(value) && value >= 0 } /** @@ -138,12 +144,13 @@ function isPosition(value: unknown): boolean { * @throws Error when an element is neither a `Location` nor a `LocationLink`. */ export function normalizeLocations(payload: unknown): LspLocation[] { - if (payload === null || payload === undefined) return [] + if (payload === null) return [] + if (payload === undefined) throw malformedResponse('LSP navigation result was missing') const elements = Array.isArray(payload) ? payload : [payload] const locations: LspLocation[] = [] for (const element of elements) { if (element === null || typeof element !== 'object') { - throw new Error('LSP navigation result contained a non-object entry') + throw malformedResponse('LSP navigation result contained a non-object entry') } const record = element as Record if (isLocationLink(record)) { @@ -153,7 +160,7 @@ export function normalizeLocations(payload: unknown): LspLocation[] { const location = record as unknown as WireLocation locations.push({ uri: location.uri, range: toRange(location.range) }) } else { - throw new Error('LSP navigation result contained neither a Location nor a LocationLink') + throw malformedResponse('LSP navigation result contained neither a Location nor a LocationLink') } } return locations @@ -168,39 +175,61 @@ function renderMarkedString(value: WireMarkedString): string { /** * Normalize a `Hover` (or `null`) to the seam's hover. `MarkupContent` uses its `value`; a string * `MarkedString` is verbatim; a language-tagged `MarkedString` becomes a fenced code block; an array - * joins its rendered parts with one blank line. `maxHoverChars` is NOT applied here — the tool caps. + * joins its rendered parts with one blank line. The model-facing tool owns the complete result cap. * @param payload - the raw `textDocument/hover` result. * @returns the normalized hover, or `null` when there is no content. * @throws Error when the payload is a non-null, non-object, or structurally invalid hover. */ export function normalizeHover(payload: unknown): LspHover | null { - if (payload === null || payload === undefined) return null - if (typeof payload !== 'object') throw new Error('LSP hover result was not an object') + if (payload === null) return null + if (payload === undefined) throw malformedResponse('LSP hover result was missing') + if (typeof payload !== 'object') throw malformedResponse('LSP hover result was not an object') const hover = payload as unknown as WireHover const contents = renderHoverContents(hover.contents) if (contents === '') return null const range = hover.range - return range !== undefined && isRange(range) ? { contents, range: toRange(range) } : { contents } + if (range === undefined) return { contents } + if (!isRange(range)) throw malformedResponse('LSP hover result contained a malformed range') + return { contents, range: toRange(range) } } /** Render the three `Hover.contents` encodings into one string (input is untrusted wire data). */ function renderHoverContents(contents: unknown): string { if (contents === null || contents === undefined) { - throw new Error('LSP hover result had no contents') + throw malformedResponse('LSP hover result had no contents') } if (typeof contents === 'string') return contents if (Array.isArray(contents)) { - return contents.map(renderMarkedString).join('\n\n') + return contents.map((value) => { + if (isMarkedString(value)) return renderMarkedString(value) + throw malformedResponse('LSP hover contents contained a malformed MarkedString') + }).join('\n\n') } if (typeof contents !== 'object') { - throw new Error('LSP hover contents were not MarkupContent, MarkedString, or an array') + throw malformedResponse('LSP hover contents were not MarkupContent, MarkedString, or an array') } const record = contents as Record if (record.kind === 'markdown' || record.kind === 'plaintext') { - return typeof record.value === 'string' ? record.value : '' + if (typeof record.value !== 'string') { + throw malformedResponse('LSP hover MarkupContent value was not a string') + } + return record.value } if (typeof record.language === 'string' && typeof record.value === 'string') { return renderMarkedString({ language: record.language, value: record.value }) } - throw new Error('LSP hover contents were not MarkupContent, MarkedString, or an array') + throw malformedResponse('LSP hover contents were not MarkupContent, MarkedString, or an array') +} + +/** Whether an untrusted value is either form of `MarkedString`. */ +function isMarkedString(value: unknown): value is WireMarkedString { + if (typeof value === 'string') return true + if (value === null || typeof value !== 'object') return false + const record = value as Record + return typeof record.language === 'string' && typeof record.value === 'string' +} + +/** Create the stable structured error used for malformed server result payloads. */ +function malformedResponse(message: string): LspError { + return new LspError(message, 'LSP_MALFORMED_RESPONSE') } diff --git a/packages/lsp/lsp-local/tests/built-lib.e2e.ts b/packages/lsp/lsp-local/tests/built-lib.e2e.ts index 799069c0fd..a2da86d87c 100644 --- a/packages/lsp/lsp-local/tests/built-lib.e2e.ts +++ b/packages/lsp/lsp-local/tests/built-lib.e2e.ts @@ -18,9 +18,7 @@ const pkgDir = fileURLToPath(new URL('..', import.meta.url)) const seamLib = join(pkgDir, '../lsp/lib/index.js') const built = existsSync(join(pkgDir, 'lib/index.js')) && existsSync(seamLib) -const tsxLoader = fileURLToPath(import.meta.resolve('tsx')) const fixtureServer = fileURLToPath(new URL('./fixture-server.ts', import.meta.url)) -const repoTsconfig = fileURLToPath(new URL('../../../../tsconfig.json', import.meta.url)) let root: string let ws: string @@ -49,13 +47,13 @@ describe.skipIf(!built)('built lib real load path (plain node)', () => { servers: { fake: { command: ${JSON.stringify(process.execPath)}, - args: ['--import', ${JSON.stringify(tsxLoader)}, ${JSON.stringify(fixtureServer)}], - env: { TSX_TSCONFIG_PATH: ${JSON.stringify(repoTsconfig)}, LSP_FAKE_DEF: ${JSON.stringify(location)} }, + args: [${JSON.stringify(fixtureServer)}], + env: { LSP_FAKE_DEF: ${JSON.stringify(location)} }, extensionToLanguage: { '.ts': 'typescript' }, }, }, }) - const result = await ctx.lsp.query({ operation: 'definition', filePath: 'a.ts', position: { line: 0, character: 6 }, workspaceRoot: ${JSON.stringify(ws)} }) + const result = await ctx.lsp.query({ operation: 'goToDefinition', filePath: 'a.ts', position: { line: 0, character: 6 }, workspaceRoot: ${JSON.stringify(ws)} }) console.log(JSON.stringify(result)) await ctx.fiber.dispose() ` diff --git a/packages/lsp/lsp-local/tests/connection.spec.ts b/packages/lsp/lsp-local/tests/connection.spec.ts index 25ac9f2971..6d9ca6d6a3 100644 --- a/packages/lsp/lsp-local/tests/connection.spec.ts +++ b/packages/lsp/lsp-local/tests/connection.spec.ts @@ -2,9 +2,7 @@ import { afterEach, describe, expect, it } from 'vitest' import { fileURLToPath } from 'node:url' import { LspConnection } from '@deepseek-ai/dsh-lsp-local' -const tsxLoader = fileURLToPath(import.meta.resolve('tsx')) const fixtureServer = fileURLToPath(new URL('./fixture-server.ts', import.meta.url)) -const repoTsconfig = fileURLToPath(new URL('../../../../tsconfig.json', import.meta.url)) /** A recorded server→client request the test's handler saw. */ interface SeenRequest { method: string; params: unknown } @@ -27,9 +25,9 @@ function connect( ): LspConnection { const conn = new LspConnection({ command: process.execPath, - args: ['--import', tsxLoader, fixtureServer], + args: [fixtureServer], cwd: process.cwd(), - env: { ...process.env as Record, TSX_TSCONFIG_PATH: repoTsconfig, ...env }, + env: { ...process.env as Record, ...env }, maxMessageBytes: 16_000_000, maxStderrBytes: 100_000, configuration: { setting: 42 }, @@ -69,7 +67,7 @@ describe('LspConnection', () => { seen, ) await conn.request('initialize', { capabilities: {} }) - conn.notify('textDocument/didOpen', { textDocument: { uri: 'file:///x', languageId: 'ts', version: 1, text: '' } }) + await conn.notify('textDocument/didOpen', { textDocument: { uri: 'file:///x', languageId: 'ts', version: 1, text: '' } }) await waitFor(() => seen.some(s => s.method === 'workspace/configuration')) expect(seen[0]?.method).toBe('workspace/configuration') }) @@ -77,7 +75,7 @@ describe('LspConnection', () => { it('drops a server→client notification without replying', async () => { const conn = connect({ LSP_FAKE_ON_OPEN: 'notification' }) await conn.request('initialize', { capabilities: {} }) - conn.notify('textDocument/didOpen', { textDocument: { uri: 'file:///x', languageId: 'ts', version: 1, text: '' } }) + await conn.notify('textDocument/didOpen', { textDocument: { uri: 'file:///x', languageId: 'ts', version: 1, text: '' } }) // No throw and the connection stays usable. await expect(conn.request('textDocument/hover', {})).resolves.toBeDefined() }) @@ -90,7 +88,7 @@ describe('LspConnection', () => { seen, ) await conn.request('initialize', { capabilities: {} }) - conn.notify('textDocument/didOpen', { textDocument: { uri: 'file:///x', languageId: 'ts', version: 1, text: '' } }) + await conn.notify('textDocument/didOpen', { textDocument: { uri: 'file:///x', languageId: 'ts', version: 1, text: '' } }) await waitFor(() => seen.some(s => s.method === 'workspace/applyEdit')) // The connection remains healthy after emitting the error response. await expect(conn.request('textDocument/hover', {})).resolves.toBeDefined() @@ -211,6 +209,15 @@ describe('LspConnection edge behavior', () => { await expect(conn.request('initialize', {})).rejects.toThrow(/exited|closed/) }) + it('rejects a pending request when child stdin closes but the process stays alive', async () => { + const conn = connectScript('require("node:fs").closeSync(0); 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('ignores a frame that is neither a valid request nor a numeric-id response', async () => { // A frame with a string id and no method: not dispatchable; the client must ignore it and still // answer our real request. diff --git a/packages/lsp/lsp-local/tests/fixture-server.ts b/packages/lsp/lsp-local/tests/fixture-server.ts index 1654cb820d..8dec856274 100644 --- a/packages/lsp/lsp-local/tests/fixture-server.ts +++ b/packages/lsp/lsp-local/tests/fixture-server.ts @@ -12,6 +12,9 @@ * - LSP_FAKE_CRASH_ON_OPEN: "1" exits the process when a didOpen arrives (crash test). * - LSP_FAKE_EXIT_AFTER_REPLY: "1" exits the process right after answering a textDocument/* request, * 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_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). * - LSP_FAKE_ON_OPEN: server→client request to emit when a didOpen arrives, one of @@ -19,10 +22,10 @@ * - LSP_FAKE_ERROR: "1" answers textDocument/* requests with a JSON-RPC error response. * - LSP_FAKE_GARBAGE: "1" emits an unframed garbage byte before the initialize reply. * - * Run: node --import tsx fixture-server.ts + * Run: node fixture-server.ts (Node's erasable TypeScript syntax support). */ -import { appendFileSync } from 'node:fs' +import { appendFileSync, closeSync } 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 @@ -30,6 +33,9 @@ const extraCaps: unknown = process.env.LSP_FAKE_CAPS !== undefined ? JSON.parse( const hang = process.env.LSP_FAKE_HANG === '1' 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 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' @@ -124,17 +130,29 @@ function handle(message: { id?: number; method?: string; params?: unknown; resul } if (method === 'textDocument/didOpen') { if (crashOnOpen) process.exit(1) + if (openMarker !== undefined) { + const params = message.params as { textDocument?: { text?: unknown } } | undefined + appendFileSync(openMarker, `${JSON.stringify(params?.textDocument?.text)}\n`) + } if (onOpen !== undefined) emitServerRequest(onOpen) return } if (method === 'textDocument/didClose' || method === 'initialized') return if (method?.startsWith('textDocument/')) { if (hang) return - if (errorReply) { send({ id, error: { code: -32000, message: 'server refused the request' } }); return } - send({ id, result: resultFor(method) }) - // Simulate an idle death: answer this request, then exit before the next one arrives so the pool - // is left holding a dead instance. - if (exitAfterReply) setTimeout(() => process.exit(0), 20) + const reply = (): void => { + if (closeStdinAfterReply) closeSync(0) + if (errorReply) { + send({ id, error: { code: -32000, message: 'server refused the request' } }) + } else { + send({ id, result: resultFor(method) }) + } + // Simulate an idle death: answer this request, then exit before the next one arrives so the + // pool is left holding a dead instance. + if (exitAfterReply) setTimeout(() => process.exit(0), 20) + } + if (replyDelayMs > 0) setTimeout(reply, replyDelayMs) + else reply() return } // Unknown request with an id: answer null so the client never stalls. @@ -172,3 +190,4 @@ function send(message: Record): void { // Keep the event loop alive. process.stdin.resume() +if (closeStdinAfterReply) setInterval(() => {}, 1000) diff --git a/packages/lsp/lsp-local/tests/host.spec.ts b/packages/lsp/lsp-local/tests/host.spec.ts index 3fb9f804ac..8629502fd1 100644 --- a/packages/lsp/lsp-local/tests/host.spec.ts +++ b/packages/lsp/lsp-local/tests/host.spec.ts @@ -3,8 +3,13 @@ import { mkdtemp, mkdir, rm, symlink, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { realpath } from 'node:fs/promises' +import { execFile } from 'node:child_process' +import { promisify } from 'node:util' +import { deadline } from '@deepseek-ai/dsh-timeout' import { canonicalizeWorkspace, readHostSource } from '@deepseek-ai/dsh-lsp-local' +const execFileAsync = promisify(execFile) + let root: string let ws: string @@ -87,6 +92,19 @@ describe('readHostSource', () => { await expect(readHostSource('dir', ws, BIG)).rejects.toThrow(/not a regular file/) }) + it('rejects a FIFO with no writer without blocking in open', async () => { + const fifo = join(ws, 'pipe.ts') + await execFileAsync('mkfifo', [fifo]) + using d = deadline(undefined, 1000, 'FIFO_READ_TIMEOUT') + await expect(readHostSource('pipe.ts', ws, BIG, d.signal)).rejects.toThrow(/not a regular file/) + }) + + it('honors a pre-aborted source read before filesystem work', async () => { + const controller = new AbortController() + controller.abort(new Error('source read cancelled')) + await expect(readHostSource('missing.ts', ws, BIG, controller.signal)).rejects.toThrow(/source read cancelled/) + }) + it('treats the workspace root itself as inside, then rejects it as non-regular', async () => { // filePath '.' canonicalizes to the workspace dir: isInside's identity branch is taken, and the // directory then fails the regular-file check. diff --git a/packages/lsp/lsp-local/tests/instance.spec.ts b/packages/lsp/lsp-local/tests/instance.spec.ts index f46d72571b..6019d870fa 100644 --- a/packages/lsp/lsp-local/tests/instance.spec.ts +++ b/packages/lsp/lsp-local/tests/instance.spec.ts @@ -7,9 +7,7 @@ import { LspInstance, readHostSource } from '@deepseek-ai/dsh-lsp-local' import type { InstanceSpec } from '@deepseek-ai/dsh-lsp-local/src/instance.ts' import type { LspProviderQuery, LspQueryResult } from '@deepseek-ai/dsh-lsp' -const tsxLoader = fileURLToPath(import.meta.resolve('tsx')) const fixtureServer = fileURLToPath(new URL('./fixture-server.ts', import.meta.url)) -const repoTsconfig = fileURLToPath(new URL('../../../../tsconfig.json', import.meta.url)) let root: string let ws: string @@ -31,9 +29,9 @@ afterEach(async () => { function makeInstance(env: Record = {}, overrides: Partial = {}): LspInstance { const instance = new LspInstance({ command: process.execPath, - args: ['--import', tsxLoader, fixtureServer], + args: [fixtureServer], cwd: ws, - env: { ...process.env as Record, TSX_TSCONFIG_PATH: repoTsconfig, ...env }, + env: { ...process.env as Record, ...env }, configuration: { setting: 42 }, initializationOptions: { init: true }, maxMessageBytes: 16_000_000, @@ -46,12 +44,12 @@ function makeInstance(env: Record = {}, overrides: Partial { +async function run(instance: LspInstance, operation: LspProviderQuery['operation'] = 'goToDefinition', signal?: AbortSignal): Promise { const source = await readHostSource('a.ts', ws, 4_000_000) return instance.query(query(operation), source, signal) } @@ -91,43 +89,43 @@ describe('LspInstance server-request handling', () => { const instance = makeInstance({ LSP_FAKE_ON_OPEN: 'configuration', LSP_FAKE_DEF: locJson() }) // The query drives didOpen, which makes the fake emit workspace/configuration; a healthy answer // keeps the query working. - await expect(run(instance, 'definition')).resolves.toMatchObject({ kind: 'locations' }) + await expect(run(instance, 'goToDefinition')).resolves.toMatchObject({ kind: 'locations' }) }) it('accepts a lifecycle client/registerCapability request', async () => { const instance = makeInstance({ LSP_FAKE_ON_OPEN: 'lifecycle', LSP_FAKE_DEF: 'null' }) - await expect(run(instance, 'definition')).resolves.toEqual({ kind: 'locations', locations: [], resolvedWorkspaceRoot: ws }) + await expect(run(instance, 'goToDefinition')).resolves.toEqual({ kind: 'locations', locations: [], resolvedWorkspaceRoot: ws }) }) it('rejects a workspace/applyEdit request but keeps serving', async () => { const instance = makeInstance({ LSP_FAKE_ON_OPEN: 'applyEdit', LSP_FAKE_DEF: 'null' }) - await expect(run(instance, 'definition')).resolves.toEqual({ kind: 'locations', locations: [], resolvedWorkspaceRoot: ws }) + await expect(run(instance, 'goToDefinition')).resolves.toEqual({ kind: 'locations', locations: [], resolvedWorkspaceRoot: ws }) }) it('rejects an unknown server request but keeps serving', async () => { const instance = makeInstance({ LSP_FAKE_ON_OPEN: 'unknown', LSP_FAKE_DEF: 'null' }) - await expect(run(instance, 'definition')).resolves.toEqual({ kind: 'locations', locations: [], resolvedWorkspaceRoot: ws }) + await expect(run(instance, 'goToDefinition')).resolves.toEqual({ kind: 'locations', locations: [], resolvedWorkspaceRoot: ws }) }) }) describe('LspInstance query and abort', () => { it('sends includeDeclaration for references', async () => { const instance = makeInstance({ LSP_FAKE_REFS: JSON.stringify([JSON.parse(locJson())]) }) - await expect(run(instance, 'references')).resolves.toMatchObject({ kind: 'locations' }) + await expect(run(instance, 'findReferences')).resolves.toMatchObject({ kind: 'locations' }) }) it('rejects a query aborted before it starts', async () => { const instance = makeInstance({ LSP_FAKE_DEF: 'null' }) const controller = new AbortController() controller.abort(new Error('pre-abort')) - await expect(run(instance, 'definition', controller.signal)).rejects.toThrow(/pre-abort/) + await expect(run(instance, 'goToDefinition', controller.signal)).rejects.toThrow(/pre-abort/) }) it('cancels an in-flight request on abort and rejects', async () => { const instance = makeInstance({ LSP_FAKE_HANG: '1' }) const controller = new AbortController() // Warm the instance first so the abort lands during the hanging request, not during startup. - const pending = run(instance, 'definition', controller.signal) + const pending = run(instance, 'goToDefinition', controller.signal) await new Promise(resolve => setTimeout(resolve, 300)) controller.abort(new Error('mid-flight')) await expect(pending).rejects.toThrow(/mid-flight/) @@ -138,7 +136,7 @@ describe('LspInstance query and abort', () => { // down (its process closed) rather than left with an active request. const instance = makeInstance({ LSP_FAKE_HANG: '1' }, { killGraceMs: 100 }) const controller = new AbortController() - const pending = run(instance, 'definition', controller.signal) + const pending = run(instance, 'goToDefinition', controller.signal) await new Promise(resolve => setTimeout(resolve, 300)) controller.abort(new Error('mid-flight')) await expect(pending).rejects.toThrow(/mid-flight/) @@ -159,7 +157,7 @@ describe('LspInstance query and abort', () => { + '}});' const instance = scriptInstance(script, { killGraceMs: 2_000 }) const controller = new AbortController() - const pending = run(instance, 'definition', controller.signal) + const pending = run(instance, 'goToDefinition', controller.signal) await new Promise(resolve => setTimeout(resolve, 300)) controller.abort(new Error('mid-flight')) await expect(pending).rejects.toThrow(/mid-flight/) @@ -173,7 +171,7 @@ describe('LspInstance query and abort', () => { // observed during that wait instead of hanging the tool-timeout signal. const instance = scriptInstance('setInterval(()=>{},1000)', { killGraceMs: 100 }) const controller = new AbortController() - const pending = run(instance, 'definition', controller.signal) + const pending = run(instance, 'goToDefinition', controller.signal) await new Promise(resolve => setTimeout(resolve, 150)) controller.abort(new Error('handshake-abort')) await expect(pending).rejects.toThrow(/handshake-abort/) @@ -182,7 +180,7 @@ describe('LspInstance query and abort', () => { 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, 'definition')).rejects.toThrow(/does not support definition/) + await expect(run(instance, 'goToDefinition')).rejects.toThrow(/does not support goToDefinition/) }) it('propagates a server error response even when a signal is supplied (not an abort)', async () => { @@ -190,7 +188,20 @@ describe('LspInstance query and abort', () => { // without treating it as an abort. const instance = makeInstance({ LSP_FAKE_ERROR: '1' }) const controller = new AbortController() - await expect(run(instance, 'definition', controller.signal)).rejects.toThrow(/server refused/) + await expect(run(instance, 'goToDefinition', controller.signal)).rejects.toThrow(/server refused/) + }) + + 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 }) + await expect(run(instance, 'goToDefinition')).resolves.toEqual({ + kind: 'locations', + locations: [], + resolvedWorkspaceRoot: ws, + }) + expect(instance.dead).toBe(true) }) }) @@ -202,28 +213,28 @@ describe('LspInstance disposal', () => { LSP_FAKE_EXIT_DELAY_MS: '75', LSP_FAKE_EXIT_MARKER: marker, }, { shutdownTimeoutMs: 500 }) - await run(instance, 'definition') + await run(instance, 'goToDefinition') await instance.dispose() expect(await readFile(marker, 'utf8')).toBe('EXIT\nCLEAN\n') }) it('is idempotent — a second dispose awaits close without error', async () => { const instance = makeInstance({ LSP_FAKE_DEF: 'null' }) - await run(instance, 'definition') + await run(instance, 'goToDefinition') await instance.dispose() await expect(instance.dispose()).resolves.toBeUndefined() }) it('rejects a query after disposal', async () => { const instance = makeInstance({ LSP_FAKE_DEF: 'null' }) - await run(instance, 'definition') + await run(instance, 'goToDefinition') await instance.dispose() - await expect(run(instance, 'definition')).rejects.toThrow(/disposed/) + await expect(run(instance, 'goToDefinition')).rejects.toThrow(expect.objectContaining({ code: 'LSP_DISPOSED' })) }) it('reports dead after the process closes', async () => { const instance = makeInstance({ LSP_FAKE_DEF: 'null' }) - await run(instance, 'definition') + await run(instance, 'goToDefinition') await instance.dispose() expect(instance.dead).toBe(true) }) @@ -232,7 +243,7 @@ describe('LspInstance disposal', () => { // Server answers initialize, ignores shutdown, and traps SIGTERM so only SIGKILL stops it. const script = RESPONDING_SERVER + 'process.on("SIGTERM",()=>{});' const instance = scriptInstance(script, { shutdownTimeoutMs: 100, killGraceMs: 100 }) - await run(instance, 'definition') + await run(instance, 'goToDefinition') await expect(instance.dispose()).resolves.toBeUndefined() }) @@ -244,7 +255,7 @@ describe('LspInstance disposal', () => { + `writeFileSync(${JSON.stringify(marker)},String(helper.pid));` + RESPONDING_SERVER const instance = scriptInstance(script, { shutdownTimeoutMs: 100, killGraceMs: 100 }) - await run(instance, 'definition') + await run(instance, 'goToDefinition') const helperPid = Number(await readFile(marker, 'utf8')) try { const first = instance.dispose() @@ -259,7 +270,7 @@ describe('LspInstance disposal', () => { it('carries a non-Error abort reason as a generic aborted error', async () => { const instance = makeInstance({ LSP_FAKE_HANG: '1' }) const controller = new AbortController() - const pending = run(instance, 'definition', controller.signal) + const pending = run(instance, 'goToDefinition', controller.signal) await new Promise(resolve => setTimeout(resolve, 200)) controller.abort('a string reason, not an Error') await expect(pending).rejects.toThrow(/aborted/) diff --git a/packages/lsp/lsp-local/tests/lifecycle.spec.ts b/packages/lsp/lsp-local/tests/lifecycle.spec.ts index 3d06b7576b..826905ed26 100644 --- a/packages/lsp/lsp-local/tests/lifecycle.spec.ts +++ b/packages/lsp/lsp-local/tests/lifecycle.spec.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest' -import { mkdtemp, mkdir, rm, writeFile } from 'node:fs/promises' +import { mkdtemp, mkdir, readFile, rm, writeFile } from 'node:fs/promises' import { realpath } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -10,9 +10,7 @@ import { deadline } from '@deepseek-ai/dsh-timeout' import * as LspLocal from '@deepseek-ai/dsh-lsp-local' import type { LspLocalServerConfig } from '@deepseek-ai/dsh-lsp-local' -const tsxLoader = fileURLToPath(import.meta.resolve('tsx')) const fixtureServer = fileURLToPath(new URL('./fixture-server.ts', import.meta.url)) -const repoTsconfig = fileURLToPath(new URL('../../../../tsconfig.json', import.meta.url)) let root: string let ws: string @@ -32,8 +30,8 @@ afterEach(async () => { function fakeServer(fakeEnv: Record = {}, overrides: Partial = {}): LspLocalServerConfig { return { command: process.execPath, - args: ['--import', tsxLoader, fixtureServer], - env: { TSX_TSCONFIG_PATH: repoTsconfig, ...fakeEnv }, + args: [fixtureServer], + env: { ...fakeEnv }, extensionToLanguage: { '.ts': 'typescript' }, ...overrides, } @@ -79,7 +77,7 @@ describe('lsp-local end to end over a fake server', () => { it('resolves definition to normalized locations', async () => { const ctx = await mount({ LSP_FAKE_DEF: JSON.stringify(locationJson(0)) }) - const result = await ctx.lsp.query(query('definition')) + const result = await ctx.lsp.query(query('goToDefinition')) expect(result).toEqual({ kind: 'locations', locations: [{ uri: pathToFileURL(join(ws, 'a.ts')).href, range: { start: { line: 0, character: 0 }, end: { line: 0, character: 3 } } }], @@ -91,14 +89,14 @@ describe('lsp-local end to end over a fake server', () => { it('maps a LocationLink for implementation', async () => { const link = { targetUri: pathToFileURL(join(ws, 'a.ts')).href, targetSelectionRange: { start: { line: 1, character: 0 }, end: { line: 1, character: 2 } } } const ctx = await mount({ LSP_FAKE_IMPL: JSON.stringify([link]) }) - const result = await ctx.lsp.query(query('implementation')) + const result = await ctx.lsp.query(query('goToImplementation')) expect(result).toMatchObject({ kind: 'locations', locations: [{ range: { start: { line: 1, character: 0 } } }] }) await ctx.fiber.dispose() }) it('returns references (server includes the declaration)', async () => { const ctx = await mount({ LSP_FAKE_REFS: JSON.stringify([locationJson(0), locationJson(1)]) }) - const result = await ctx.lsp.query(query('references')) + const result = await ctx.lsp.query(query('findReferences')) expect(result).toMatchObject({ kind: 'locations' }) if (result.kind !== 'locations') throw new Error('expected locations') expect(result.locations).toHaveLength(2) @@ -114,7 +112,7 @@ describe('lsp-local end to end over a fake server', () => { it('returns an empty locations result for a null definition', async () => { const ctx = await mount({ LSP_FAKE_DEF: 'null' }) - expect(await ctx.lsp.query(query('definition'))).toEqual({ kind: 'locations', locations: [], resolvedWorkspaceRoot: ws }) + expect(await ctx.lsp.query(query('goToDefinition'))).toEqual({ kind: 'locations', locations: [], resolvedWorkspaceRoot: ws }) await ctx.fiber.dispose() }) @@ -126,7 +124,7 @@ describe('lsp-local end to end over a fake server', () => { it('rejects a non-utf-16 position encoding at initialize', async () => { const ctx = await mount({ LSP_FAKE_ENCODING: 'utf-8', LSP_FAKE_DEF: 'null' }) - await expect(ctx.lsp.query(query('definition'))).rejects.toThrow(/unsupported position encoding/) + await expect(ctx.lsp.query(query('goToDefinition'))).rejects.toThrow(/unsupported position encoding/) await ctx.fiber.dispose() }) @@ -134,21 +132,21 @@ describe('lsp-local end to end over a fake server', () => { // A utf-8 server makes `initialize` reject; the instance must be torn down (not left with a // permanently-rejecting `ready`) so a later query starts a fresh process rather than reusing it. const ctx = await mount({ LSP_FAKE_ENCODING: 'utf-8', LSP_FAKE_DEF: 'null' }) - await expect(ctx.lsp.query(query('definition'))).rejects.toThrow(/unsupported position encoding/) + await expect(ctx.lsp.query(query('goToDefinition'))).rejects.toThrow(/unsupported position encoding/) // A second query must also fail the same way (fresh instance), and must NOT hang on a poisoned one. - await expect(ctx.lsp.query(query('definition'))).rejects.toThrow(/unsupported position encoding/) + await expect(ctx.lsp.query(query('goToDefinition'))).rejects.toThrow(/unsupported position encoding/) await ctx.fiber.dispose() }) it('rejects a server without transient-open sync (None)', async () => { const ctx = await mount({ LSP_FAKE_SYNC: '0', LSP_FAKE_DEF: 'null' }) - await expect(ctx.lsp.query(query('definition'))).rejects.toThrow(/transient textDocument\/didOpen/) + await expect(ctx.lsp.query(query('goToDefinition'))).rejects.toThrow(/transient textDocument\/didOpen/) await ctx.fiber.dispose() }) it('accepts openClose options sync', async () => { const ctx = await mount({ LSP_FAKE_SYNC: JSON.stringify({ openClose: true, change: 2 }), LSP_FAKE_DEF: 'null' }) - expect(await ctx.lsp.query(query('definition'))).toEqual({ kind: 'locations', locations: [], resolvedWorkspaceRoot: ws }) + expect(await ctx.lsp.query(query('goToDefinition'))).toEqual({ kind: 'locations', locations: [], resolvedWorkspaceRoot: ws }) await ctx.fiber.dispose() }) @@ -162,25 +160,44 @@ describe('lsp-local end to end over a fake server', () => { const outside = join(root, 'out.ts') await writeFile(outside, 'x') const ctx = await mount({ LSP_FAKE_DEF: 'null' }) - await expect(ctx.lsp.query({ ...query('definition'), filePath: outside })).rejects.toThrow(/outside the workspace/) + await expect(ctx.lsp.query({ ...query('goToDefinition'), filePath: outside })).rejects.toThrow(/outside the workspace/) await ctx.fiber.dispose() }) it('serializes queries through one instance and runs them in order', async () => { const ctx = await mount({ LSP_FAKE_DEF: JSON.stringify(locationJson(0)) }) const results = await Promise.all([ - ctx.lsp.query(query('definition')), - ctx.lsp.query(query('definition')), - ctx.lsp.query(query('definition')), + ctx.lsp.query(query('goToDefinition')), + ctx.lsp.query(query('goToDefinition')), + ctx.lsp.query(query('goToDefinition')), ]) for (const result of results) expect(result).toMatchObject({ kind: 'locations' }) await ctx.fiber.dispose() }) + it('reads a queued query source only when its lifecycle starts', async () => { + const marker = join(root, 'opened.jsonl') + const ctx = await mount({ + LSP_FAKE_DEF: 'null', + LSP_FAKE_REPLY_DELAY_MS: '300', + LSP_FAKE_OPEN_MARKER: marker, + }) + const first = ctx.lsp.query(query('goToDefinition')) + await waitFor(async () => (await markerLines(marker)).length === 1) + const second = ctx.lsp.query(query('goToDefinition')) + await writeFile(join(ws, 'a.ts'), 'const changed = 2\n') + await Promise.all([first, second]) + expect(await markerLines(marker)).toEqual([ + 'const x = 1\nconst y = x\n', + 'const changed = 2\n', + ]) + await ctx.fiber.dispose() + }) + it('aborts an in-flight query when the signal fires', async () => { const ctx = await mount({ LSP_FAKE_HANG: '1' }) const controller = new AbortController() - const pending = ctx.lsp.query(query('definition'), controller.signal) + const pending = ctx.lsp.query(query('goToDefinition'), controller.signal) controller.abort(new Error('caller cancelled')) await expect(pending).rejects.toThrow(/cancelled/) await ctx.fiber.dispose() @@ -190,7 +207,7 @@ describe('lsp-local end to end over a fake server', () => { const ctx = await mount({ LSP_FAKE_DEF: 'null' }) const controller = new AbortController() controller.abort(new Error('pre-aborted')) - await expect(ctx.lsp.query(query('definition'), controller.signal)).rejects.toThrow(/pre-aborted/) + await expect(ctx.lsp.query(query('goToDefinition'), controller.signal)).rejects.toThrow(/pre-aborted/) await ctx.fiber.dispose() }) @@ -201,22 +218,22 @@ describe('lsp-local end to end over a fake server', () => { command: process.execPath, args: ['-e', 'process.stderr.write("FATAL: boom\\n"); setTimeout(()=>process.exit(1), 50)'], }) - await expect(ctx.lsp.query(query('definition'))).rejects.toThrow(/FATAL: boom/) + await expect(ctx.lsp.query(query('goToDefinition'))).rejects.toThrow(/FATAL: boom/) await ctx.fiber.dispose() }) it('classifies a timeout deadline as the abort reason', async () => { const ctx = await mount({ LSP_FAKE_HANG: '1' }) using d = deadline(undefined, 50, 'TEST_TIMEOUT') - await expect(ctx.lsp.query(query('definition'), d.signal)).rejects.toThrow(/TEST_TIMEOUT/) + await expect(ctx.lsp.query(query('goToDefinition'), d.signal)).rejects.toThrow(/TEST_TIMEOUT/) await ctx.fiber.dispose() }) it('fails the active query when the server crashes on open, and replaces it next query', async () => { const ctx = await mount({ LSP_FAKE_CRASH_ON_OPEN: '1', LSP_FAKE_DEF: 'null' }, { shutdownTimeoutMs: 100, killGraceMs: 100 }) - await expect(ctx.lsp.query(query('definition'))).rejects.toThrow() + await expect(ctx.lsp.query(query('goToDefinition'))).rejects.toThrow() // A later query starts a fresh process; still crashes, but proves the slot was replaced (no hang). - await expect(ctx.lsp.query(query('definition'))).rejects.toThrow() + await expect(ctx.lsp.query(query('goToDefinition'))).rejects.toThrow() await ctx.fiber.dispose() }) @@ -225,10 +242,10 @@ describe('lsp-local end to end over a fake server', () => { // instance in the pool. The next query must evict-and-replace it and still succeed, rather than // failing once on the closed connection first. const ctx = await mount({ LSP_FAKE_EXIT_AFTER_REPLY: '1', LSP_FAKE_DEF: JSON.stringify(locationJson(0)) }) - expect(await ctx.lsp.query(query('definition'))).toMatchObject({ kind: 'locations' }) + expect(await ctx.lsp.query(query('goToDefinition'))).toMatchObject({ kind: 'locations' }) // Wait past the fixture's post-reply exit so the pooled instance is observably dead. await new Promise(resolve => setTimeout(resolve, 60)) - expect(await ctx.lsp.query(query('definition'))).toMatchObject({ kind: 'locations' }) + expect(await ctx.lsp.query(query('goToDefinition'))).toMatchObject({ kind: 'locations' }) await ctx.fiber.dispose() }) @@ -237,11 +254,11 @@ describe('lsp-local end to end over a fake server', () => { // are awaited, so the pre-spawn recheck must reject without ever creating a pooled instance. const ctx = await mount({ LSP_FAKE_DEF: 'null' }) const controller = new AbortController() - const pending = ctx.lsp.query(query('definition'), controller.signal) + const pending = ctx.lsp.query(query('goToDefinition'), controller.signal) controller.abort(new Error('mid-read cancel')) await expect(pending).rejects.toThrow(/mid-read cancel/) // A subsequent live query still works, proving no half-created instance poisoned the pool. - expect(await ctx.lsp.query(query('definition'))).toEqual({ kind: 'locations', locations: [], resolvedWorkspaceRoot: ws }) + expect(await ctx.lsp.query(query('goToDefinition'))).toEqual({ kind: 'locations', locations: [], resolvedWorkspaceRoot: ws }) await ctx.fiber.dispose() }) @@ -251,8 +268,8 @@ describe('lsp-local end to end over a fake server', () => { await writeFile(join(ws2, 'a.ts'), 'const z = 2\n') const ctx = await mount({ LSP_FAKE_DEF: JSON.stringify(locationJson(0)) }) const [r1, r2] = await Promise.all([ - ctx.lsp.query({ ...query('definition'), workspaceRoot: ws }), - ctx.lsp.query({ ...query('definition'), workspaceRoot: ws2 }), + ctx.lsp.query({ ...query('goToDefinition'), workspaceRoot: ws }), + ctx.lsp.query({ ...query('goToDefinition'), workspaceRoot: ws2 }), ]) expect(r1).toMatchObject({ kind: 'locations' }) expect(r2).toMatchObject({ kind: 'locations' }) @@ -261,7 +278,7 @@ describe('lsp-local end to end over a fake server', () => { it('disposes cleanly, terminating a server that ignores shutdown', async () => { const ctx = await mount({ LSP_FAKE_NO_SHUTDOWN: '1', LSP_FAKE_DEF: 'null' }, { killGraceMs: 100, shutdownTimeoutMs: 100 }) - await ctx.lsp.query(query('definition')) + await ctx.lsp.query(query('goToDefinition')) await expect(ctx.fiber.dispose()).resolves.toBeUndefined() }) @@ -280,3 +297,23 @@ describe('lsp-local end to end over a fake server', () => { await ctx.fiber.dispose() }) }) + +/** Read the fixture's JSON-lines didOpen marker, returning no entries before it exists. */ +async function markerLines(path: string): Promise { + try { + const text = await readFile(path, 'utf8') + return text.trim().split('\n').filter(Boolean).map(line => JSON.parse(line) as string) + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return [] + throw error + } +} + +/** Poll an asynchronous condition until it succeeds or the test-local deadline expires. */ +async function waitFor(condition: () => Promise, timeoutMs = 3000): Promise { + const started = Date.now() + while (!await condition()) { + if (Date.now() - started > timeoutMs) throw new Error('waitFor timed out') + await new Promise(resolve => setTimeout(resolve, 10)) + } +} diff --git a/packages/lsp/lsp-local/tests/provider.spec.ts b/packages/lsp/lsp-local/tests/provider.spec.ts index 65a22bfb5b..8746b7a903 100644 --- a/packages/lsp/lsp-local/tests/provider.spec.ts +++ b/packages/lsp/lsp-local/tests/provider.spec.ts @@ -6,6 +6,7 @@ import { Context } from 'cordis' import Lsp, { type LspQueryRequest } from '@deepseek-ai/dsh-lsp' import * as LspLocal from '@deepseek-ai/dsh-lsp-local' import type { Config, LspLocalServerConfig } from '@deepseek-ai/dsh-lsp-local' +import { MAX_TIMER_DELAY_MS } from '@deepseek-ai/dsh-timeout' let root: string let ws: string @@ -22,7 +23,7 @@ afterEach(async () => { }) function query(): LspQueryRequest { - return { operation: 'definition', filePath: 'a.ts', position: { line: 0, character: 0 }, workspaceRoot: ws } + return { operation: 'goToDefinition', filePath: 'a.ts', position: { line: 0, character: 0 }, workspaceRoot: ws } } /** Wrap one server entry in the plugin's named server table. */ @@ -91,6 +92,30 @@ describe('lsp-local provider resolution', () => { await ctx.fiber.dispose() }) + it('rejects a nonpositive byte cap at load', async () => { + const ctx = new Context() + await ctx.plugin(Lsp) + await expect(ctx.plugin(LspLocal, config('bad-cap', { + command: process.execPath, + args: ['-e', ''], + extensionToLanguage: { '.ts': 'typescript' }, + maxDocumentBytes: 0, + }))).rejects.toThrow(/servers\.bad-cap\.maxDocumentBytes must be a positive integer/) + await ctx.fiber.dispose() + }) + + it.each(['shutdownTimeoutMs', 'killGraceMs'] as const)('rejects %s above Node timer range at load', async (name) => { + const ctx = new Context() + await ctx.plugin(Lsp) + await expect(ctx.plugin(LspLocal, config('bad-timer', { + command: process.execPath, + args: ['-e', ''], + extensionToLanguage: { '.ts': 'typescript' }, + [name]: MAX_TIMER_DELAY_MS + 1, + }))).rejects.toThrow(new RegExp(`servers\\.bad-timer\\.${name}`)) + await ctx.fiber.dispose() + }) + it('rejects an absolute command that is not executable at load', async () => { const notExe = join(root, 'not-exe.txt') await writeFile(notExe, 'plain text, not executable') diff --git a/packages/lsp/lsp-local/tests/translate.spec.ts b/packages/lsp/lsp-local/tests/translate.spec.ts index 7727112089..a68afebdd5 100644 --- a/packages/lsp/lsp-local/tests/translate.spec.ts +++ b/packages/lsp/lsp-local/tests/translate.spec.ts @@ -13,9 +13,9 @@ const RANGE = { start: { line: 1, character: 2 }, end: { line: 1, character: 5 } describe('requestMethod', () => { it('maps each operation to its textDocument request', () => { - expect(requestMethod('definition')).toBe('textDocument/definition') - expect(requestMethod('references')).toBe('textDocument/references') - expect(requestMethod('implementation')).toBe('textDocument/implementation') + expect(requestMethod('goToDefinition')).toBe('textDocument/definition') + expect(requestMethod('findReferences')).toBe('textDocument/references') + expect(requestMethod('goToImplementation')).toBe('textDocument/implementation') expect(requestMethod('hover')).toBe('textDocument/hover') }) }) @@ -27,9 +27,9 @@ describe('supportsOperation', () => { referencesProvider: { workDoneProgress: true }, implementationProvider: false, } - expect(supportsOperation(caps, 'definition')).toBe(true) - expect(supportsOperation(caps, 'references')).toBe(true) - expect(supportsOperation(caps, 'implementation')).toBe(false) + expect(supportsOperation(caps, 'goToDefinition')).toBe(true) + expect(supportsOperation(caps, 'findReferences')).toBe(true) + expect(supportsOperation(caps, 'goToImplementation')).toBe(false) expect(supportsOperation(caps, 'hover')).toBe(false) }) }) @@ -66,9 +66,9 @@ describe('negotiatePositionEncoding', () => { }) describe('normalizeLocations', () => { - it('returns empty for null and undefined', () => { + it('returns empty only for the protocol no-result value null', () => { expect(normalizeLocations(null)).toEqual([]) - expect(normalizeLocations(undefined)).toEqual([]) + expect(() => normalizeLocations(undefined)).toThrow(expect.objectContaining({ code: 'LSP_MALFORMED_RESPONSE' })) }) it('maps a single Location', () => { @@ -100,6 +100,13 @@ describe('normalizeLocations', () => { it('rejects a Location whose range positions are malformed', () => { expect(() => normalizeLocations([{ uri: 'file:///a', range: { start: null, end: null } }])).toThrow(/neither a Location/) }) + + it('rejects negative and fractional position coordinates', () => { + expect(() => normalizeLocations([{ uri: 'file:///a', range: { start: { line: -1, character: 0 }, end: RANGE.end } }])) + .toThrow(expect.objectContaining({ code: 'LSP_MALFORMED_RESPONSE' })) + expect(() => normalizeLocations([{ uri: 'file:///a', range: { start: RANGE.start, end: { line: 1.5, character: 5 } } }])) + .toThrow(expect.objectContaining({ code: 'LSP_MALFORMED_RESPONSE' })) + }) }) describe('normalizeHover', () => { @@ -107,6 +114,10 @@ describe('normalizeHover', () => { expect(normalizeHover(null)).toBeNull() }) + it('rejects a missing hover result', () => { + expect(() => normalizeHover(undefined)).toThrow(expect.objectContaining({ code: 'LSP_MALFORMED_RESPONSE' })) + }) + it('reads MarkupContent value and keeps a range', () => { expect(normalizeHover({ contents: { kind: 'markdown', value: '# H' }, range: RANGE })) .toEqual({ contents: '# H', range: RANGE }) @@ -130,8 +141,9 @@ describe('normalizeHover', () => { expect(normalizeHover({ contents: { kind: 'plaintext', value: '' } })).toBeNull() }) - it('treats a MarkupContent with a non-string value as empty (null)', () => { - expect(normalizeHover({ contents: { kind: 'markdown', value: 42 } })).toBeNull() + it('rejects a MarkupContent with a non-string value', () => { + expect(() => normalizeHover({ contents: { kind: 'markdown', value: 42 } })) + .toThrow(expect.objectContaining({ code: 'LSP_MALFORMED_RESPONSE' })) }) it('rejects a non-object payload', () => { @@ -143,11 +155,19 @@ describe('normalizeHover', () => { expect(() => normalizeHover({ contents: 42 })).toThrow(/were not MarkupContent/) }) + it('rejects a malformed MarkedString array member', () => { + expect(() => normalizeHover({ contents: ['ok', { language: 'ts', value: 42 }] })) + .toThrow(expect.objectContaining({ code: 'LSP_MALFORMED_RESPONSE' })) + expect(() => normalizeHover({ contents: [null] })) + .toThrow(expect.objectContaining({ code: 'LSP_MALFORMED_RESPONSE' })) + }) + it('rejects a hover with no contents field', () => { expect(() => normalizeHover({ range: RANGE })).toThrow(/no contents/) }) - it('ignores a malformed range and keeps the contents', () => { - expect(normalizeHover({ contents: 'x', range: { start: { line: 1 } } })).toEqual({ contents: 'x' }) + it('rejects a malformed range instead of silently dropping it', () => { + expect(() => normalizeHover({ contents: 'x', range: { start: { line: 1 } } })) + .toThrow(expect.objectContaining({ code: 'LSP_MALFORMED_RESPONSE' })) }) }) diff --git a/packages/lsp/lsp-local/tests/typescript-server.e2e.ts b/packages/lsp/lsp-local/tests/typescript-server.e2e.ts index 300ce7d318..8ba61c0717 100644 --- a/packages/lsp/lsp-local/tests/typescript-server.e2e.ts +++ b/packages/lsp/lsp-local/tests/typescript-server.e2e.ts @@ -81,7 +81,7 @@ function locations(result: LspQueryResult): readonly { uri: string }[] { describe('real typescript-language-server', () => { it('resolves the definition of a call site to its declaration', async () => { // `export const text = describe(c)` (line 15): `describe` begins at column 21. - const result = await ctx.lsp.query(at('definition', 15, 22)) + const result = await ctx.lsp.query(at('goToDefinition', 15, 22)) const locs = locations(result) expect(locs.length).toBeGreaterThanOrEqual(1) expect(locs.some(l => l.uri.endsWith('shapes.ts'))).toBe(true) @@ -89,7 +89,7 @@ describe('real typescript-language-server', () => { it('finds references to a symbol including its declaration', async () => { // References to `describe` from its declaration (line 10, col 17). - const result = await ctx.lsp.query(at('references', 10, 17)) + const result = await ctx.lsp.query(at('findReferences', 10, 17)) const locs = locations(result) // At least the declaration plus the call site. expect(locs.length).toBeGreaterThanOrEqual(2) @@ -97,7 +97,7 @@ describe('real typescript-language-server', () => { it('resolves implementations of an interface', async () => { // Implementations of `Shape` (line 1, col 18) → Circle. - const result = await ctx.lsp.query(at('implementation', 1, 18)) + const result = await ctx.lsp.query(at('goToImplementation', 1, 18)) const locs = locations(result) expect(locs.length).toBeGreaterThanOrEqual(1) }, 60_000) diff --git a/packages/lsp/lsp/README.md b/packages/lsp/lsp/README.md index 2f7fc6ff9f..df9ced7dc3 100644 --- a/packages/lsp/lsp/README.md +++ b/packages/lsp/lsp/README.md @@ -10,7 +10,7 @@ This package is the interface third of the LSP capability: | `@deepseek-ai/dsh-lsp-local` | a generic local backend that registers configured stdio language-server providers | | `@deepseek-ai/dsh-tool-lsp` | the model-facing `lsp` tool over `ctx.lsp` | -The seam exposes exactly four semantic operations — `definition`, `references`, `implementation`, `hover` — and no generic JSON-RPC escape hatch, so no protocol payload or unreviewed command/mutation reaches a provider through `ctx.lsp`. +The seam exposes exactly four semantic operations — `goToDefinition`, `findReferences`, `goToImplementation`, `hover` — and no generic JSON-RPC escape hatch, so no protocol payload or unreviewed command/mutation reaches a provider through `ctx.lsp`. ## Service API (`ctx.lsp`) @@ -25,7 +25,7 @@ Providers register **capabilities**, not tools. `dsh-tool-lsp` is the only owner ## Vocabulary -`LspQueryRequest` (`operation`, `filePath`, `position`, `workspaceRoot`) — every field required, so no field needs implementation defaulting and there is no `resolve()` step. Positions and ranges are zero-based UTF-16, matching the protocol; the tool owns the one-based cursor convention. `references` always includes declarations — providers enforce this internally, so callers get no flag. `LspQueryResult` is a CLOSED discriminated union: `{ kind: 'locations'; locations; resolvedWorkspaceRoot }` for navigation, `{ kind: 'hover'; hover }` for hover (content or `null`) — consumers `switch` to exhaustiveness so a new arm breaks compilation until handled. `resolvedWorkspaceRoot` is the provider's canonical form of the request's `workspaceRoot` and the root its `file:` URIs are relative to; a caller relativizing display paths uses it, not the (possibly symlinked) request root. See `src/types.ts` for the full contracts and `src/index.ts` for the `LspError` codes. +`LspQueryRequest` (`operation`, `filePath`, `position`, `workspaceRoot`) — every field required, so no field needs implementation defaulting and there is no `resolve()` step. Positions and ranges are zero-based UTF-16, matching the protocol; the tool owns the one-based cursor convention. `findReferences` always includes declarations — providers enforce this internally, so callers get no flag. `LspQueryResult` is a CLOSED discriminated union: `{ kind: 'locations'; locations; resolvedWorkspaceRoot }` for navigation, `{ kind: 'hover'; hover }` for hover (content or `null`) — consumers `switch` to exhaustiveness so a new arm breaks compilation until handled. `resolvedWorkspaceRoot` is the provider's canonical form of the request's `workspaceRoot` and the root its `file:` URIs are relative to; a caller relativizing display paths uses it, not the (possibly symlinked) request root. See `src/types.ts` for the full contracts and `src/index.ts` for the `LspError` codes, including `LSP_DISPOSED` and `LSP_MALFORMED_RESPONSE`. ## Model Experience diff --git a/packages/lsp/lsp/src/index.ts b/packages/lsp/lsp/src/index.ts index e00005acde..d7b1a80d01 100644 --- a/packages/lsp/lsp/src/index.ts +++ b/packages/lsp/lsp/src/index.ts @@ -1,6 +1,7 @@ /** * The LSP capability seam (`ctx.lsp`): a language-server provider registry and per-query, - * order-independent selection over normalized definition/references/implementation/hover queries. + * order-independent selection over normalized goToDefinition/findReferences/goToImplementation/ + * hover queries. * * A provider reserves a branded id and an exclusive set of file extensions atomically: * {@link Lsp.registerProvider} validates and conflict-checks everything before mutating, so an @@ -42,8 +43,9 @@ declare module 'cordis' { /** * Structured LSP failure. Extends {@link HarnessError} with a stable `code` - * (`LSP_INVALID_PROVIDER`, `LSP_CONFLICT`, `LSP_UNAVAILABLE`, `LSP_UNSUPPORTED_OPERATION`, …) that - * callers route on instead of parsing `message`. + * (`LSP_INVALID_PROVIDER`, `LSP_CONFLICT`, `LSP_UNAVAILABLE`, `LSP_DISPOSED`, + * `LSP_UNSUPPORTED_OPERATION`, `LSP_MALFORMED_RESPONSE`, …) that callers route on instead of + * parsing `message`. */ export class LspError extends HarnessError {} diff --git a/packages/lsp/lsp/src/types.ts b/packages/lsp/lsp/src/types.ts index d1ae71dccd..d0c84f606d 100644 --- a/packages/lsp/lsp/src/types.ts +++ b/packages/lsp/lsp/src/types.ts @@ -14,7 +14,7 @@ import type { LspProviderId } from './brand.ts' * compile-enforced change across the seam, providers, and the tool. Symbols and call hierarchy are * deliberately deferred (they need different schemas). */ -export type LspOperation = 'definition' | 'references' | 'implementation' | 'hover' +export type LspOperation = 'goToDefinition' | 'findReferences' | 'goToImplementation' | 'hover' /** A zero-based UTF-16 cursor coordinate, matching the LSP wire convention. */ export interface LspPosition { @@ -73,9 +73,9 @@ export interface LspHover { } /** - * The closed result union. Navigation operations (`definition`, `references`, `implementation`) - * normalize to `locations`; `hover` normalizes to content or `null`. Consumers `switch` on `kind` - * to exhaustiveness so a new arm breaks compilation until handled. + * The closed result union. Navigation operations (`goToDefinition`, `findReferences`, + * `goToImplementation`) normalize to `locations`; `hover` normalizes to content or `null`. + * Consumers `switch` on `kind` to exhaustiveness so a new arm breaks compilation until handled. * * The `locations` variant carries `resolvedWorkspaceRoot`: the provider's canonical form of the * request's `workspaceRoot`, and the root its `file:` location URIs are relative to. A caller that @@ -88,8 +88,9 @@ export type LspQueryResult = /** * A language-server backend registered on `ctx.lsp`. Each provider owns a stable {@link - * LspProviderId} and an extension-to-language-id map (lowercase, leading-dot keys). `references` - * always includes declarations — the provider enforces this internally; callers get no flag. + * LspProviderId} and an extension-to-language-id map (lowercase, leading-dot keys). + * `findReferences` always includes declarations — the provider enforces this internally; callers + * get no flag. */ export interface LspProvider { /** Stable provider identity, reserved atomically with the extension mappings. */ diff --git a/packages/lsp/lsp/tests/lsp.spec.ts b/packages/lsp/lsp/tests/lsp.spec.ts index 8561891df1..77ea9687c7 100644 --- a/packages/lsp/lsp/tests/lsp.spec.ts +++ b/packages/lsp/lsp/tests/lsp.spec.ts @@ -39,7 +39,7 @@ async function mountLsp(): Promise<{ ctx: Context; lsp: Lsp }> { const hover: LspQueryResult = { kind: 'hover', hover: { contents: 'x' } } -function query(filePath: string, operation: LspProviderQuery['operation'] = 'definition'): Parameters[0] { +function query(filePath: string, operation: LspProviderQuery['operation'] = 'goToDefinition'): Parameters[0] { return { operation, filePath, position: { line: 0, character: 0 }, workspaceRoot: '/ws' } } diff --git a/packages/lsp/tool-lsp/README.md b/packages/lsp/tool-lsp/README.md index a3ffb9d871..4a844d2537 100644 --- a/packages/lsp/tool-lsp/README.md +++ b/packages/lsp/tool-lsp/README.md @@ -6,7 +6,7 @@ Namespace plugin (`name` / `inject` / `Config` / `apply`, no default export). In ## The tool -`lsp` accepts `operation` (`definition` | `references` | `implementation` | `hover`), `file_path`, `line`, and `character`. `line` and `character` are positive, one-based UTF-16 cursor coordinates; the tool converts them to the seam's zero-based positions and converts rendered locations back. `references` includes declarations so impact analysis does not omit the defining site. Provider, language id, workspace root, limits, timeout, initialization, and executable stay outside model input. +`lsp` accepts `operation` (`goToDefinition` | `findReferences` | `goToImplementation` | `hover`), `file_path`, `line`, and `character`. `line` and `character` are positive, one-based UTF-16 cursor coordinates; the tool converts them to the seam's zero-based positions and converts rendered locations back. `findReferences` includes declarations so impact analysis does not omit the defining site. Provider, language id, workspace root, limits, timeout, initialization, and executable stay outside model input. The tool requires the workspace root from the session `header.cwd`, with no fallback: absence fails as `LSP_WORKSPACE_REQUIRED` before querying. Locations render as stable, file-grouped `path:line:character` entries relativized against the result's `resolvedWorkspaceRoot` (the provider's canonical root), not the session cwd — so a symlinked cwd still renders in-workspace results as workspace-relative paths; a `file:` URI becomes a workspace-relative path (inside) or absolute path (outside), and any other URI stays verbatim. Empty locations and `null` hover are successful no-result responses; malformed provider payloads remain structured errors. @@ -15,7 +15,7 @@ The tool requires the workspace root from the session `header.cwd`, with no fall | Key | Default | Meaning | |---|---|---| | `maxLocations` | `100` | Largest number of rendered locations before an omission marker. | -| `maxHoverChars` | `16000` | Largest hover length in characters, applied after normalization. | +| `maxResultChars` | `16000` | Largest complete rendered result, including truncation metadata. | | `timeoutMs` | `60000` | Tool-call timeout budget, enforced by `dsh-timeout-policy`; covers the complete queued open/query/close lifecycle and is not model-configurable. | ## Model Experience @@ -29,7 +29,7 @@ One system-prompt section (order 112) positions LSP as a precision aid with the ##### Verbatim guidance ```markdown -Use search/read for ordinary navigation. Use lsp when textual matches are ambiguous or before a change requires precise definitions, implementations, or references. Positions are one-based line and character (UTF-16) at the cursor; an off-symbol position may return no results. references always includes the declaration. +Use search/read for ordinary navigation. Use lsp when textual matches are ambiguous or before a change requires precise definitions, implementations, or references. Positions are one-based line and character (UTF-16) at the cursor; an off-symbol position may return no results. findReferences always includes the declaration. ``` #### Token effect @@ -58,11 +58,11 @@ Prefix-stable while the visible tool definition and order are unchanged; registr #### What the model sees -File-grouped `path:line:character` location lines or normalized hover text, capped by `maxLocations` / `maxHoverChars` with an omission marker when truncated, and distinct `No results.` / `No hover information.` lines for empty results. +File-grouped `path:line:character` location lines or normalized hover text, capped first by `maxLocations` and then by `maxResultChars`; omission and truncation markers are included inside the complete character cap. Empty results use distinct `No results.` / `No hover information.` lines. #### Token effect -Capped per tool result by the two limits above. +Capped per tool result by `maxResultChars`, with `maxLocations` additionally bounding navigation item count. #### KV Cache effect diff --git a/packages/lsp/tool-lsp/package.json b/packages/lsp/tool-lsp/package.json index 4559bbd1b5..abedad1e53 100644 --- a/packages/lsp/tool-lsp/package.json +++ b/packages/lsp/tool-lsp/package.json @@ -1,6 +1,6 @@ { "name": "@deepseek-ai/dsh-tool-lsp", - "description": "Model-facing lsp tool over the DeepSeek Harness LSP capability seam (ctx.lsp) — one read-only tool with definition/references/implementation/hover operations, one-based UTF-16 cursor coordinates, workspace-grouped location rendering, and hover normalization", + "description": "Model-facing lsp tool over the DeepSeek Harness LSP capability seam (ctx.lsp) — one read-only tool with goToDefinition/findReferences/goToImplementation/hover operations, one-based UTF-16 cursor coordinates, bounded location rendering, and hover normalization", "version": "0.0.1", "private": true, "type": "module", @@ -25,6 +25,7 @@ "@deepseek-ai/dsh-llm": "^0.0.1", "@deepseek-ai/dsh-lsp": "^0.0.1", "@deepseek-ai/dsh-system-prompt": "^0.0.1", + "@deepseek-ai/dsh-timeout": "^0.0.1", "@deepseek-ai/dsh-tools": "^0.0.1", "cordis": "^4.0.0-rc.7" }, @@ -38,6 +39,7 @@ "@deepseek-ai/dsh-lsp-local": "workspace:^", "@deepseek-ai/dsh-session": "workspace:^", "@deepseek-ai/dsh-system-prompt": "workspace:^", + "@deepseek-ai/dsh-timeout": "workspace:^", "@deepseek-ai/dsh-timeout-policy": "workspace:^", "@deepseek-ai/dsh-tools": "workspace:^", "cordis": "^4.0.0-rc.7" diff --git a/packages/lsp/tool-lsp/src/index.ts b/packages/lsp/tool-lsp/src/index.ts index 3a37b7b6ed..c298bdffb8 100644 --- a/packages/lsp/tool-lsp/src/index.ts +++ b/packages/lsp/tool-lsp/src/index.ts @@ -1,9 +1,10 @@ /** * Model-facing `lsp` tool over `ctx.lsp`. One read-only tool with four operations - * (`definition`/`references`/`implementation`/`hover`); it converts one-based UTF-16 cursor - * coordinates to the seam's zero-based positions, requires the session workspace with no fallback, - * caps and renders results, and attaches a configurable timeout budget for `dsh-timeout-policy` to - * enforce. It runtime-injects only `tools`, `lsp`, and `systemPrompt` and imports no provider. + * (`goToDefinition`/`findReferences`/`goToImplementation`/`hover`); it converts one-based UTF-16 + * cursor coordinates to the seam's zero-based positions, requires the session workspace with no + * fallback, caps and renders results, and attaches a configurable timeout budget for + * `dsh-timeout-policy` to enforce. It runtime-injects only `tools`, `lsp`, and `systemPrompt` and + * imports no provider. * * Namespace plugin (named exports, no default export). * @module @deepseek-ai/dsh-tool-lsp @@ -12,13 +13,14 @@ import type { Context } from 'cordis' import z from 'schemastery' import { defineTool } from '@deepseek-ai/dsh-tools' -import type { ContentBlock } from '@deepseek-ai/dsh-llm' +import { assertNever, type ContentBlock } from '@deepseek-ai/dsh-llm' import { LspError } from '@deepseek-ai/dsh-lsp' import type {} from '@deepseek-ai/dsh-lsp' import type {} from '@deepseek-ai/dsh-system-prompt' +import { MAX_TIMER_DELAY_MS } from '@deepseek-ai/dsh-timeout' import { - DEFAULT_MAX_HOVER_CHARS, DEFAULT_MAX_LOCATIONS, + DEFAULT_MAX_RESULT_CHARS, formatHover, formatLocations, LSP_OPERATIONS, @@ -28,8 +30,8 @@ import { import { sessionCwd } from './session-cwd.ts' export { - DEFAULT_MAX_HOVER_CHARS, DEFAULT_MAX_LOCATIONS, + DEFAULT_MAX_RESULT_CHARS, formatHover, formatLocations, LSP_OPERATIONS, @@ -50,22 +52,22 @@ export const DEFAULT_LSP_TOOL_TIMEOUT_MS = 60_000 /** The stable system-prompt guidance positioning LSP as a precision aid. */ export const LSP_PROMPT_TEXT = - 'Use search/read for ordinary navigation. Use lsp when textual matches are ambiguous or before a change requires precise definitions, implementations, or references. Positions are one-based line and character (UTF-16) at the cursor; an off-symbol position may return no results. references always includes the declaration.' + 'Use search/read for ordinary navigation. Use lsp when textual matches are ambiguous or before a change requires precise definitions, implementations, or references. Positions are one-based line and character (UTF-16) at the cursor; an off-symbol position may return no results. findReferences always includes the declaration.' /** Plugin configuration: result caps and the timeout budget. */ export interface Config { /** Largest number of rendered locations before an omission marker (default 100). */ maxLocations?: number - /** Largest hover length in characters after normalization (default 16000). */ - maxHoverChars?: number + /** Largest complete rendered result in characters, including truncation metadata (default 16000). */ + maxResultChars?: number /** Tool-call timeout budget in ms (default 60000). */ timeoutMs?: number } export const Config: z = z.object({ maxLocations: z.number().default(DEFAULT_MAX_LOCATIONS), - maxHoverChars: z.number().default(DEFAULT_MAX_HOVER_CHARS), - timeoutMs: z.number().default(DEFAULT_LSP_TOOL_TIMEOUT_MS), + maxResultChars: z.number().default(DEFAULT_MAX_RESULT_CHARS), + timeoutMs: z.number().max(MAX_TIMER_DELAY_MS).default(DEFAULT_LSP_TOOL_TIMEOUT_MS), }) type ResolvedConfig = Required @@ -78,21 +80,21 @@ type ResolvedConfig = Required export function apply(ctx: Context, config: Config): void { const resolved = config as ResolvedConfig assertPositiveInteger('maxLocations', resolved.maxLocations) - assertPositiveInteger('maxHoverChars', resolved.maxHoverChars) - assertPositiveInteger('timeoutMs', resolved.timeoutMs) + assertPositiveInteger('maxResultChars', resolved.maxResultChars) + assertTimer('timeoutMs', resolved.timeoutMs) ctx.systemPrompt.section({ name: 'tool:lsp', order: 112, text: LSP_PROMPT_TEXT }) ctx.tools.register(defineTool({ name: 'lsp', description: - 'Query a language server for precise code navigation. operation is one of definition, references, implementation, hover. line and character are one-based UTF-16 cursor coordinates. references includes the declaration.', + 'Query a language server for precise code navigation. operation is one of goToDefinition, findReferences, goToImplementation, hover. line and character are one-based UTF-16 cursor coordinates. findReferences includes the declaration.', parameters: { operation: { type: 'string', required: true, enum: [...LSP_OPERATIONS], - description: 'definition, references, implementation, or hover.', + description: 'goToDefinition, findReferences, goToImplementation, or hover.', }, file_path: { type: 'string', required: true, description: 'The source file to query, relative to the workspace or absolute.' }, line: { type: 'number', required: true, description: 'One-based line of the cursor.' }, @@ -116,9 +118,12 @@ export function apply(ctx: Context, config: Config): void { // Relativize against the provider's canonical workspace root (which its file: URIs are // relative to), not the session cwd: a symlinked cwd would otherwise misclassify every // in-workspace location as external and render it as an absolute path. - return [{ type: 'text', text: formatLocations(result.locations, result.resolvedWorkspaceRoot, resolved.maxLocations) }] + return [{ type: 'text', text: formatLocations(result.locations, result.resolvedWorkspaceRoot, resolved.maxLocations, resolved.maxResultChars) }] case 'hover': - return [{ type: 'text', text: formatHover(result.hover, resolved.maxHoverChars) }] + return [{ type: 'text', text: formatHover(result.hover, resolved.maxResultChars) }] + /* v8 ignore next -- exhaustive over the closed LspQueryResult union; unreachable. */ + default: + return assertNever(result, 'tool-lsp result') } }, presentCall: presentLspCall, @@ -131,3 +136,10 @@ function assertPositiveInteger(name: string, value: number): void { throw new Error(`tool-lsp: ${name} must be a positive integer`) } } + +/** Reject a timer value Node would clamp instead of scheduling as configured. */ +function assertTimer(name: string, value: number): void { + if (!Number.isInteger(value) || value < 1 || value > MAX_TIMER_DELAY_MS) { + throw new Error(`tool-lsp: ${name} must be a positive integer no greater than ${MAX_TIMER_DELAY_MS}`) + } +} diff --git a/packages/lsp/tool-lsp/src/render.ts b/packages/lsp/tool-lsp/src/render.ts index 0051adc719..b6341ae407 100644 --- a/packages/lsp/tool-lsp/src/render.ts +++ b/packages/lsp/tool-lsp/src/render.ts @@ -1,8 +1,8 @@ /** * Pure formatting and coordinate conversion for the `lsp` tool: one-based↔zero-based UTF-16 cursor - * conversion, workspace-grouped location rendering with `file:`-URI resolution, hover capping, and - * ACP presentation. No I/O — a UI may call the presenter on live streaming and on replay, so it - * depends only on the tool arguments. + * conversion, workspace-grouped location rendering with `file:`-URI resolution, complete-result + * capping, and ACP presentation. No I/O — a UI may call the presenter on live streaming and on + * replay, so it depends only on the tool arguments. * @module @deepseek-ai/dsh-tool-lsp/render */ @@ -12,13 +12,13 @@ import type { GenericCallView } from '@deepseek-ai/dsh-tools' import type { LspHover, LspLocation, LspOperation, LspPosition } from '@deepseek-ai/dsh-lsp' /** The four operations the tool exposes, as a runtime tuple for schema enum + validation. */ -export const LSP_OPERATIONS: readonly LspOperation[] = ['definition', 'references', 'implementation', 'hover'] +export const LSP_OPERATIONS: readonly LspOperation[] = ['goToDefinition', 'findReferences', 'goToImplementation', 'hover'] /** Default cap on rendered locations before an omission marker is appended. */ export const DEFAULT_MAX_LOCATIONS = 100 -/** Default cap on hover characters (applied after normalization) before truncation is marked. */ -export const DEFAULT_MAX_HOVER_CHARS = 16_000 +/** Default cap on the complete rendered tool result, including truncation metadata. */ +export const DEFAULT_MAX_RESULT_CHARS = 16_000 /** Validated `lsp` arguments after coordinate checks. */ export interface LspToolInput { @@ -75,18 +75,20 @@ function oneBased(value: number, name: string): number { * Render a locations result grouped by file, converting each zero-based location back to a one-based * `path:line:character` entry. A `file:` URI inside the workspace becomes a workspace-relative path; * outside it, an absolute path; a non-`file:` URI is kept verbatim. Applies `maxLocations` and - * appends an omission marker when it truncates. + * appends an omission marker when it truncates by count, then applies the complete result cap. * @param locations - the seam's locations (possibly empty). * @param workspaceRoot - the canonical workspace root for relativizing `file:` paths. * @param maxLocations - the cap before truncation. + * @param maxResultChars - the complete rendered-text cap, including truncation metadata. * @returns the rendered text; a distinct no-result line when there are none. */ export function formatLocations( locations: readonly LspLocation[], workspaceRoot: string, maxLocations: number, + maxResultChars: number, ): string { - if (locations.length === 0) return 'No results.' + if (locations.length === 0) return boundResult('No results.', maxResultChars, 'locations') const shown = locations.slice(0, maxLocations) const omitted = locations.length - shown.length const grouped = new Map() @@ -103,20 +105,26 @@ export function formatLocations( if (omitted > 0) { lines.push(`… ${omitted} more location${omitted === 1 ? '' : 's'} omitted (limit ${maxLocations}).`) } - return lines.join('\n') + return boundResult(lines.join('\n'), maxResultChars, 'locations') } /** - * Render a hover result, applying `maxHoverChars` last and marking truncation. + * Render a hover result, applying `maxResultChars` last and keeping its marker within the cap. * @param hover - the normalized hover, or `null` for no hover. - * @param maxHoverChars - the cap applied after normalization. + * @param maxResultChars - the complete rendered-text cap, including truncation metadata. * @returns the rendered hover text; a distinct no-result line for `null`. */ -export function formatHover(hover: LspHover | null, maxHoverChars: number): string { - if (hover === null) return 'No hover information.' - const contents = hover.contents - if (contents.length <= maxHoverChars) return contents - return `${contents.slice(0, maxHoverChars)}\n… hover truncated (limit ${maxHoverChars} characters).` +export function formatHover(hover: LspHover | null, maxResultChars: number): string { + const text = hover === null ? 'No hover information.' : hover.contents + return boundResult(text, maxResultChars, 'hover') +} + +/** Bound a complete rendered result, including the truncation notice itself. */ +function boundResult(text: string, maxChars: number, label: string): string { + if (text.length <= maxChars) return text + const notice = `\n… ${label} truncated (limit ${maxChars} characters).` + if (notice.length >= maxChars) return notice.slice(0, maxChars) + return `${text.slice(0, maxChars - notice.length)}${notice}` } /** diff --git a/packages/lsp/tool-lsp/tests/integration.spec.ts b/packages/lsp/tool-lsp/tests/integration.spec.ts index 5c98d9fff0..8b4cb79de6 100644 --- a/packages/lsp/tool-lsp/tests/integration.spec.ts +++ b/packages/lsp/tool-lsp/tests/integration.spec.ts @@ -78,7 +78,7 @@ function call(ctx: Context, args: unknown) { describe('tool-lsp integration', () => { it('round-trips a definition query through the real provider and renders a location', async () => { const ctx = await mount(false) - const result = await call(ctx, { operation: 'definition', file_path: 'a.ts', line: 1, character: 7 }) + const result = await call(ctx, { operation: 'goToDefinition', file_path: 'a.ts', line: 1, character: 7 }) expect(result.isError).toBe(false) expect(result.content[0]).toEqual({ type: 'text', text: 'a.ts:1:1' }) await ctx.fiber.dispose() @@ -86,7 +86,7 @@ describe('tool-lsp integration', () => { it('enforces the TOOL_TIMEOUT budget when the server hangs', async () => { const ctx = await mount(true, 300) - const result = await call(ctx, { operation: 'definition', file_path: 'a.ts', line: 1, character: 7 }) + const result = await call(ctx, { operation: 'goToDefinition', file_path: 'a.ts', line: 1, character: 7 }) expect(result.isError).toBe(true) expect(result.error?.code).toBe('TOOL_TIMEOUT') await ctx.fiber.dispose() diff --git a/packages/lsp/tool-lsp/tests/render.spec.ts b/packages/lsp/tool-lsp/tests/render.spec.ts index 88b02a1fd5..a277539879 100644 --- a/packages/lsp/tool-lsp/tests/render.spec.ts +++ b/packages/lsp/tool-lsp/tests/render.spec.ts @@ -2,8 +2,8 @@ import { describe, expect, it } from 'vitest' import { pathToFileURL } from 'node:url' import { join } from 'node:path' import { - DEFAULT_MAX_HOVER_CHARS, DEFAULT_MAX_LOCATIONS, + DEFAULT_MAX_RESULT_CHARS, formatHover, formatLocations, LSP_OPERATIONS, @@ -79,52 +79,63 @@ describe('renderUri', () => { describe('formatLocations', () => { it('renders a no-result line for an empty list', () => { - expect(formatLocations([], WS, DEFAULT_MAX_LOCATIONS)).toBe('No results.') + expect(formatLocations([], WS, DEFAULT_MAX_LOCATIONS, DEFAULT_MAX_RESULT_CHARS)).toBe('No results.') }) it('renders one-based path:line:character grouped by file', () => { const a = pathToFileURL(join(WS, 'a.ts')).href - const text = formatLocations([loc(a, 0, 0), loc(a, 4, 2)], WS, DEFAULT_MAX_LOCATIONS) + const text = formatLocations([loc(a, 0, 0), loc(a, 4, 2)], WS, DEFAULT_MAX_LOCATIONS, DEFAULT_MAX_RESULT_CHARS) expect(text).toBe('a.ts:1:1\na.ts:5:3') }) it('caps at maxLocations and marks the omission', () => { const a = pathToFileURL(join(WS, 'a.ts')).href const many = Array.from({ length: 5 }, (_, i) => loc(a, i)) - const text = formatLocations(many, WS, 2) + const text = formatLocations(many, WS, 2, DEFAULT_MAX_RESULT_CHARS) expect(text).toContain('a.ts:1:1') expect(text).toContain('3 more locations omitted (limit 2).') }) it('uses the singular omission marker for exactly one extra', () => { const a = pathToFileURL(join(WS, 'a.ts')).href - const text = formatLocations([loc(a, 0), loc(a, 1)], WS, 1) + const text = formatLocations([loc(a, 0), loc(a, 1)], WS, 1, DEFAULT_MAX_RESULT_CHARS) expect(text).toContain('1 more location omitted (limit 1).') }) + + it('caps the complete location text even when one URI is enormous', () => { + const maxResultChars = 80 + const text = formatLocations([loc(`custom:${'x'.repeat(1_000_000)}`, 0)], WS, 1, maxResultChars) + expect(text).toHaveLength(maxResultChars) + expect(text).toContain('locations truncated') + }) }) describe('formatHover', () => { it('renders a no-result line for null', () => { - expect(formatHover(null, DEFAULT_MAX_HOVER_CHARS)).toBe('No hover information.') + expect(formatHover(null, DEFAULT_MAX_RESULT_CHARS)).toBe('No hover information.') }) it('returns short hover verbatim', () => { - expect(formatHover({ contents: '```ts\nx: number\n```' }, DEFAULT_MAX_HOVER_CHARS)).toBe('```ts\nx: number\n```') + expect(formatHover({ contents: '```ts\nx: number\n```' }, DEFAULT_MAX_RESULT_CHARS)).toBe('```ts\nx: number\n```') }) - it('caps hover at maxHoverChars and marks truncation', () => { - const text = formatHover({ contents: 'a'.repeat(50) }, 10) - expect(text.startsWith('aaaaaaaaaa\n')).toBe(true) - expect(text).toContain('hover truncated (limit 10 characters).') + it('caps the complete hover text including its truncation marker', () => { + const text = formatHover({ contents: 'a'.repeat(100) }, 60) + expect(text).toHaveLength(60) + expect(text).toContain('hover truncated (limit 60 characters).') + }) + + it('still honors a cap smaller than the truncation marker', () => { + expect(formatHover({ contents: 'a'.repeat(100) }, 10)).toHaveLength(10) }) }) describe('presentLspCall', () => { it('is a generic search card with an operation/cursor title and a line location', () => { - expect(presentLspCall({ operation: 'references', file_path: 'a.ts', line: 3, character: 7 })).toEqual({ + expect(presentLspCall({ operation: 'findReferences', file_path: 'a.ts', line: 3, character: 7 })).toEqual({ card: 'generic', kind: 'search', - title: 'LSP references a.ts:3:7', + title: 'LSP findReferences a.ts:3:7', locations: [{ path: 'a.ts', line: 3 }], }) }) diff --git a/packages/lsp/tool-lsp/tests/tool-lsp.spec.ts b/packages/lsp/tool-lsp/tests/tool-lsp.spec.ts index 577fa07396..37c995a3a6 100644 --- a/packages/lsp/tool-lsp/tests/tool-lsp.spec.ts +++ b/packages/lsp/tool-lsp/tests/tool-lsp.spec.ts @@ -5,6 +5,7 @@ import ToolRegistry from '@deepseek-ai/dsh-tools' import Lsp, { LspProviderId, type LspProvider, type LspProviderQuery, type LspQueryResult } from '@deepseek-ai/dsh-lsp' import * as ToolLsp from '@deepseek-ai/dsh-tool-lsp' import { DEFAULT_LSP_TOOL_TIMEOUT_MS, LSP_PROMPT_TEXT } from '@deepseek-ai/dsh-tool-lsp' +import { MAX_TIMER_DELAY_MS } from '@deepseek-ai/dsh-timeout' /** A scripted provider recording queries; `respond` yields the result or throws. */ function stubProvider( @@ -76,7 +77,7 @@ describe('tool-lsp registration', () => { it('exposes exactly the four operations in the schema enum', async () => { const { ctx } = await mount(stubProvider(() => okLocations)) const schema = ctx.tools.get('lsp')?.parameters as { properties: { operation: { enum: string[] } } } - expect(schema.properties.operation.enum).toEqual(['definition', 'references', 'implementation', 'hover']) + expect(schema.properties.operation.enum).toEqual(['goToDefinition', 'findReferences', 'goToImplementation', 'hover']) }) it('has no default export (namespace plugin shape)', () => { @@ -86,16 +87,28 @@ describe('tool-lsp registration', () => { it('rejects a non-positive config value at load', async () => { await expect(mount(stubProvider(() => okLocations), { maxLocations: 0 })).rejects.toThrow(/maxLocations/) }) + + it('rejects a timeout above Node timer range at load', async () => { + await expect(mount(stubProvider(() => okLocations), { timeoutMs: MAX_TIMER_DELAY_MS + 1 })) + .rejects.toThrow(/timeoutMs/) + expect(() => { + ToolLsp.apply(new Context(), { + maxLocations: 100, + maxResultChars: 16_000, + timeoutMs: MAX_TIMER_DELAY_MS + 1, + }) + }).toThrow(/timeoutMs/) + }) }) describe('tool-lsp execution', () => { it('converts one-based coordinates and passes the session cwd as workspaceRoot', async () => { const provider = stubProvider(() => okLocations) const { ctx } = await mount(provider) - const result = await call(ctx, { operation: 'definition', file_path: 'a.ts', line: 3, character: 5 }, '/ws') + const result = await call(ctx, { operation: 'goToDefinition', file_path: 'a.ts', line: 3, character: 5 }, '/ws') expect(result.isError).toBe(false) expect(provider.seen[0]).toMatchObject({ - operation: 'definition', + operation: 'goToDefinition', filePath: 'a.ts', position: { line: 2, character: 4 }, workspaceRoot: '/ws', @@ -104,7 +117,7 @@ describe('tool-lsp execution', () => { it('renders locations relative to the workspace', async () => { const { ctx } = await mount(stubProvider(() => okLocations)) - const result = await call(ctx, { operation: 'references', file_path: 'a.ts', line: 1, character: 1 }, '/ws') + const result = await call(ctx, { operation: 'findReferences', file_path: 'a.ts', line: 1, character: 1 }, '/ws') expect(result.content[0]).toEqual({ type: 'text', text: 'a.ts:1:1' }) }) @@ -118,7 +131,7 @@ describe('tool-lsp execution', () => { resolvedWorkspaceRoot: '/real/ws', })) const { ctx } = await mount(provider) - const result = await call(ctx, { operation: 'definition', file_path: 'a.ts', line: 1, character: 1 }, '/alias') + const result = await call(ctx, { operation: 'goToDefinition', file_path: 'a.ts', line: 1, character: 1 }, '/alias') expect(provider.seen[0]).toMatchObject({ workspaceRoot: '/alias' }) expect(result.content[0]).toEqual({ type: 'text', text: 'a.ts:1:1' }) }) @@ -131,14 +144,14 @@ describe('tool-lsp execution', () => { it('fails LSP_WORKSPACE_REQUIRED without a session cwd', async () => { const { ctx } = await mount(stubProvider(() => okLocations)) - const result = await call(ctx, { operation: 'definition', file_path: 'a.ts', line: 1, character: 1 }, null) + const result = await call(ctx, { operation: 'goToDefinition', file_path: 'a.ts', line: 1, character: 1 }, null) expect(result.isError).toBe(true) expect(result.error?.code).toBe('LSP_WORKSPACE_REQUIRED') }) it('surfaces a structured LSP_UNAVAILABLE when no provider handles the file', async () => { const { ctx } = await mount(stubProvider(() => okLocations, { '.py': 'python' })) - const result = await call(ctx, { operation: 'definition', file_path: 'a.ts', line: 1, character: 1 }, '/ws') + const result = await call(ctx, { operation: 'goToDefinition', file_path: 'a.ts', line: 1, character: 1 }, '/ws') expect(result.isError).toBe(true) expect(result.error?.code).toBe('LSP_UNAVAILABLE') }) @@ -161,7 +174,7 @@ describe('tool-lsp execution', () => { }, } const { ctx } = await mount(provider) - await call(ctx, { operation: 'definition', file_path: 'a.ts', line: 1, character: 1 }, '/ws') + await call(ctx, { operation: 'goToDefinition', file_path: 'a.ts', line: 1, character: 1 }, '/ws') // The timeout policy is not mounted here, so the signal is whatever the registry passes (may be // undefined); the point is the tool threads it through without throwing. expect(seen).toHaveLength(1) diff --git a/packages/lsp/tool-lsp/tsconfig.json b/packages/lsp/tool-lsp/tsconfig.json index be656effd2..e53735f570 100644 --- a/packages/lsp/tool-lsp/tsconfig.json +++ b/packages/lsp/tool-lsp/tsconfig.json @@ -26,6 +26,9 @@ { "path": "../../core/system-prompt" }, + { + "path": "../../util/timeout" + }, { "path": "../lsp" } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5fa837631f..c74251e7ba 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1512,6 +1512,9 @@ importers: '@deepseek-ai/dsh-system-prompt': specifier: workspace:^ version: link:../../core/system-prompt + '@deepseek-ai/dsh-timeout': + specifier: workspace:^ + version: link:../../util/timeout '@deepseek-ai/dsh-timeout-policy': specifier: workspace:^ version: link:../../timeout/timeout-policy