feat(subprocess): migrate lsp-local, subagent-acp, and the env scrubs onto the seam
Review direction (tianyicui, PR #660): in a stacked PR, change all other process-running places to use the new service. - lsp-local: LspConnection spawns through ctx.subprocess (piped protocol streams + a no-spill collected stderr tail); its private process-tree helpers (POSIX group signalling, Windows taskkill, liveness polling) are deleted in favor of the seam's handle verbs, and its buildChildEnv now rides scrubbedParentEnv (LSP children also stop inheriting stale DSH_*). The plugin injects 'subprocess'; compositions/tests mount dsh-subprocess-local. - subagent-acp: the ACP child spawns through the seam (piped ndjson streams, inherited stderr); spawn failure surfaces through done-rejection into the same startup race; disposal is handle.dispose with the plugin's configured graces. dsh-subagent-subprocess is DELETED — its dispose ladder and scrub are the seam's, and the isolated-config-dir helper had no consumer. - mcp-client, pty-local, sdk-helper: adopt scrubbedParentEnv as the one scrub definition (their spawns stay put by ownership: the MCP SDK and node-pty own those calls; the SDK wizard runs outside any composition). - Coverage: per-file 100% over every touched src file, with each v8 ignore carrying a platform or contract reason; new suites cover stdio dispositions, the dispose ladder tiers, injected-win32 tree semantics, waitForExit, settled-kill/terminate no-ops, and spawn-failure disposal. - Docs: consumer-migration Agent Note (en; zh follows in this PR), seam note updated in place, subprocess.md rewritten for the reshaped vocabulary (type-equiv re-registered), READMEs and SERVICE_ROLES updated, taskkill added to knip ignoreBinaries.
This commit is contained in:
@@ -16,7 +16,8 @@ import { afterAll, beforeAll, describe, expect, it } from 'vitest'
|
||||
|
||||
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 subprocessLib = join(pkgDir, '../../subprocess/subprocess-local/lib/index.js')
|
||||
const built = existsSync(join(pkgDir, 'lib/index.js')) && existsSync(seamLib) && existsSync(subprocessLib)
|
||||
|
||||
const fixtureServer = fileURLToPath(new URL('./fixture-server.ts', import.meta.url))
|
||||
|
||||
@@ -41,8 +42,10 @@ describe.skipIf(!built)('built lib real load path (plain node)', () => {
|
||||
const { Context } = await import('cordis')
|
||||
const { default: Lsp } = await import('@deepseek-ai/dsh-lsp')
|
||||
const LspLocal = await import('@deepseek-ai/dsh-lsp-local')
|
||||
const { default: LocalSubprocessService } = await import('@deepseek-ai/dsh-subprocess-local')
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(Lsp)
|
||||
await ctx.plugin(LocalSubprocessService)
|
||||
await ctx.plugin(LspLocal, {
|
||||
servers: {
|
||||
fake: {
|
||||
|
||||
@@ -1,18 +1,8 @@
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import { fileURLToPath } from 'node:url'
|
||||
import { LspConnection } from '@deepseek-ai/dsh-lsp-local'
|
||||
import {
|
||||
signalProcessGroup,
|
||||
signalProcessTree,
|
||||
taskkillProcessTree,
|
||||
waitForTreeExit,
|
||||
} from '@deepseek-ai/dsh-lsp-local/src/connection.ts'
|
||||
import type {
|
||||
ConnectionWriter,
|
||||
ProcessSignalRunner,
|
||||
ProcessTreeOperations,
|
||||
TaskkillRunner,
|
||||
} from '@deepseek-ai/dsh-lsp-local/src/connection.ts'
|
||||
import type { ConnectionWriter } from '@deepseek-ai/dsh-lsp-local/src/connection.ts'
|
||||
import { spawnSubprocess } from '@deepseek-ai/dsh-subprocess-local/src/spawn.ts'
|
||||
|
||||
const fixtureServer = fileURLToPath(new URL('./fixture-server.ts', import.meta.url))
|
||||
|
||||
@@ -42,8 +32,9 @@ function connect(
|
||||
env: { ...process.env as Record<string, string>, ...env },
|
||||
maxMessageBytes: 16_000_000,
|
||||
maxStderrBytes: 100_000,
|
||||
pipeDrainGraceMs: 3_000,
|
||||
configuration: { setting: 42 },
|
||||
}, (method, params) => {
|
||||
}, spawnSubprocess, (method, params) => {
|
||||
seen?.push({ method, params })
|
||||
return onServerRequest(method, params)
|
||||
})
|
||||
@@ -151,8 +142,9 @@ function connectScript(script: string, maxStderrBytes = 100_000, writer?: Connec
|
||||
env: { ...process.env as Record<string, string> },
|
||||
maxMessageBytes: 16_000_000,
|
||||
maxStderrBytes,
|
||||
pipeDrainGraceMs: 3_000,
|
||||
configuration: null,
|
||||
}, () => Promise.resolve(null), writer)
|
||||
}, spawnSubprocess, () => Promise.resolve(null), writer)
|
||||
open.push(conn)
|
||||
return conn
|
||||
}
|
||||
@@ -166,8 +158,9 @@ describe('LspConnection edge behavior', () => {
|
||||
env: {},
|
||||
maxMessageBytes: 1000,
|
||||
maxStderrBytes: 1000,
|
||||
pipeDrainGraceMs: 3_000,
|
||||
configuration: null,
|
||||
}, () => Promise.resolve(null))
|
||||
}, spawnSubprocess, () => Promise.resolve(null))
|
||||
open.push(conn)
|
||||
await expect(conn.request('initialize', {})).rejects.toThrow()
|
||||
})
|
||||
@@ -248,72 +241,6 @@ describe('LspConnection edge behavior', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('process-tree signaling', () => {
|
||||
it('forwards POSIX process-group signals through the host runner', () => {
|
||||
const run: ProcessSignalRunner = vi.fn(() => true)
|
||||
signalProcessGroup(-42, 'SIGKILL', run)
|
||||
expect(run).toHaveBeenCalledWith(-42, 'SIGKILL')
|
||||
})
|
||||
|
||||
it('waits for tree exit and stops when its bound aborts', async () => {
|
||||
const isAlive = vi.fn()
|
||||
.mockReturnValueOnce(true)
|
||||
.mockReturnValue(false)
|
||||
const yieldNow = vi.fn(() => Promise.resolve())
|
||||
await expect(waitForTreeExit(isAlive, undefined, yieldNow)).resolves.toBe(true)
|
||||
expect(yieldNow).toHaveBeenCalledOnce()
|
||||
|
||||
const controller = new AbortController()
|
||||
controller.abort()
|
||||
await expect(waitForTreeExit(() => true, controller.signal, yieldNow)).resolves.toBe(false)
|
||||
})
|
||||
|
||||
it('uses taskkill for a Windows tree and a negative pid for a POSIX group', () => {
|
||||
const operations = fakeProcessTreeOperations()
|
||||
signalProcessTree('win32', 42, 'SIGTERM', operations)
|
||||
expect(operations.taskkill).toHaveBeenCalledWith(42)
|
||||
expect(operations.signal).not.toHaveBeenCalled()
|
||||
|
||||
signalProcessTree('linux', 42, 'SIGKILL', operations)
|
||||
expect(operations.signal).toHaveBeenCalledWith(-42, 'SIGKILL')
|
||||
})
|
||||
|
||||
it('surfaces a Windows taskkill failure without downgrading to the direct child', () => {
|
||||
const fallback = fakeProcessTreeOperations()
|
||||
vi.mocked(fallback.taskkill).mockImplementation(() => { throw new Error('taskkill unavailable') })
|
||||
expect(() => { signalProcessTree('win32', 42, 'SIGTERM', fallback) }).toThrow(/taskkill unavailable/)
|
||||
expect(fallback.killChild).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('tolerates a POSIX tree-signaling race after the direct child is already gone', () => {
|
||||
const posixGone = fakeProcessTreeOperations()
|
||||
vi.mocked(posixGone.signal).mockImplementation(() => { throw new Error('group gone') })
|
||||
vi.mocked(posixGone.killChild).mockImplementation(() => { throw new Error('child gone') })
|
||||
expect(() => { signalProcessTree('linux', 42, 'SIGKILL', posixGone) }).not.toThrow()
|
||||
})
|
||||
|
||||
it('runs taskkill for the full tree, accepts an absent tree, and rejects command failures', () => {
|
||||
const success: TaskkillRunner = vi.fn(() => ({ status: 0 }))
|
||||
taskkillProcessTree(42, success)
|
||||
expect(success).toHaveBeenCalledWith('taskkill', ['/PID', '42', '/T', '/F'], { stdio: 'ignore' })
|
||||
|
||||
expect(() => { taskkillProcessTree(42, () => ({ status: 128 })) }).not.toThrow()
|
||||
|
||||
const spawnFailure = new Error('cannot spawn taskkill')
|
||||
expect(() => { taskkillProcessTree(42, () => ({ status: null, error: spawnFailure })) }).toThrow(spawnFailure)
|
||||
expect(() => { taskkillProcessTree(42, () => ({ status: 1 })) }).toThrow(/status 1/)
|
||||
})
|
||||
})
|
||||
|
||||
/** Create observable process-tree operations without touching host processes. */
|
||||
function fakeProcessTreeOperations(): ProcessTreeOperations {
|
||||
return {
|
||||
signal: vi.fn(),
|
||||
killChild: vi.fn(),
|
||||
taskkill: vi.fn(),
|
||||
}
|
||||
}
|
||||
|
||||
/** Poll a predicate until it holds or a deadline elapses. */
|
||||
async function waitFor(predicate: () => boolean, timeoutMs = 3000): Promise<void> {
|
||||
const start = Date.now()
|
||||
|
||||
@@ -9,6 +9,7 @@ import type { ConnectionWriter } from '@deepseek-ai/dsh-lsp-local/src/connection
|
||||
import { escalateProcessTree } from '@deepseek-ai/dsh-lsp-local/src/instance.ts'
|
||||
import type { InstanceSpec } from '@deepseek-ai/dsh-lsp-local/src/instance.ts'
|
||||
import type { LspProviderQuery, LspQueryResult } from '@deepseek-ai/dsh-lsp'
|
||||
import { spawnSubprocess } from '@deepseek-ai/dsh-subprocess-local/src/spawn.ts'
|
||||
|
||||
const fixtureServer = fileURLToPath(new URL('./fixture-server.ts', import.meta.url))
|
||||
|
||||
@@ -43,10 +44,11 @@ function makeInstance(
|
||||
initializationOptions: { init: true },
|
||||
maxMessageBytes: 16_000_000,
|
||||
maxStderrBytes: 100_000,
|
||||
pipeDrainGraceMs: 200,
|
||||
shutdownTimeoutMs: 200,
|
||||
killGraceMs: 200,
|
||||
...overrides,
|
||||
}, writer)
|
||||
}, spawnSubprocess, writer)
|
||||
live.push(instance)
|
||||
return instance
|
||||
}
|
||||
@@ -72,10 +74,11 @@ function scriptInstance(script: string, overrides: Partial<InstanceSpec> = {}):
|
||||
initializationOptions: null,
|
||||
maxMessageBytes: 16_000_000,
|
||||
maxStderrBytes: 100_000,
|
||||
pipeDrainGraceMs: 150,
|
||||
shutdownTimeoutMs: 150,
|
||||
killGraceMs: 150,
|
||||
...overrides,
|
||||
})
|
||||
}, spawnSubprocess)
|
||||
live.push(instance)
|
||||
return instance
|
||||
}
|
||||
|
||||
@@ -7,6 +7,7 @@ import { pathToFileURL, fileURLToPath } from 'node:url'
|
||||
import { Context } from 'cordis'
|
||||
import Lsp, { type LspProvider, type LspQueryRequest, type LspQueryResult } from '@deepseek-ai/dsh-lsp'
|
||||
import { deadline } from '@deepseek-ai/dsh-timeout'
|
||||
import LocalSubprocessService from '@deepseek-ai/dsh-subprocess-local'
|
||||
import * as LspLocal from '@deepseek-ai/dsh-lsp-local'
|
||||
import type { LspLocalServerConfig } from '@deepseek-ai/dsh-lsp-local'
|
||||
|
||||
@@ -45,6 +46,7 @@ async function mount(
|
||||
): Promise<Context> {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(Lsp)
|
||||
await ctx.plugin(LocalSubprocessService)
|
||||
const register = ctx.lsp.registerProvider.bind(ctx.lsp)
|
||||
const registrationSpy = captureProvider === undefined
|
||||
? undefined
|
||||
@@ -76,6 +78,7 @@ describe('lsp-local end to end over a fake server', () => {
|
||||
await writeFile(join(ws, 'a.py'), 'x = 1\n')
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(Lsp)
|
||||
await ctx.plugin(LocalSubprocessService)
|
||||
await ctx.plugin(LspLocal, {
|
||||
servers: {
|
||||
typescript: fakeServer({ LSP_FAKE_HOVER: JSON.stringify({ contents: 'ts' }) }),
|
||||
@@ -318,6 +321,7 @@ describe('lsp-local end to end over a fake server', () => {
|
||||
it('rejects at load when the command is not found', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(Lsp)
|
||||
await ctx.plugin(LocalSubprocessService)
|
||||
await expect(ctx.plugin(LspLocal, {
|
||||
servers: {
|
||||
missing: {
|
||||
|
||||
@@ -3,6 +3,7 @@ import { chmod, mkdtemp, mkdir, rm, writeFile, realpath } from 'node:fs/promises
|
||||
import { tmpdir } from 'node:os'
|
||||
import { delimiter, join } from 'node:path'
|
||||
import { Context } from 'cordis'
|
||||
import LocalSubprocessService from '@deepseek-ai/dsh-subprocess-local'
|
||||
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'
|
||||
@@ -42,6 +43,7 @@ describe('lsp-local provider resolution', () => {
|
||||
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(Lsp)
|
||||
await ctx.plugin(LocalSubprocessService)
|
||||
await expect(ctx.plugin(LspLocal, config('onpath', {
|
||||
command: 'fake-lsp',
|
||||
args: [],
|
||||
@@ -54,6 +56,7 @@ describe('lsp-local provider resolution', () => {
|
||||
it('skips empty PATH segments and fails when the command is absent', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(Lsp)
|
||||
await ctx.plugin(LocalSubprocessService)
|
||||
await expect(ctx.plugin(LspLocal, config('nope', {
|
||||
command: 'fake-lsp',
|
||||
args: [],
|
||||
@@ -67,6 +70,7 @@ describe('lsp-local provider resolution', () => {
|
||||
// Use a server that never emits results and dispose the plugin, then confirm queries are refused.
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(Lsp)
|
||||
await ctx.plugin(LocalSubprocessService)
|
||||
// Grab the provider instance by registering, then dispose the whole plugin fiber.
|
||||
const lsp = ctx.lsp
|
||||
const fiber = await ctx.plugin(LspLocal, config('disp', {
|
||||
@@ -83,6 +87,7 @@ describe('lsp-local provider resolution', () => {
|
||||
it('rejects a nonpositive teardown budget at load', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(Lsp)
|
||||
await ctx.plugin(LocalSubprocessService)
|
||||
await expect(ctx.plugin(LspLocal, config('bad-budget', {
|
||||
command: process.execPath,
|
||||
args: ['-e', ''],
|
||||
@@ -95,6 +100,7 @@ describe('lsp-local provider resolution', () => {
|
||||
it('rejects a nonpositive byte cap at load', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(Lsp)
|
||||
await ctx.plugin(LocalSubprocessService)
|
||||
await expect(ctx.plugin(LspLocal, config('bad-cap', {
|
||||
command: process.execPath,
|
||||
args: ['-e', ''],
|
||||
@@ -107,6 +113,7 @@ describe('lsp-local provider resolution', () => {
|
||||
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 ctx.plugin(LocalSubprocessService)
|
||||
await expect(ctx.plugin(LspLocal, config('bad-timer', {
|
||||
command: process.execPath,
|
||||
args: ['-e', ''],
|
||||
@@ -122,6 +129,7 @@ describe('lsp-local provider resolution', () => {
|
||||
await writeFile(notExe, 'plain text, not executable')
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(Lsp)
|
||||
await ctx.plugin(LocalSubprocessService)
|
||||
await expect(ctx.plugin(LspLocal, config('abs-bad', {
|
||||
command: notExe,
|
||||
args: [],
|
||||
@@ -133,6 +141,7 @@ describe('lsp-local provider resolution', () => {
|
||||
it('rejects an executable directory as a command at load', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(Lsp)
|
||||
await ctx.plugin(LocalSubprocessService)
|
||||
await expect(ctx.plugin(LspLocal, config('abs-directory', {
|
||||
command: ws,
|
||||
args: [],
|
||||
@@ -144,6 +153,7 @@ describe('lsp-local provider resolution', () => {
|
||||
it('rejects an empty server table at load', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(Lsp)
|
||||
await ctx.plugin(LocalSubprocessService)
|
||||
await expect(ctx.plugin(LspLocal, { servers: {} })).rejects.toThrow(/servers must contain at least one server/)
|
||||
await ctx.fiber.dispose()
|
||||
})
|
||||
@@ -151,6 +161,7 @@ describe('lsp-local provider resolution', () => {
|
||||
it('rejects an empty server id at load', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(Lsp)
|
||||
await ctx.plugin(LocalSubprocessService)
|
||||
await expect(ctx.plugin(LspLocal, config('', {
|
||||
command: process.execPath,
|
||||
extensionToLanguage: { '.ts': 'typescript' },
|
||||
@@ -161,6 +172,7 @@ describe('lsp-local provider resolution', () => {
|
||||
it('resolves every executable before publishing any provider', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(Lsp)
|
||||
await ctx.plugin(LocalSubprocessService)
|
||||
await expect(ctx.plugin(LspLocal, {
|
||||
servers: {
|
||||
valid: { command: process.execPath, extensionToLanguage: { '.ts': 'typescript' } },
|
||||
@@ -174,6 +186,7 @@ describe('lsp-local provider resolution', () => {
|
||||
it('rolls back earlier registrations when a later server conflicts', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(Lsp)
|
||||
await ctx.plugin(LocalSubprocessService)
|
||||
await expect(ctx.plugin(LspLocal, {
|
||||
servers: {
|
||||
first: { command: process.execPath, extensionToLanguage: { '.ts': 'typescript' } },
|
||||
|
||||
@@ -10,6 +10,7 @@ import { mkdtemp, mkdir, rm, writeFile, realpath } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { Context } from 'cordis'
|
||||
import LocalSubprocessService from '@deepseek-ai/dsh-subprocess-local'
|
||||
import Lsp, { type LspQueryRequest, type LspQueryResult } from '@deepseek-ai/dsh-lsp'
|
||||
import * as LspLocal from '@deepseek-ai/dsh-lsp-local'
|
||||
|
||||
@@ -52,6 +53,7 @@ beforeAll(async () => {
|
||||
|
||||
ctx = new Context()
|
||||
await ctx.plugin(Lsp)
|
||||
await ctx.plugin(LocalSubprocessService)
|
||||
await ctx.plugin(LspLocal, {
|
||||
servers: {
|
||||
typescript: {
|
||||
|
||||
Reference in New Issue
Block a user