Merge origin/master: port the SDK stack onto the subprocess seam
Master's #660 replaced dsh-subagent-subprocess with the dsh-subprocess capability seam (ctx.subprocess + scrubbedParentEnv, tree-scoped teardown) and moved subagent-acp onto it. Convergence for this branch's packages: - The shared out-of-process provider vocabulary this branch had grown in the deleted library (NO_START_CAPABILITIES, assertPositiveFinite, cwd resolution, settleRunResult, subprocessRunHandle) moves into the subagent seam package as out-of-process.ts — it enforces subagent-seam contracts, not process mechanics, and both out-of-process backends now import it from there (subagent-acp keeps master's shape otherwise). - subagent-sdk spawns THROUGH the SDK client (the subprocess README's documented exception for SDK-managed transports) and now applies the seam's scrubbedParentEnv() + explicit-env merge in place of the deleted buildChildEnv. - sdk-client inlines the EOF→SIGTERM→SIGKILL ladder as private helpers (it runs outside any harness context, so it cannot ride ctx.subprocess). - The child harness fixture gains the now-required dsh-subprocess-local entry for bash-local; the fixture cordis.yml keeps exercising the shipped provider default.
This commit is contained in:
@@ -46,6 +46,7 @@ import type {
|
||||
} from './types.ts'
|
||||
import { SubagentRunId } from './types.ts'
|
||||
|
||||
export * from './out-of-process.ts'
|
||||
export { SubagentRunId } from './types.ts'
|
||||
export type {
|
||||
SubagentCapabilities,
|
||||
|
||||
212
packages/subagent/subagent/src/out-of-process.ts
Normal file
212
packages/subagent/subagent/src/out-of-process.ts
Normal file
@@ -0,0 +1,212 @@
|
||||
/**
|
||||
* Provider-side vocabulary for OUT-OF-PROCESS subagent backends — the pieces
|
||||
* that enforce this seam's own contracts around a child in another process:
|
||||
* the no-capabilities advertisement, timing-bound validation, child
|
||||
* working-directory resolution (config override, else the delegating parent
|
||||
* session's workspace), the never-reject result settlement, and the standard
|
||||
* run-handle publication. Backends compose these with their own wire drivers;
|
||||
* the process machinery itself (spawn, env scrub, tree-scoped teardown)
|
||||
* belongs to the `dsh-subprocess` seam.
|
||||
*
|
||||
* @module @deepseek-ai/dsh-subagent/out-of-process
|
||||
*/
|
||||
|
||||
import { accessSync, constants, statSync } from 'node:fs'
|
||||
import { isAbsolute, resolve } from 'node:path'
|
||||
import type { ContentBlock } from '@deepseek-ai/dsh-llm'
|
||||
import type { SubagentCapabilities, SubagentResult, SubagentRun, SubagentStopReason } from './types.ts'
|
||||
|
||||
/**
|
||||
* The capability advertisement of an out-of-process backend: NONE. A child in
|
||||
* another process cannot honor parent-enforced start features
|
||||
* (`outputSchema`/`maxDepth`/`toolFilter`/`persona`), so the service rejects a
|
||||
* request needing any of them before `start` runs — never accepted-then-ignored.
|
||||
*/
|
||||
export const NO_START_CAPABILITIES: SubagentCapabilities = Object.freeze({
|
||||
outputSchema: false,
|
||||
depthLimit: false,
|
||||
toolFilter: false,
|
||||
persona: false,
|
||||
})
|
||||
|
||||
/**
|
||||
* Assert a configured timing bound is a positive finite number (it bounds a
|
||||
* teardown or shutdown wait; zero, negative, or NaN would skip or wedge it).
|
||||
* @param prefix - the consuming plugin's diagnostic prefix (e.g. `subagent-acp`).
|
||||
* @param name - the config field name, for the diagnostic.
|
||||
* @param value - the configured value.
|
||||
*/
|
||||
export function assertPositiveFinite(prefix: string, name: string, value: number): void {
|
||||
if (!Number.isFinite(value) || value <= 0) {
|
||||
throw new Error(`${prefix}: ${name} must be a positive finite number`)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether `path` names an existing directory the harness can ENTER. The
|
||||
* search-permission probe matters: `statSync().isDirectory()` is true for a
|
||||
* mode-600 directory, but a subprocess cwd needs `X_OK` or spawn fails EACCES.
|
||||
*/
|
||||
function isEnterableDirectory(path: string): boolean {
|
||||
try {
|
||||
if (!statSync(path).isDirectory()) return false
|
||||
accessSync(path, constants.X_OK)
|
||||
return true
|
||||
} catch {
|
||||
// statSync/accessSync throw only filesystem access errors here
|
||||
// (ENOENT/EACCES/ENOTDIR/…), and every one of them means the path cannot
|
||||
// serve as the child's cwd.
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Assert `cwd` can actually host the child: absolute (it doubles as the
|
||||
* child's workspace identity, and a relative path would be re-anchored to the
|
||||
* server process's launch directory) and an existing directory (fail here,
|
||||
* before the process boundary, instead of as an ambiguous spawn ENOENT).
|
||||
* @param prefix - the consuming plugin's diagnostic prefix.
|
||||
* @param label - which source supplied the value, for the diagnostic.
|
||||
* @param cwd - the candidate working directory.
|
||||
* @returns `cwd`, validated.
|
||||
*/
|
||||
export function assertUsableCwd(prefix: string, label: string, cwd: string): string {
|
||||
if (!isAbsolute(cwd)) {
|
||||
throw new Error(`${prefix}: ${label} must be an absolute path: ${cwd}`)
|
||||
}
|
||||
if (!isEnterableDirectory(cwd)) {
|
||||
throw new Error(`${prefix}: ${label} is not an accessible directory: ${cwd}`)
|
||||
}
|
||||
return cwd
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate a configured `cwd` override ONCE, at plugin load: reject the empty
|
||||
* string (`path.resolve('')` is the process cwd — it would silently
|
||||
* reintroduce the launch-directory fallback this resolution removes),
|
||||
* interpret a relative path against the harness launch directory, and require
|
||||
* an enterable directory.
|
||||
* @param prefix - the consuming plugin's diagnostic prefix.
|
||||
* @param cwd - the configured override, or `undefined` when the config omits it.
|
||||
* @returns the validated absolute override, or `undefined` when omitted.
|
||||
*/
|
||||
export function validateConfiguredCwd(prefix: string, cwd: string | undefined): string | undefined {
|
||||
if (cwd === undefined) return undefined
|
||||
if (cwd === '') {
|
||||
throw new Error(`${prefix}: config cwd must not be empty — omit the key to inherit the parent session cwd`)
|
||||
}
|
||||
return assertUsableCwd(prefix, 'config cwd', resolve(cwd))
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the child's working directory at start: the deployment override
|
||||
* when configured (already validated at load), else the parent session's
|
||||
* workspace cwd (validated here, its earliest resolvable point). Fails loud
|
||||
* when neither exists — falling back to the harness process cwd would
|
||||
* silently bind the child to the server's launch directory instead of the
|
||||
* delegating session's workspace (one server process serves many sessions,
|
||||
* each with its own cwd).
|
||||
* @param prefix - the consuming plugin's diagnostic prefix.
|
||||
* @param configured - the load-validated override, or `undefined`.
|
||||
* @param parentCwd - the delegating parent session's workspace cwd, if any.
|
||||
* @returns the absolute child working directory.
|
||||
*/
|
||||
export function resolveChildCwd(prefix: string, configured: string | undefined, parentCwd: string | undefined): string {
|
||||
if (configured !== undefined) return configured
|
||||
if (parentCwd === undefined) {
|
||||
throw new Error(`${prefix}: no working directory for the child — configure \`cwd\` or delegate from a parent session that has one`)
|
||||
}
|
||||
return assertUsableCwd(prefix, 'parent session cwd', parentCwd)
|
||||
}
|
||||
|
||||
/** Normalize an unknown thrown value to an Error (the catch binding is `unknown`). */
|
||||
function toError(value: unknown): Error {
|
||||
// The rejecting surfaces (wire clients, spawn failures) only throw
|
||||
// `Error`s; the `String(value)` arm is a defensive fallback for a non-Error
|
||||
// throw the typed surfaces cannot produce.
|
||||
/* v8 ignore next */
|
||||
return value instanceof Error ? value : new Error(String(value))
|
||||
}
|
||||
|
||||
/** Inputs to {@link settleRunResult}. */
|
||||
export interface RunResultSettlement {
|
||||
/** The turn attempt (typically racing local cancellation); returns the terminal result. */
|
||||
attempt: () => Promise<SubagentResult>
|
||||
/** Snapshot of the child output streamed so far (a partial answer survives failure). */
|
||||
collectOutput: () => ContentBlock[]
|
||||
/** Whether local cancellation settled (an in-flight rejection then reads as `aborted`). */
|
||||
cancelled: () => boolean
|
||||
/** Diagnostic sink for a failure flattened to a stop reason; a throw from it is contained. */
|
||||
onError?: ((error: Error, stopReason: SubagentStopReason) => void) | undefined
|
||||
/** The request's cancellation signal (the listener is removed at settlement). */
|
||||
signal: AbortSignal
|
||||
/** The abort listener registered on {@link signal} at start. */
|
||||
onAbort: () => void
|
||||
}
|
||||
|
||||
/**
|
||||
* Settle an out-of-process run result under the seam contract: `result` never
|
||||
* rejects after publication. A rejection from the attempt resolves as
|
||||
* `aborted` when cancellation already settled locally, else it is flattened
|
||||
* to `stopReason: 'error'` through the contained diagnostic sink; the abort
|
||||
* listener is removed on every path.
|
||||
* @param parts - the attempt, output snapshot, cancellation state, sink, and signal wiring.
|
||||
* @returns the terminal result (never a rejection).
|
||||
*/
|
||||
export async function settleRunResult(parts: RunResultSettlement): Promise<SubagentResult> {
|
||||
try {
|
||||
return await parts.attempt()
|
||||
} catch (error: unknown) {
|
||||
// Cover a rejection already queued when cancellation arrives.
|
||||
if (parts.cancelled()) return { output: parts.collectOutput(), stopReason: 'aborted' }
|
||||
// Flatten post-publication transport failures while preserving diagnostics.
|
||||
try {
|
||||
parts.onError?.(toError(error), 'error')
|
||||
} catch {
|
||||
// The diagnostic sink cannot reject the run result.
|
||||
}
|
||||
return { output: parts.collectOutput(), stopReason: 'error' }
|
||||
} finally {
|
||||
parts.signal.removeEventListener('abort', parts.onAbort)
|
||||
}
|
||||
}
|
||||
|
||||
/** Inputs to {@link subprocessRunHandle}. */
|
||||
export interface SubprocessRunHandleParts {
|
||||
/** The parent-scoped run id. */
|
||||
id: SubagentRun['id']
|
||||
/** The flattened, never-rejecting result (the seam contract). */
|
||||
result: Promise<SubagentResult>
|
||||
/** The request's cancellation signal (the listener is removed on dispose). */
|
||||
signal: AbortSignal
|
||||
/** The abort listener registered on {@link signal} at start. */
|
||||
onAbort: () => void
|
||||
/** Settle local cancellation so {@link result} resolves without the child. */
|
||||
requestCancel: () => void
|
||||
/** Tear the child process down to quiescence (backend-owned ladder). */
|
||||
teardown: () => Promise<void>
|
||||
}
|
||||
|
||||
/**
|
||||
* Publish the seam run handle for an out-of-process child. `dispose()` is
|
||||
* idempotent (one memoized teardown): it removes the abort listener, settles
|
||||
* local cancellation — there is no assumption the child cooperates — and then
|
||||
* awaits the backend's teardown to actual exit.
|
||||
* @param parts - the run identity, result, cancellation wiring, and teardown.
|
||||
* @returns the seam run handle (`localAgent` is `undefined` for remote runs).
|
||||
*/
|
||||
export function subprocessRunHandle(parts: SubprocessRunHandleParts): SubagentRun {
|
||||
let disposal: Promise<void> | undefined
|
||||
return {
|
||||
id: parts.id,
|
||||
localAgent: undefined,
|
||||
result: parts.result,
|
||||
dispose(): Promise<void> {
|
||||
if (disposal !== undefined) return disposal
|
||||
parts.signal.removeEventListener('abort', parts.onAbort)
|
||||
parts.requestCancel()
|
||||
disposal = parts.teardown()
|
||||
return disposal
|
||||
},
|
||||
}
|
||||
}
|
||||
174
packages/subagent/subagent/tests/out-of-process.spec.ts
Normal file
174
packages/subagent/subagent/tests/out-of-process.spec.ts
Normal file
@@ -0,0 +1,174 @@
|
||||
/**
|
||||
* Unit coverage for the seam's out-of-process provider vocabulary: cwd
|
||||
* resolution against the real filesystem, and the settlement/handle helpers
|
||||
* under their never-reject and idempotence contracts.
|
||||
*/
|
||||
|
||||
import { chmodSync, mkdtempSync, rmSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join, relative, resolve } from 'node:path'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { SessionId } from '@deepseek-ai/dsh-session'
|
||||
import {
|
||||
assertPositiveFinite,
|
||||
assertUsableCwd,
|
||||
NO_START_CAPABILITIES,
|
||||
resolveChildCwd,
|
||||
settleRunResult,
|
||||
subprocessRunHandle,
|
||||
validateConfiguredCwd,
|
||||
} from '../src/index.ts'
|
||||
|
||||
describe('NO_START_CAPABILITIES', () => {
|
||||
it('advertises nothing and is frozen (shared by every out-of-process backend)', () => {
|
||||
expect(NO_START_CAPABILITIES).toEqual({ outputSchema: false, depthLimit: false, toolFilter: false, persona: false })
|
||||
expect(Object.isFrozen(NO_START_CAPABILITIES)).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe('assertPositiveFinite', () => {
|
||||
it('accepts positive finite bounds and rejects zero, negatives, and NaN', () => {
|
||||
expect(() => { assertPositiveFinite('p', 'graceMs', 1) }).not.toThrow()
|
||||
expect(() => { assertPositiveFinite('p', 'graceMs', 0) }).toThrow('p: graceMs must be a positive finite number')
|
||||
expect(() => { assertPositiveFinite('p', 'graceMs', -5) }).toThrow('positive finite')
|
||||
expect(() => { assertPositiveFinite('p', 'graceMs', Number.NaN) }).toThrow('positive finite')
|
||||
expect(() => { assertPositiveFinite('p', 'graceMs', Number.POSITIVE_INFINITY) }).toThrow('positive finite')
|
||||
})
|
||||
})
|
||||
|
||||
describe('child cwd resolution', () => {
|
||||
it('accepts an absolute enterable directory and rejects relative or missing paths', () => {
|
||||
expect(assertUsableCwd('p', 'config cwd', tmpdir())).toBe(tmpdir())
|
||||
expect(() => assertUsableCwd('p', 'config cwd', 'relative/path')).toThrow('must be an absolute path')
|
||||
expect(() => assertUsableCwd('p', 'config cwd', join(tmpdir(), 'dsh-no-such-dir-xyz'))).toThrow('not an accessible directory')
|
||||
})
|
||||
|
||||
// Windows ACLs do not expose the POSIX directory search-bit state this fixture creates.
|
||||
it.skipIf(process.platform === 'win32')('rejects a directory without search permission', () => {
|
||||
// statSync().isDirectory() is true for a mode-600 directory, but a
|
||||
// subprocess cwd needs SEARCH permission — spawn would fail EACCES.
|
||||
const tmp = mkdtempSync(join(tmpdir(), 'oop-noexec-'))
|
||||
chmodSync(tmp, 0o600)
|
||||
try {
|
||||
expect(() => assertUsableCwd('p', 'config cwd', tmp)).toThrow('not an accessible directory')
|
||||
} finally {
|
||||
chmodSync(tmp, 0o700)
|
||||
rmSync(tmp, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
it('validateConfiguredCwd: undefined passes through, empty fails, relative resolves at load', () => {
|
||||
expect(validateConfiguredCwd('p', undefined)).toBeUndefined()
|
||||
expect(() => validateConfiguredCwd('p', '')).toThrow('config cwd must not be empty')
|
||||
const tmp = mkdtempSync(join(tmpdir(), 'oop-rel-'))
|
||||
try {
|
||||
const relativeCwd = relative(process.cwd(), tmp)
|
||||
// Resolution is lexical against the launch directory; the probe then
|
||||
// requires the resolved path to exist and be enterable.
|
||||
expect(validateConfiguredCwd('p', relativeCwd)).toBe(resolve(relativeCwd))
|
||||
} finally {
|
||||
rmSync(tmp, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
it('resolveChildCwd: override wins, else the parent session cwd validates, else loud failure', () => {
|
||||
expect(resolveChildCwd('p', tmpdir(), undefined)).toBe(tmpdir())
|
||||
expect(resolveChildCwd('p', undefined, tmpdir())).toBe(tmpdir())
|
||||
expect(() => resolveChildCwd('p', undefined, undefined)).toThrow('no working directory for the child')
|
||||
expect(() => resolveChildCwd('p', undefined, 'relative/parent')).toThrow('parent session cwd must be an absolute path')
|
||||
})
|
||||
})
|
||||
|
||||
describe('settleRunResult', () => {
|
||||
const wiring = () => {
|
||||
const controller = new AbortController()
|
||||
const onAbort = vi.fn()
|
||||
controller.signal.addEventListener('abort', onAbort)
|
||||
return { controller, onAbort }
|
||||
}
|
||||
|
||||
it('passes a successful attempt through and removes the abort listener', async () => {
|
||||
const { controller, onAbort } = wiring()
|
||||
const result = await settleRunResult({
|
||||
attempt: async () => ({ output: [{ type: 'text', text: 'done' }], stopReason: 'completed' }),
|
||||
collectOutput: () => [],
|
||||
cancelled: () => false,
|
||||
signal: controller.signal,
|
||||
onAbort,
|
||||
})
|
||||
expect(result.stopReason).toBe('completed')
|
||||
controller.abort()
|
||||
// The listener was removed at settlement, so the abort never reaches it.
|
||||
expect(onAbort).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('reads an in-flight rejection as aborted when cancellation already settled', async () => {
|
||||
const { controller, onAbort } = wiring()
|
||||
const result = await settleRunResult({
|
||||
attempt: async () => { throw new Error('pipe torn mid-cancel') },
|
||||
collectOutput: () => [{ type: 'text', text: 'partial' }],
|
||||
cancelled: () => true,
|
||||
signal: controller.signal,
|
||||
onAbort,
|
||||
})
|
||||
expect(result).toEqual({ output: [{ type: 'text', text: 'partial' }], stopReason: 'aborted' })
|
||||
})
|
||||
|
||||
it('flattens a failure through a contained onError sink', async () => {
|
||||
const { controller, onAbort } = wiring()
|
||||
const seen: string[] = []
|
||||
const result = await settleRunResult({
|
||||
attempt: async () => { throw new Error('transport died') },
|
||||
collectOutput: () => [],
|
||||
cancelled: () => false,
|
||||
onError: (error, stopReason) => {
|
||||
seen.push(`${stopReason}:${error.message}`)
|
||||
throw new Error('sink failure must be contained')
|
||||
},
|
||||
signal: controller.signal,
|
||||
onAbort,
|
||||
})
|
||||
expect(result.stopReason).toBe('error')
|
||||
expect(seen).toEqual(['error:transport died'])
|
||||
})
|
||||
|
||||
it('flattens a failure without a sink', async () => {
|
||||
const { controller, onAbort } = wiring()
|
||||
const result = await settleRunResult({
|
||||
attempt: async () => { throw new Error('no sink configured') },
|
||||
collectOutput: () => [],
|
||||
cancelled: () => false,
|
||||
signal: controller.signal,
|
||||
onAbort,
|
||||
})
|
||||
expect(result.stopReason).toBe('error')
|
||||
})
|
||||
})
|
||||
|
||||
describe('subprocessRunHandle', () => {
|
||||
it('publishes an idempotent dispose that cancels locally and awaits teardown', async () => {
|
||||
const controller = new AbortController()
|
||||
const onAbort = vi.fn()
|
||||
controller.signal.addEventListener('abort', onAbort)
|
||||
const requestCancel = vi.fn()
|
||||
const teardown = vi.fn(() => Promise.resolve())
|
||||
const run = subprocessRunHandle({
|
||||
id: SessionId('run-1'),
|
||||
result: Promise.resolve({ output: [], stopReason: 'completed' }),
|
||||
signal: controller.signal,
|
||||
onAbort,
|
||||
requestCancel,
|
||||
teardown,
|
||||
})
|
||||
expect(run.localAgent).toBeUndefined()
|
||||
expect(String(run.id)).toBe('run-1')
|
||||
const disposal = run.dispose()
|
||||
expect(run.dispose()).toBe(disposal)
|
||||
await disposal
|
||||
expect(requestCancel).toHaveBeenCalledTimes(1)
|
||||
expect(teardown).toHaveBeenCalledTimes(1)
|
||||
controller.abort()
|
||||
// dispose removed the abort listener before cancelling.
|
||||
expect(onAbort).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user