diff --git a/examples/acp-agent/tests/acp.snapshot.ts b/examples/acp-agent/tests/acp.snapshot.ts index 0d29f7b5ce..15670aa995 100644 --- a/examples/acp-agent/tests/acp.snapshot.ts +++ b/examples/acp-agent/tests/acp.snapshot.ts @@ -28,6 +28,7 @@ const CODE_MODE_CONFIG = fileURLToPath(new URL('../code-mode.cordis.yml', import const BOTH_MODE_CONFIG = fileURLToPath(new URL('../both-mode.cordis.yml', import.meta.url)) const ADVANCED_CONFIG = fileURLToPath(new URL('../advanced.cordis.yml', import.meta.url)) const FS_CONFIG = fileURLToPath(new URL('../fs.cordis.yml', import.meta.url)) +const LSP_CONFIG = fileURLToPath(new URL('./lsp.cordis.yml', import.meta.url)) function snapshotModeFromEnv(value: string | undefined): SnapshotSuiteOptions['mode'] { switch (value) { @@ -55,6 +56,7 @@ const SCENARIOS: Scenario[] = [ { name: 'todo-plan', hasModelTurn: true, recorded: true }, { name: 'skill-load', hasModelTurn: true, recorded: false, pinsHeader: true, headerClass: 'skill' }, { name: 'workspace-edit', hasModelTurn: true, recorded: true, pinsHeader: true, headerClass: 'fs', configPath: FS_CONFIG }, + { name: 'lsp-definition', hasModelTurn: true, recorded: false, pinsHeader: true, headerClass: 'lsp', configPath: LSP_CONFIG }, { name: 'fs-read', hasModelTurn: true, recorded: true, headerClass: 'fs', configPath: FS_CONFIG }, { name: 'fs-write', hasModelTurn: true, recorded: true, headerClass: 'fs', configPath: FS_CONFIG }, { name: 'fs-edit', hasModelTurn: true, recorded: true, headerClass: 'fs', configPath: FS_CONFIG }, diff --git a/examples/acp-agent/tests/lsp.cordis.snapshot.yml b/examples/acp-agent/tests/lsp.cordis.snapshot.yml new file mode 100644 index 0000000000..4d24ead86d --- /dev/null +++ b/examples/acp-agent/tests/lsp.cordis.snapshot.yml @@ -0,0 +1,27 @@ +# Keyless replay keeps the LSP composition intact and replaces only the model adapter. +- id: base + name: '@cordisjs/plugin-include' + config: + path: ../cordis.yml + patches: + - id: llm-deepseek + name: '@deepseek-ai/dsh-llm-deepseek' + disabled: true + - insert: + - id: lsp + name: '@deepseek-ai/dsh-lsp' + - id: lsp-local + name: '@deepseek-ai/dsh-lsp-local' + config: + servers: + fixture: + command: !!js process.execPath + args: ['./lsp-server.mjs'] + extensionToLanguage: + '.ts': typescript + - id: tool-lsp + name: '@deepseek-ai/dsh-tool-lsp' + config: + maxLocations: 1 + - id: llm-replay + name: '@deepseek-ai/dsh-llm-replay' diff --git a/examples/acp-agent/tests/lsp.cordis.yml b/examples/acp-agent/tests/lsp.cordis.yml new file mode 100644 index 0000000000..f1eefa99af --- /dev/null +++ b/examples/acp-agent/tests/lsp.cordis.yml @@ -0,0 +1,23 @@ +# Exercise the model-facing LSP tool through the shipped ACP app and Loader entry path. +# The scenario workspace supplies the deterministic stdio server used by this test composition. +- id: base + name: '@cordisjs/plugin-include' + config: + path: ../cordis.yml + patches: + - insert: + - id: lsp + name: '@deepseek-ai/dsh-lsp' + - id: lsp-local + name: '@deepseek-ai/dsh-lsp-local' + config: + servers: + fixture: + command: !!js process.execPath + args: ['./lsp-server.mjs'] + extensionToLanguage: + '.ts': typescript + - id: tool-lsp + name: '@deepseek-ai/dsh-tool-lsp' + config: + maxLocations: 1 diff --git a/examples/acp-agent/tests/snapshots/lsp-definition/input.json b/examples/acp-agent/tests/snapshots/lsp-definition/input.json new file mode 100644 index 0000000000..2b49f7d280 --- /dev/null +++ b/examples/acp-agent/tests/snapshots/lsp-definition/input.json @@ -0,0 +1,7 @@ +{ + "steps": [ + { "op": "initialize" }, + { "op": "newSession" }, + { "op": "prompt", "text": "Use the lsp tool exactly once to find the definition at subject.ts line 1 character 7, then reply with exactly DONE." } + ] +} diff --git a/examples/acp-agent/tests/snapshots/lsp-definition/session.jsonl b/examples/acp-agent/tests/snapshots/lsp-definition/session.jsonl new file mode 100644 index 0000000000..e1a93752bb --- /dev/null +++ b/examples/acp-agent/tests/snapshots/lsp-definition/session.jsonl @@ -0,0 +1,23 @@ +{"type":"session","version":0,"id":"{{sessionId}}","createdAt":0,"cwd":"{{cwd}}"} +{"type":"turn/start","seq":0,"time":0,"data":{"turn":1,"trigger":{"kind":"message","source":{"kind":"user"}}}} +{"type":"user/message","seq":1,"time":0,"data":{"content":[{"type":"text","text":"Use the lsp tool exactly once to find the definition at subject.ts line 1 character 7, then reply with exactly DONE."}],"source":{"kind":"user"}},"surfaceOp":"append"} +{"type":"step/start","seq":2,"time":0,"data":{"turn":1,"step":1}} +{"type":"request/header","seq":3,"time":0,"data":{"header":{"config":{"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":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}"}],"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":"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}} +{"type":"assistant/chunk","seq":14,"time":0,"data":{"turn":1,"step":2,"chunk":{"type":"block-start","index":0,"blockType":"text"}}} +{"type":"assistant/chunk","seq":15,"time":0,"data":{"turn":1,"step":2,"chunk":{"type":"text-delta","index":0,"text":"DONE"}}} +{"type":"assistant/chunk","seq":16,"time":0,"data":{"turn":1,"step":2,"chunk":{"type":"block-end","index":0,"block":{"type":"text","text":"DONE"}}}} +{"type":"assistant/chunk","seq":17,"time":0,"data":{"turn":1,"step":2,"chunk":{"type":"usage","usage":{"inputTokens":10,"outputTokens":2}}}} +{"type":"assistant/chunk","seq":18,"time":0,"data":{"turn":1,"step":2,"chunk":{"type":"finish","reason":{"kind":"stop"}}}} +{"type":"assistant/message","seq":19,"time":0,"data":{"turn":1,"step":2,"content":[{"type":"text","text":"DONE"}],"usage":{"inputTokens":10,"outputTokens":2}},"sourceEventSeqs":[14,15,16,17,18],"surfaceOp":"append"} +{"type":"step/end","seq":20,"time":0,"data":{"turn":1,"step":2}} +{"type":"turn/end","seq":21,"time":0,"data":{"turn":1,"reason":{"kind":"completed"}}} diff --git a/examples/acp-agent/tests/snapshots/lsp-definition/stdout.golden.jsonl b/examples/acp-agent/tests/snapshots/lsp-definition/stdout.golden.jsonl new file mode 100644 index 0000000000..0abf55a261 --- /dev/null +++ b/examples/acp-agent/tests/snapshots/lsp-definition/stdout.golden.jsonl @@ -0,0 +1,6 @@ +{"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":"Sets this session's sandbox and approval behavior.","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":"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_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.golden.md b/examples/acp-agent/tests/snapshots/lsp-definition/system-prompt.golden.md new file mode 100644 index 0000000000..21a80508f7 --- /dev/null +++ b/examples/acp-agent/tests/snapshots/lsp-definition/system-prompt.golden.md @@ -0,0 +1,15 @@ +You are an AI agent powered by the DeepSeek Harness SDK. + +You are a coding assistant powered by the deepseek-v4-flash model. Your working directory is {{cwd}}. Your bash tool runs under a file sandbox — a `[sandbox: file access denied …]` result is policy, not a command bug. + +Verify your work by running the code or tests. Keep answers brief and factual. + + +Check the [exit code: N] marker on every bash result; investigate failures before moving on. + +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. + +Approval prompts are disabled in this session: actions that require approval are rejected automatically — do not request sandbox escalation (do not set `sandbox_permissions`). + + +Use the workflow tool ONLY when the user explicitly asks for a workflow or for large multi-agent orchestration: you write a JavaScript script (the tool description documents the exact format) that fans work out across many subagents with phases and structured results. For one or two delegations, prefer plain subagent calls. diff --git a/examples/acp-agent/tests/snapshots/lsp-definition/tool-schemas.golden.json b/examples/acp-agent/tests/snapshots/lsp-definition/tool-schemas.golden.json new file mode 100644 index 0000000000..72aaa3bd7d --- /dev/null +++ b/examples/acp-agent/tests/snapshots/lsp-definition/tool-schemas.golden.json @@ -0,0 +1,282 @@ +{ + "initial": [ + { + "name": "bash", + "description": "Execute a bash command (`bash -c`) and return its stdout/stderr. Each call runs in a fresh shell: no state (cwd, variables, functions) persists between calls — pass `workdir` instead of using `cd`. Non-zero exits are reported as `[exit code: N]`. Commands may run under a file sandbox; a blocked file operation is reported as `[sandbox: file access denied under mode]` — a policy denial, not a bug in the command; do not retry another way (a background task reports the same marker via bash_output once it has finished). Long output is truncated to its tail; the full output is saved to a file whose path is reported when available. Set `run_in_background: true` for long-running commands: the call returns a task id immediately; poll it with `bash_output` and stop it with `bash_kill`. Attempting a command the sandbox may deny is safe and expected: run it and read the marker rather than assuming the denial. When a command is denied and a wider mode would let it succeed, escalate immediately in the same turn — the one sanctioned exception to a denial: retry the exact same command once with `sandbox_permissions` (the narrowest wider mode that suffices) plus a one-sentence `justification`. Do not detour through chat to ask permission first — the approval prompt raised by that retry is how the user consents. If the session states approval prompts are disabled, there is no exception: a denial is final — do not set `sandbox_permissions`. Never escalate speculatively: ground the request in a real denial — normally the one this command just hit; escalating up front is fine only when this session already denied the same access. A rejected escalation is final for that command — stop and explain, never work around it — but it does not forbid attempting or escalating other commands later.", + "parameters": { + "type": "object", + "properties": { + "command": { + "type": "string", + "description": "The bash command to execute." + }, + "description": { + "type": "string", + "description": "Clear, concise description of what this command does in active voice, 5-10 words (shown in the UI). Examples: \"ls\" → \"List files in current directory\"; \"git status\" → \"Show working tree status\"; \"npm install\" → \"Install package dependencies\"." + }, + "timeoutMs": { + "type": "number", + "description": "Timeout in milliseconds. The executor applies its configured default and cap, and kills the command on expiry." + }, + "workdir": { + "type": "string", + "description": "Working directory for this command. Defaults to the session workspace; a relative path is resolved against it." + }, + "run_in_background": { + "type": "boolean", + "description": "Run in the background and return a task id immediately. No timeout applies." + }, + "sandbox_permissions": { + "type": "string", + "description": "The wider sandbox mode this command needs. Only valid as a one-shot retry of a command the sandbox just denied; requires justification and user approval.", + "enum": [ + "workspace-write", + "danger-full-access" + ] + }, + "justification": { + "type": "string", + "description": "Required with sandbox_permissions: one sentence for the user explaining why this exact command needs the wider access." + } + }, + "required": [ + "command", + "description" + ] + } + }, + { + "name": "bash_kill", + "description": "Ask the executor to kill a running background bash task by task id.", + "parameters": { + "type": "object", + "properties": { + "task_id": { + "type": "string", + "description": "Task id returned by the bash tool." + } + }, + "required": [ + "task_id" + ] + } + }, + { + "name": "bash_output", + "description": "Read new output from a background bash task started with `bash` + `run_in_background`. Returns only output produced since the previous bash_output call, plus the task status. Tasks keep running while you do other work; poll again later for more output.", + "parameters": { + "type": "object", + "properties": { + "task_id": { + "type": "string", + "description": "Task id returned by the bash tool." + } + }, + "required": [ + "task_id" + ] + } + }, + { + "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.", + "parameters": { + "type": "object", + "properties": { + "operation": { + "type": "string", + "description": "definition, references, implementation, or hover.", + "enum": [ + "definition", + "references", + "implementation", + "hover" + ] + }, + "file_path": { + "type": "string", + "description": "The source file to query, relative to the workspace or absolute." + }, + "line": { + "type": "number", + "description": "One-based line of the cursor." + }, + "character": { + "type": "number", + "description": "One-based UTF-16 column of the cursor." + } + }, + "required": [ + "operation", + "file_path", + "line", + "character" + ] + } + }, + { + "name": "skill", + "description": "Load the full instructions for an available skill. Call this with the exact skill name from the session skill catalog before acting on a task that names or clearly matches that skill.", + "parameters": { + "type": "object", + "properties": { + "name": { + "type": "string", + "description": "The exact skill name from the available skills list." + } + }, + "required": [ + "name" + ] + } + }, + { + "name": "subagent", + "description": "Delegate a self-contained task to a subagent (a separate agent that works in its own context) and return its final result. Use this to offload focused, independent work — research, a scoped implementation, an analysis — so it does not consume this conversation's context. The subagent runs to completion and you receive only its final answer, not its intermediate steps. Give it a complete, standalone prompt: it does not see this conversation.", + "parameters": { + "type": "object", + "properties": { + "description": { + "type": "string", + "description": "A short (3-5 word) description of the delegated task, for display." + }, + "prompt": { + "type": "string", + "description": "The complete, self-contained task for the subagent. It does not share this conversation's context, so include everything it needs." + } + }, + "required": [ + "description", + "prompt" + ] + } + }, + { + "name": "subagent_fork", + "description": "Delegate a task to a subagent that INHERITS this conversation: a child agent seeded with all completed turns so far (it does not see the current in-flight turn), returning only its final result. Use this when the subtask builds on this conversation's context — a follow-up analysis, a review, a continuation — without consuming this conversation's context for the work itself. You receive only its final answer, not its intermediate steps.", + "parameters": { + "type": "object", + "properties": { + "description": { + "type": "string", + "description": "A short (3-5 word) description of the delegated task, for display." + }, + "prompt": { + "type": "string", + "description": "The task for the subagent. It already sees this conversation's completed turns, so build on them freely and state only what is new." + } + }, + "required": [ + "description", + "prompt" + ] + } + }, + { + "name": "todo_write", + "description": "Record and update a structured task list for the current work. Send the ENTIRE list every call — it REPLACES the previous list (there are no partial updates, no per-item edits). Use it to plan multi-step work and show progress: add one todo per concrete step before you start. Keep AT MOST ONE todo `in_progress` at a time; while work remains, exactly one active task should be `in_progress`. Mark a todo `completed` the moment it is done (do not batch completions), and allow no `in_progress` item only once all work is complete. Skip the list for trivial single-step tasks. Statuses: `pending` (not started), `in_progress` (being worked on now), `completed` (finished).", + "parameters": { + "type": "object", + "properties": { + "todos": { + "type": "array", + "description": "The COMPLETE task list, replacing any previous list.", + "items": { + "type": "object", + "properties": { + "content": { + "type": "string", + "description": "What the task is — a short imperative line." + }, + "status": { + "type": "string", + "description": "pending (not started) | in_progress (now) | completed (done).", + "enum": [ + "pending", + "in_progress", + "completed" + ] + } + }, + "required": [ + "content", + "status" + ] + } + } + }, + "required": [ + "todos" + ] + } + }, + { + "name": "workflow", + "description": "Run a JavaScript workflow script that orchestrates subagents at scale. Use this for work that fans out across many independent pieces — an audit over many files, a migration, multi-angle research, adversarial verification of findings — where you write the orchestration as a script instead of delegating turn by turn.\n\nThe workflow's identity rides the `meta` parameter as JSON: required `name` (short kebab-case) and `description` strings, optional `whenToUse` string and `phases` array (`{title, detail?, model?}`). The `script` parameter is the plain JavaScript body ONLY (NOT TypeScript, and NO `export const meta` statement — meta is a parameter, not code), running with top-level await; end with `return ` — the value must be JSON-serializable and is this tool's result.\n\nScript-body hooks:\n- `agent(prompt, opts?): Promise` — run one subagent to completion. Without `opts.schema` it resolves to the child's final text; with `opts.schema` (an object-rooted JSON Schema using ONLY type/properties/required/additionalProperties/items/enum/const — no oneOf/pattern/format/numeric bounds) it resolves to the validated object. Resolves `null` when the child fails (filter with `.filter(Boolean)`). Other opts: `label` (display), `phase` (progress group), `model` (override). Anything else (`effort`/`isolation`/`agentType`) is rejected loudly.\n- `pipeline(items, ...stages): Promise` — run each item through the stages independently with NO barrier between stages (prefer this for multi-stage work). Each stage receives `(prev, item, index)`. An ordinary stage throw drops that ITEM to `null` and skips its remaining stages.\n- `parallel(thunks): Promise` — run zero-argument functions concurrently and await ALL of them (a barrier; use only when a stage genuinely needs every prior result together). A throwing thunk resolves to `null`.\n- `phase(title)` — start a progress phase; `log(message)` — narrate progress; `args` — the tool call's `args` input, verbatim.\n\nMisused hooks (bad arguments, unknown options, unsupported schemas, tripped caps) throw errors that ALWAYS kill the script — they never dissolve into a per-item `null`.\n\nConstraints: concurrency and total-agent caps apply; no filesystem, network, timers, or Node.js APIs are provided — the agents do the work, the script only coordinates them. The run executes in the foreground: this call returns when the whole script finishes.", + "parameters": { + "type": "object", + "properties": { + "script": { + "type": "string", + "description": "The plain-JS workflow script body (top-level await allowed; NO `export const meta` statement; end with `return `)." + }, + "meta": { + "type": "object", + "description": "The workflow identity block (plain JSON — never code).", + "properties": { + "name": { + "type": "string", + "description": "Short kebab-case workflow name." + }, + "description": { + "type": "string", + "description": "One-line description of what the workflow does." + }, + "whenToUse": { + "type": "string", + "description": "Optional guidance on when this workflow applies." + }, + "phases": { + "type": "array", + "description": "Optional phase declarations matched by phase() calls.", + "items": { + "type": "object", + "properties": { + "title": { + "type": "string", + "description": "The phase title phase() calls match by exact string." + }, + "detail": { + "type": "string", + "description": "Optional one-line description of the phase." + }, + "model": { + "type": "string", + "description": "Optional model override this phase is expected to use." + } + }, + "required": [ + "title" + ] + } + } + }, + "required": [ + "name", + "description" + ] + }, + "args": { + "type": "object", + "description": "Optional JSON input exposed to the script as the `args` global (wrap a bare list as a field, e.g. {\"files\": [...]})." + } + }, + "required": [ + "script", + "meta" + ] + } + } + ], + "deltas": [] +} diff --git a/examples/acp-agent/tests/snapshots/lsp-definition/workspace/lsp-server.mjs b/examples/acp-agent/tests/snapshots/lsp-definition/workspace/lsp-server.mjs new file mode 100644 index 0000000000..431b322e0b --- /dev/null +++ b/examples/acp-agent/tests/snapshots/lsp-definition/workspace/lsp-server.mjs @@ -0,0 +1,57 @@ +import { resolve } from 'node:path' +import { pathToFileURL } from 'node:url' + +let buffered = Buffer.alloc(0) + +function frame(message) { + const body = Buffer.from(JSON.stringify({ jsonrpc: '2.0', ...message })) + return Buffer.concat([Buffer.from(`Content-Length: ${body.length}\r\n\r\n`), body]) +} + +function location(line) { + return { + uri: pathToFileURL(resolve('subject.ts')).href, + range: { start: { line, character: 6 }, end: { line, character: 12 } }, + } +} + +function handle(message) { + switch (message.method) { + case 'initialize': + process.stdout.write(frame({ + id: message.id, + result: { + capabilities: { + positionEncoding: 'utf-16', + textDocumentSync: 1, + definitionProvider: true, + }, + }, + })) + break + case 'textDocument/definition': + process.stdout.write(frame({ id: message.id, result: [location(0), location(1)] })) + break + case 'shutdown': + process.stdout.write(frame({ id: message.id, result: null })) + break + case 'exit': + process.exit(0) + } +} + +process.stdin.on('data', (chunk) => { + buffered = Buffer.concat([buffered, chunk]) + for (;;) { + const headerEnd = buffered.indexOf('\r\n\r\n') + if (headerEnd < 0) return + const match = /Content-Length: (\d+)/i.exec(buffered.toString('ascii', 0, headerEnd)) + if (match === null) throw new Error('missing Content-Length') + const length = Number(match[1]) + const bodyStart = headerEnd + 4 + if (buffered.length < bodyStart + length) return + const message = JSON.parse(buffered.toString('utf8', bodyStart, bodyStart + length)) + buffered = buffered.subarray(bodyStart + length) + handle(message) + } +}) diff --git a/examples/acp-agent/tests/snapshots/lsp-definition/workspace/subject.ts b/examples/acp-agent/tests/snapshots/lsp-definition/workspace/subject.ts new file mode 100644 index 0000000000..6f3d62ca43 --- /dev/null +++ b/examples/acp-agent/tests/snapshots/lsp-definition/workspace/subject.ts @@ -0,0 +1,2 @@ +export const answer = 42 +console.log(answer) diff --git a/knip.json b/knip.json index 76634b0b78..210aea5cae 100644 --- a/knip.json +++ b/knip.json @@ -1,6 +1,7 @@ { "$schema": "https://unpkg.com/knip@5/schema.json", "exclude": ["duplicates"], + "ignore": ["examples/*/tests/snapshots/*/workspace/**/*"], "ignoreBinaries": ["bwrap", "sandbox-exec"], "ignoreWorkspaces": ["vendor/*", "python/sdk-runtime"], "workspaces": { diff --git a/packages/lsp/lsp-local/src/connection.ts b/packages/lsp/lsp-local/src/connection.ts index 795fe0dbff..e655877eb5 100644 --- a/packages/lsp/lsp-local/src/connection.ts +++ b/packages/lsp/lsp-local/src/connection.ts @@ -10,6 +10,7 @@ import type { ChildProcessByStdio } from 'node:child_process' import { spawn } from 'node:child_process' import type { Readable, Writable } from 'node:stream' +import { setImmediate as yieldToEventLoop } from 'node:timers/promises' import { encodeMessage, MessageDecoder } from './framing.ts' /** How to launch the server and answer its config requests. */ @@ -163,6 +164,19 @@ export class LspConnection { this.signalGroup('SIGKILL') } + /** + * Wait until the owned process group has no members. + * @param signal - optional bound for the wait. + * @returns `true` when the group exited, or `false` when the signal aborted first. + */ + async waitForProcessGroupExit(signal?: AbortSignal): Promise { + while (this.processGroupAlive()) { + if (signal?.aborted) return false + await yieldToEventLoop() + } + return true + } + /** * Signal the whole process group (negative pid) so helper processes are reached; fall back to the * direct child if the group send fails. Never throws — teardown races process exit. @@ -182,6 +196,25 @@ export class LspConnection { } } + /** Whether the detached process group still has at least one member. */ + private processGroupAlive(): boolean { + const pid = this.child.pid + /* v8 ignore next -- only an asynchronous spawn failure omits pid; its close path owns cleanup. */ + if (pid === undefined) return false + try { + process.kill(-pid, 0) + return true + } catch (error) { + const code = (error as NodeJS.ErrnoException).code + if (code === 'ESRCH') return false + /* v8 ignore start -- EPERM and non-POSIX negative-pid failures are platform defenses; CI runs + process-group lifecycle tests on POSIX hosts where absence reports ESRCH. */ + if (code === 'EPERM') return true + return this.child.exitCode === null && this.child.signalCode === null + /* v8 ignore stop */ + } + } + private onStdout(chunk: Buffer): void { let messages: unknown[] try { diff --git a/packages/lsp/lsp-local/src/index.ts b/packages/lsp/lsp-local/src/index.ts index 03ae5e9781..67e86da11c 100644 --- a/packages/lsp/lsp-local/src/index.ts +++ b/packages/lsp/lsp-local/src/index.ts @@ -169,8 +169,8 @@ function assertPositiveInteger(providerId: string, name: string, value: number): class LocalLspProvider implements LspProvider { readonly id: LspProviderId readonly extensionToLanguage: Readonly> - /** Single-flight map: canonical workspace realpath → the (pending) instance for it. */ - private readonly instances = new Map>() + /** One live instance per canonical workspace realpath. */ + private readonly instances = new Map() private disposed = false constructor( @@ -188,12 +188,17 @@ class LocalLspProvider implements LspProvider { return this.disposed } - async query(request: LspProviderQuery, signal?: AbortSignal): Promise { - /* v8 ignore next -- the seam unregisters this provider on dispose, so a query never reaches it disposed; defensive. */ + /** Reject work that cannot publish or use a provider-owned instance. */ + 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') - // Honor an already-aborted signal before any host I/O so a canceled request neither reads nor - // spawns a server. 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 @@ -201,49 +206,37 @@ class LocalLspProvider implements LspProvider { 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. - /* v8 ignore next -- guards a dispose landing during the canonicalize/read await; not a reproducible unit race. */ - if (this.isDisposed()) throw new Error('lsp-local provider is disposed') - // Re-check cancellation too: an abort during the canonicalize/read awaits must not go on to spawn - // (or pool) a server solely for an operation the caller already gave up on. - if (signal?.aborted) throw abortError(signal) - let instance = await this.instanceFor(workspace) - // A pooled server that exited while idle resolves to a dead instance: evict it and create a fresh - // one before dispatch, so this query does not have to fail on a closed connection first. One retry - // suffices — the replacement was just constructed and has not been used. + 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) { - await this.evictIfCurrent(workspace, instance) - instance = await this.instanceFor(workspace) + 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) await this.evictIfCurrent(workspace, instance) + if (instance.dead) this.evictIfCurrent(workspace, instance) } } - /** Single-flight one instance per canonical workspace; a rejected creation clears the slot. */ - private instanceFor(workspace: string): Promise { + /** Return or synchronously publish the one instance for a canonical workspace. */ + private instanceFor(workspace: string): LspInstance { + this.assertActive() const existing = this.instances.get(workspace) if (existing !== undefined) return existing - const created = Promise.resolve().then(() => this.createInstance(workspace)) + const created = this.createInstance(workspace) this.instances.set(workspace, created) - /* v8 ignore next 3 -- createInstance (the LspInstance constructor) does not throw; spawn failures - surface asynchronously through the instance, so this creation-rejection cleanup is defensive. */ - created.catch(() => { - if (this.instances.get(workspace) === created) this.instances.delete(workspace) - }) return created } - /** Drop the slot for `workspace` iff it still resolves to `instance` (a concurrent replacement survives). */ - private async evictIfCurrent(workspace: string, instance: LspInstance): Promise { - const slot = this.instances.get(workspace) - /* v8 ignore next -- the slot-undefined/mismatch arm needs a concurrent eviction of the same slot; defensive. */ - if (slot !== undefined && (await settledInstance(slot)) === instance) { - this.instances.delete(workspace) - } + /** Drop the slot iff it still contains this instance. */ + private evictIfCurrent(workspace: string, instance: LspInstance): void { + /* v8 ignore next -- mismatch requires another query to replace the slot before this finally runs. */ + if (this.instances.get(workspace) === instance) this.instances.delete(workspace) } private createInstance(workspace: string): LspInstance { @@ -265,26 +258,9 @@ class LocalLspProvider implements LspProvider { /** Dispose every live instance and block further queries. */ async disposeAll(): Promise { this.disposed = true - const pending = [...this.instances.values()] + const live = [...this.instances.values()] this.instances.clear() - await Promise.all(pending.map(async (entry) => { - try { - const instance = await entry - await instance.dispose() - } catch { - // A never-initialized instance already rejected; nothing to tear down. - } - })) - } -} - -/** Resolve a slot promise to its instance for identity comparison, tolerating a pending rejection. */ -async function settledInstance(slot: Promise): Promise { - try { - return await slot - } catch { - /* v8 ignore next -- a slot promise only rejects if createInstance throws, which it never does; defensive. */ - return undefined + await Promise.all(live.map(instance => instance.dispose())) } } diff --git a/packages/lsp/lsp-local/src/instance.ts b/packages/lsp/lsp-local/src/instance.ts index 8bf2e48c5b..fb67f6e777 100644 --- a/packages/lsp/lsp-local/src/instance.ts +++ b/packages/lsp/lsp-local/src/instance.ts @@ -48,6 +48,8 @@ export class LspInstance { /** The serialization tail: each query awaits the prior one, so lifecycles never interleave. */ private queue: Promise = Promise.resolve() private disposed = false + /** The one teardown transaction shared by abort, failure, and explicit disposal. */ + private teardownPromise: Promise | undefined /** Set once the process closes, so the pool can synchronously skip a dead instance. */ private processClosed = false /** Populated once `initialize` succeeds; a failed handshake rejects every query. */ @@ -116,9 +118,8 @@ export class LspInstance { await this.abortable(this.ready, signal) } catch (error) { if (!this.dead) { - this.disposed = true /* v8 ignore next -- ready rejects with an Error (abort reason or initialize failure); the String() fallback is defensive. */ - await this.tearDown(error instanceof Error ? error : new Error(String(error))) + await this.startTeardown(error instanceof Error ? error : new Error(String(error))) } throw error } @@ -155,8 +156,7 @@ export class LspInstance { listener, so a synchronous didClose write failure is a defensive path. */ // 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. - this.disposed = true - void this.tearDown(error instanceof Error ? error : new Error(String(error))) + void this.startTeardown(error instanceof Error ? error : new Error(String(error))) /* v8 ignore stop */ } } @@ -210,19 +210,20 @@ export class LspInstance { this.connection.cancel(requestId) // Wait, bounded, for the server to honor the cancellation. If it does not, the request is still // running: terminate the instance (disposal awaits process close) so nothing outlives the query. - using grace = deadline(undefined, this.spec.killGraceMs, 'LSP_CANCEL_GRACE') - // `settled` is true if the request finished (either outcome) before the grace elapsed. - const settled = await Promise.race([ - send.then(markSettled, markSettled), - new Promise((resolve) => { - /* v8 ignore next -- the cancel-grace deadline signal is freshly armed and not yet aborted here; defensive. */ - if (grace.signal.aborted) { resolve(false); return } - grace.signal.addEventListener('abort', () => { resolve(false) }, { once: true }) - }), - ]) - if (!settled && !this.disposed) { - this.disposed = true - await this.tearDown(abortError(signal)) + const grace = deadline(undefined, this.spec.killGraceMs, 'LSP_CANCEL_GRACE') + try { + // `settled` is true if the request finished (either outcome) before the grace elapsed. + const settled = await Promise.race([ + send.then(markSettled, markSettled), + new Promise((resolve) => { + /* v8 ignore next -- the cancel-grace deadline signal is freshly armed and not yet aborted here; defensive. */ + if (grace.signal.aborted) { resolve(false); return } + grace.signal.addEventListener('abort', () => { resolve(false) }, { once: true }) + }), + ]) + if (!settled) await this.startTeardown(abortError(signal)) + } finally { + grace[Symbol.dispose]() } throw error } @@ -262,21 +263,24 @@ export class LspInstance { * process close so nothing outlives disposal. */ async dispose(): Promise { - if (this.disposed) { - await this.connection.closed - return - } + await this.startTeardown(new Error('LSP instance disposed')) + } + + /** Publish disposal once and make every caller await the same quiescence boundary. */ + private startTeardown(reason: Error): Promise { this.disposed = true - await this.tearDown(new Error('LSP instance disposed')) + this.teardownPromise ??= this.tearDown(reason) + return this.teardownPromise } private async tearDown(_reason: Error): Promise { + const shutdownDeadline = deadline(undefined, this.spec.shutdownTimeoutMs, 'LSP_SHUTDOWN') try { - using shutdownDeadline = deadline(undefined, this.spec.shutdownTimeoutMs, 'LSP_SHUTDOWN') await this.gracefulShutdown(shutdownDeadline.signal) - return } catch { - // Graceful shutdown failed or timed out: fall through to signal escalation. + // Graceful shutdown failed or timed out; process-group cleanup below remains authoritative. + } finally { + shutdownDeadline[Symbol.dispose]() } await this.forceTerminate() } @@ -288,20 +292,21 @@ export class LspInstance { await this.abortable(this.connection.closed, signal) } - /** SIGTERM, wait `killGraceMs` for close, then SIGKILL; await full process close either way. */ + /** SIGTERM the group, escalate after `killGraceMs`, then await leader and helper exit. */ private async forceTerminate(): Promise { this.connection.terminate() - using graceDeadline = deadline(undefined, this.spec.killGraceMs, 'LSP_KILL_GRACE') - const closedInTime = await Promise.race([ - this.connection.closed.then(() => true), - new Promise((resolve) => { - /* v8 ignore next -- the kill-grace deadline signal is freshly armed and not yet aborted here; defensive. */ - if (graceDeadline.signal.aborted) { resolve(false); return } - graceDeadline.signal.addEventListener('abort', () => { resolve(false) }, { once: true }) - }), + const graceDeadline = deadline(undefined, this.spec.killGraceMs, 'LSP_KILL_GRACE') + let groupExited: boolean + try { + groupExited = await this.connection.waitForProcessGroupExit(graceDeadline.signal) + } finally { + graceDeadline[Symbol.dispose]() + } + if (!groupExited) this.connection.kill() + await Promise.all([ + this.connection.closed, + this.connection.waitForProcessGroupExit(), ]) - if (!closedInTime) this.connection.kill() - await this.connection.closed } } diff --git a/packages/lsp/lsp-local/tests/instance.spec.ts b/packages/lsp/lsp-local/tests/instance.spec.ts index 52fc751754..f46d72571b 100644 --- a/packages/lsp/lsp-local/tests/instance.spec.ts +++ b/packages/lsp/lsp-local/tests/instance.spec.ts @@ -236,6 +236,26 @@ describe('LspInstance disposal', () => { await expect(instance.dispose()).resolves.toBeUndefined() }) + it('awaits a surviving process-group helper on every concurrent dispose', async () => { + const marker = join(root, 'helper.pid') + const helper = 'process.on("SIGTERM",()=>{});setInterval(()=>{},1000);' + const script = 'const{spawn}=require("node:child_process");const{writeFileSync}=require("node:fs");' + + `const helper=spawn(process.execPath,["-e",${JSON.stringify(helper)}],{stdio:"ignore"});` + + `writeFileSync(${JSON.stringify(marker)},String(helper.pid));` + + RESPONDING_SERVER + const instance = scriptInstance(script, { shutdownTimeoutMs: 100, killGraceMs: 100 }) + await run(instance, 'definition') + const helperPid = Number(await readFile(marker, 'utf8')) + try { + const first = instance.dispose() + await instance.dispose() + expect(processAlive(helperPid)).toBe(false) + await first + } finally { + if (processAlive(helperPid)) process.kill(helperPid, 'SIGKILL') + } + }) + it('carries a non-Error abort reason as a generic aborted error', async () => { const instance = makeInstance({ LSP_FAKE_HANG: '1' }) const controller = new AbortController() @@ -245,3 +265,14 @@ describe('LspInstance disposal', () => { await expect(pending).rejects.toThrow(/aborted/) }) }) + +/** Probe a pid without changing its state. */ +function processAlive(pid: number): boolean { + try { + process.kill(pid, 0) + return true + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ESRCH') return false + throw error + } +} diff --git a/packages/lsp/tool-lsp/tests/integration.spec.ts b/packages/lsp/tool-lsp/tests/integration.spec.ts index 74d9c6ae77..5c98d9fff0 100644 --- a/packages/lsp/tool-lsp/tests/integration.spec.ts +++ b/packages/lsp/tool-lsp/tests/integration.spec.ts @@ -12,10 +12,8 @@ import * as TimeoutPolicy from '@deepseek-ai/dsh-timeout-policy' import * as ToolLsp from '@deepseek-ai/dsh-tool-lsp' /** - * Real-composition integration: the model-facing `lsp` tool over the real seam, the real - * `dsh-lsp-local` provider (driving an inline stdio server), and the real `dsh-timeout-policy`, all - * driven only through `ctx.tools.execute()`. Pins that a query round-trips end to end and that the - * policy's `TOOL_TIMEOUT` budget wins when the server hangs. + * Focused in-process integration of the model-facing tool, seam, local provider, and timeout policy. + * The `lsp-definition` ACP snapshot owns the shipped Loader/app entry path. */ let root: string @@ -77,7 +75,7 @@ function call(ctx: Context, args: unknown) { }) } -describe('tool-lsp real composition', () => { +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 }) diff --git a/packages/lsp/tool-lsp/tests/load-path.spec.ts b/packages/lsp/tool-lsp/tests/load-path.spec.ts index 13dbaa7b78..7b3ec0f241 100644 --- a/packages/lsp/tool-lsp/tests/load-path.spec.ts +++ b/packages/lsp/tool-lsp/tests/load-path.spec.ts @@ -1,15 +1,15 @@ /** - * Real-load-path guard for @deepseek-ai/dsh-tool-lsp. It is a NAMESPACE plugin with `inject`, so a + * Loader export-shape guard for @deepseek-ai/dsh-tool-lsp. It is a NAMESPACE plugin with `inject`, so a * stray `export default apply` would make the Loader's `unwrapExports` collapse the module to the - * bare `apply`, dropping `inject` (postmortem 0001). This unwraps through the REAL - * `Loader.prototype.unwrapExports` and verifies the namespace shape survives. + * bare `apply`, dropping `inject` (postmortem 0001). This verifies the namespace survives + * `Loader.prototype.unwrapExports`; the `lsp-definition` ACP snapshot owns full app composition. */ import { describe, expect, it } from 'vitest' import Loader from '@cordisjs/plugin-loader' import * as toolLsp from '@deepseek-ai/dsh-tool-lsp' -describe('dsh-tool-lsp real-load-path guard', () => { +describe('dsh-tool-lsp Loader export-shape guard', () => { it('has no default export and keeps name/inject/Config through unwrapExports', () => { expect('default' in toolLsp).toBe(false)