fix(runtime): honor platform quiescence semantics
This commit is contained in:
@@ -98,6 +98,22 @@ function expectReadyForNextSend(waitReason: string): void {
|
|||||||
expect(['stdin_read', 'inferred_idle']).toContain(waitReason)
|
expect(['stdin_read', 'inferred_idle']).toContain(waitReason)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function processIsRunning(pid: number): boolean {
|
||||||
|
try {
|
||||||
|
process.kill(pid, 0)
|
||||||
|
} catch (_missingProcess) {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
if (process.platform !== 'linux') return true
|
||||||
|
try {
|
||||||
|
const stat = readFileSync(`/proc/${pid}/stat`, 'utf8')
|
||||||
|
const state = stat.slice(stat.lastIndexOf(')') + 2).split(/\s+/, 1)[0]
|
||||||
|
return !/^[ZXx]$/.test(state ?? '')
|
||||||
|
} catch (_unreadableProcEntry) {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
describe('pty-local real shell', () => {
|
describe('pty-local real shell', () => {
|
||||||
it('persists cwd and environment across sends, scrubs secrets, and closes', async () => {
|
it('persists cwd and environment across sends, scrubs secrets, and closes', async () => {
|
||||||
const previous = process.env.DSH_TEST_SECRET
|
const previous = process.env.DSH_TEST_SECRET
|
||||||
@@ -156,7 +172,7 @@ describe('pty-local real shell', () => {
|
|||||||
expect(() => process.kill(pid, 0)).toThrow()
|
expect(() => process.kill(pid, 0)).toThrow()
|
||||||
}, 10_000)
|
}, 10_000)
|
||||||
|
|
||||||
it('reaps a disowned same-session descendant after the shell exits naturally', async () => {
|
it('quiesces a disowned same-session descendant after the shell exits naturally', async () => {
|
||||||
const { ctx, root, agent } = await harness('danger-full-access')
|
const { ctx, root, agent } = await harness('danger-full-access')
|
||||||
const created = await ctx.pty.spawn(agent, { type: 'shell' })
|
const created = await ctx.pty.spawn(agent, { type: 'shell' })
|
||||||
const pidFile = join(root, 'disowned.pid')
|
const pidFile = join(root, 'disowned.pid')
|
||||||
@@ -185,7 +201,7 @@ describe('pty-local real shell', () => {
|
|||||||
}
|
}
|
||||||
expect(ctx.pty.list(agent)[0]?.status.kind).toBe('exited')
|
expect(ctx.pty.list(agent)[0]?.status.kind).toBe('exited')
|
||||||
await ctx.pty.kill(agent, created.sessionId)
|
await ctx.pty.kill(agent, created.sessionId)
|
||||||
expect(() => process.kill(childPid, 0)).toThrow()
|
expect(processIsRunning(childPid)).toBe(false)
|
||||||
} finally {
|
} finally {
|
||||||
if (pid !== undefined) {
|
if (pid !== undefined) {
|
||||||
try {
|
try {
|
||||||
|
|||||||
@@ -15,7 +15,6 @@ import { tmpdir } from 'node:os'
|
|||||||
import { join } from 'node:path'
|
import { join } from 'node:path'
|
||||||
import { setTimeout as sleepMs } from 'node:timers/promises'
|
import { setTimeout as sleepMs } from 'node:timers/promises'
|
||||||
import { scrubbedParentEnv } from '@deepseek-ai/dsh-subprocess'
|
import { scrubbedParentEnv } from '@deepseek-ai/dsh-subprocess'
|
||||||
import { MAX_TIMER_DELAY_MS } from '@deepseek-ai/dsh-timeout'
|
|
||||||
import type {
|
import type {
|
||||||
CollectedOutput,
|
CollectedOutput,
|
||||||
SubprocessCollect,
|
SubprocessCollect,
|
||||||
@@ -26,14 +25,23 @@ import type {
|
|||||||
} from '@deepseek-ai/dsh-subprocess'
|
} from '@deepseek-ai/dsh-subprocess'
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Build a child environment: explicit caller entries merge after the scrubbed
|
* Build a child environment: explicit caller entries override the scrubbed
|
||||||
* parent base. A string deliberately restores or overrides an entry; an
|
* parent base using the target platform's environment-key semantics, so a
|
||||||
* explicit `undefined` tombstone removes an ordinary ambient entry.
|
* deliberately supplied credential or current `DSH_*` fact wins over the
|
||||||
* @param extra - explicit caller entries and tombstones, merged after the scrub.
|
* scrub that dropped its ambient namesake.
|
||||||
|
* @param extra - explicit caller entries merged after the scrubbed parent.
|
||||||
* @returns the environment to hand to `spawn` for the child process.
|
* @returns the environment to hand to `spawn` for the child process.
|
||||||
*/
|
*/
|
||||||
export function childEnv(extra?: Readonly<NodeJS.ProcessEnv>): NodeJS.ProcessEnv {
|
export function childEnv(extra?: Readonly<Record<string, string>>): NodeJS.ProcessEnv {
|
||||||
return { ...scrubbedParentEnv(), ...extra }
|
const env = scrubbedParentEnv()
|
||||||
|
if (process.platform !== 'win32') return { ...env, ...extra }
|
||||||
|
let entries = Object.entries(env)
|
||||||
|
for (const [key, value] of Object.entries(extra ?? {})) {
|
||||||
|
const normalized = key.toUpperCase()
|
||||||
|
entries = entries.filter(([inherited]) => inherited.toUpperCase() !== normalized)
|
||||||
|
entries.push([key, value])
|
||||||
|
}
|
||||||
|
return Object.fromEntries(entries)
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Injectable knobs so tests can exercise spill and platform behavior deterministically. */
|
/** Injectable knobs so tests can exercise spill and platform behavior deterministically. */
|
||||||
@@ -299,12 +307,8 @@ function signalTree(
|
|||||||
* @param spec - fully resolved argv, cwd, stdio, grace, cancellation, environment.
|
* @param spec - fully resolved argv, cwd, stdio, grace, cancellation, environment.
|
||||||
* @param internals - test-only spill-directory, platform, and taskkill overrides.
|
* @param internals - test-only spill-directory, platform, and taskkill overrides.
|
||||||
* @returns live subprocess handle.
|
* @returns live subprocess handle.
|
||||||
* @throws when `graceMs` cannot be represented by one Node timer.
|
|
||||||
*/
|
*/
|
||||||
export function spawnSubprocess(spec: SubprocessSpawnSpec, internals: SpawnInternals = {}): SubprocessHandle {
|
export function spawnSubprocess(spec: SubprocessSpawnSpec, internals: SpawnInternals = {}): SubprocessHandle {
|
||||||
if (!Number.isFinite(spec.graceMs) || spec.graceMs <= 0 || spec.graceMs > MAX_TIMER_DELAY_MS) {
|
|
||||||
throw new Error(`subprocess graceMs must be a positive finite number no greater than ${MAX_TIMER_DELAY_MS}`)
|
|
||||||
}
|
|
||||||
const spillDir = internals.spillDir ?? privateSpillDir()
|
const spillDir = internals.spillDir ?? privateSpillDir()
|
||||||
const platform = internals.platform ?? process.platform
|
const platform = internals.platform ?? process.platform
|
||||||
const taskkill = internals.taskkill ?? taskkillProcessTree
|
const taskkill = internals.taskkill ?? taskkillProcessTree
|
||||||
@@ -346,9 +350,7 @@ export function spawnSubprocess(spec: SubprocessSpawnSpec, internals: SpawnInter
|
|||||||
const stdoutCollector = collectStream(outMode, child.stdout, 'stdout')
|
const stdoutCollector = collectStream(outMode, child.stdout, 'stdout')
|
||||||
const stderrCollector = collectStream(errMode, child.stderr, 'stderr')
|
const stderrCollector = collectStream(errMode, child.stderr, 'stderr')
|
||||||
|
|
||||||
let graceTimer: ReturnType<typeof setTimeout> | undefined
|
let graceTimer: NodeJS.Timeout | undefined
|
||||||
let treeExitObserved = false
|
|
||||||
let treeExitObservation: Promise<void> | undefined
|
|
||||||
let settled = false
|
let settled = false
|
||||||
|
|
||||||
// Failed spawns use pid -1 so signalling remains a no-op.
|
// Failed spawns use pid -1 so signalling remains a no-op.
|
||||||
@@ -356,9 +358,6 @@ export function spawnSubprocess(spec: SubprocessSpawnSpec, internals: SpawnInter
|
|||||||
|
|
||||||
/** Whether the detached tree's root (or POSIX group) is still alive. */
|
/** Whether the detached tree's root (or POSIX group) is still alive. */
|
||||||
const treeAlive = (): boolean => {
|
const treeAlive = (): boolean => {
|
||||||
/* v8 ignore next -- only a timer callback already queued when the observer settles can enter here;
|
|
||||||
the guard is the final defense against probing an id after its tree was confirmed absent. */
|
|
||||||
if (treeExitObserved) return false
|
|
||||||
if (pid <= 0) return false
|
if (pid <= 0) return false
|
||||||
if (platform === 'win32') {
|
if (platform === 'win32') {
|
||||||
// Windows has no group-liveness probe; the direct child's exit is the
|
// Windows has no group-liveness probe; the direct child's exit is the
|
||||||
@@ -381,40 +380,19 @@ export function spawnSubprocess(spec: SubprocessSpawnSpec, internals: SpawnInter
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
|
||||||
* Start or reuse the handle's single whole-tree exit observer. The first
|
|
||||||
* confirmed absence is a permanent no-more-signals boundary: it cancels a
|
|
||||||
* pending escalation before this process-group id can be reused.
|
|
||||||
*/
|
|
||||||
const observeTreeExit = (): Promise<void> => {
|
|
||||||
treeExitObservation ??= (async () => {
|
|
||||||
while (treeAlive()) await sleepTick()
|
|
||||||
treeExitObserved = true
|
|
||||||
if (graceTimer !== undefined) clearTimeout(graceTimer)
|
|
||||||
graceTimer = undefined
|
|
||||||
})()
|
|
||||||
return treeExitObservation
|
|
||||||
}
|
|
||||||
|
|
||||||
// The escalation's tier primitive (not on the handle — terminate() is the
|
// The escalation's tier primitive (not on the handle — terminate() is the
|
||||||
// only consumer-facing termination verb). Guards on TREE liveness, not
|
// only consumer-facing termination verb). Guards on TREE liveness, not
|
||||||
// outcome settlement: a TERM-trapping helper can outlive the settled direct
|
// outcome settlement: a TERM-trapping helper can outlive the settled direct
|
||||||
// child and must stay signalable, while a fully-dead tree (possible pid
|
// child and must stay signalable, while a fully-dead tree (possible pid
|
||||||
// reuse) must not be re-signalled by a later tier.
|
// reuse) must not be re-signalled by a later tier.
|
||||||
const kill = (sig: NodeJS.Signals): void => {
|
const kill = (sig: NodeJS.Signals): void => {
|
||||||
/* v8 ignore next -- the shared exit observer cancels the ordinary dead-tree timer;
|
|
||||||
this remains the timer/death race guard and cannot be staged deterministically. */
|
|
||||||
if (!treeAlive()) return
|
if (!treeAlive()) return
|
||||||
signalTree(platform, pid, sig, child, taskkill)
|
signalTree(platform, pid, sig, child, taskkill)
|
||||||
}
|
}
|
||||||
|
|
||||||
const terminate = (): void => {
|
const terminate = (): void => {
|
||||||
if (treeExitObserved || graceTimer !== undefined) return
|
if (graceTimer !== undefined) return // escalation already in flight
|
||||||
// Observe from the first termination tier onward, even when inherited
|
if (!treeAlive()) return
|
||||||
// pipes delay `done` and no consumer has begun its own teardown wait.
|
|
||||||
void observeTreeExit()
|
|
||||||
// oxlint-disable-next-line typescript/no-unnecessary-condition -- observer can record absence before its first await.
|
|
||||||
if (treeExitObserved) return
|
|
||||||
kill('SIGTERM')
|
kill('SIGTERM')
|
||||||
// The escalation must survive direct-child settlement — the leader dying
|
// The escalation must survive direct-child settlement — the leader dying
|
||||||
// does not mean the tree died — so settle does not clear this timer, and
|
// does not mean the tree died — so settle does not clear this timer, and
|
||||||
@@ -436,7 +414,7 @@ export function spawnSubprocess(spec: SubprocessSpawnSpec, internals: SpawnInter
|
|||||||
}
|
}
|
||||||
|
|
||||||
const done = new Promise<SubprocessOutcome>((resolve, reject) => {
|
const done = new Promise<SubprocessOutcome>((resolve, reject) => {
|
||||||
let pipeDrainTimer: ReturnType<typeof setTimeout> | undefined
|
let pipeDrainTimer: NodeJS.Timeout | undefined
|
||||||
const settle = (exitCode: number | null, signal: NodeJS.Signals | null): void => {
|
const settle = (exitCode: number | null, signal: NodeJS.Signals | null): void => {
|
||||||
if (settled) return
|
if (settled) return
|
||||||
settled = true
|
settled = true
|
||||||
@@ -459,9 +437,7 @@ export function spawnSubprocess(spec: SubprocessSpawnSpec, internals: SpawnInter
|
|||||||
// A surviving descendant that inherited a pipe must not hold the
|
// A surviving descendant that inherited a pipe must not hold the
|
||||||
// outcome open indefinitely: after exit, the same bounded grace that
|
// outcome open indefinitely: after exit, the same bounded grace that
|
||||||
// governs kills also bounds the close wait.
|
// governs kills also bounds the close wait.
|
||||||
pipeDrainTimer = setTimeout(() => {
|
pipeDrainTimer = setTimeout(() => { settle(exitCode, signal) }, spec.graceMs)
|
||||||
settle(exitCode, signal)
|
|
||||||
}, spec.graceMs)
|
|
||||||
})
|
})
|
||||||
child.on('close', settle)
|
child.on('close', settle)
|
||||||
function cleanup(): void {
|
function cleanup(): void {
|
||||||
@@ -473,23 +449,11 @@ export function spawnSubprocess(spec: SubprocessSpawnSpec, internals: SpawnInter
|
|||||||
})
|
})
|
||||||
|
|
||||||
const waitForExit = async (signal?: AbortSignal): Promise<boolean> => {
|
const waitForExit = async (signal?: AbortSignal): Promise<boolean> => {
|
||||||
const observed = observeTreeExit()
|
while (treeAlive()) {
|
||||||
if (treeExitObserved) return true
|
if (signal?.aborted) return false
|
||||||
if (signal?.aborted) return false
|
await sleepTick()
|
||||||
if (signal === undefined) {
|
|
||||||
await observed
|
|
||||||
return true
|
|
||||||
}
|
|
||||||
const aborted = Promise.withResolvers<boolean>()
|
|
||||||
const onAbort = (): void => { aborted.resolve(false) }
|
|
||||||
signal.addEventListener('abort', onAbort, { once: true })
|
|
||||||
/* v8 ignore next -- closes the event-loop race between the preceding aborted check and listener registration. */
|
|
||||||
if (signal.aborted) onAbort()
|
|
||||||
try {
|
|
||||||
return await Promise.race([observed.then(() => true), aborted.promise])
|
|
||||||
} finally {
|
|
||||||
signal.removeEventListener('abort', onAbort)
|
|
||||||
}
|
}
|
||||||
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
return {
|
return {
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ import { basename, delimiter, dirname, relative } from 'node:path'
|
|||||||
import { Context } from 'cordis'
|
import { Context } from 'cordis'
|
||||||
import LocalSubprocessService from '@deepseek-ai/dsh-subprocess-local'
|
import LocalSubprocessService from '@deepseek-ai/dsh-subprocess-local'
|
||||||
import type { SubprocessSpawnSpec, SubprocessTerminalHandle, SubprocessTerminalSpawnSpec } from '@deepseek-ai/dsh-subprocess'
|
import type { SubprocessSpawnSpec, SubprocessTerminalHandle, SubprocessTerminalSpawnSpec } from '@deepseek-ai/dsh-subprocess'
|
||||||
|
import { childEnv } from '../src/spawn.ts'
|
||||||
|
|
||||||
function spec(command: string, overrides: Partial<SubprocessSpawnSpec> = {}): SubprocessSpawnSpec {
|
function spec(command: string, overrides: Partial<SubprocessSpawnSpec> = {}): SubprocessSpawnSpec {
|
||||||
return {
|
return {
|
||||||
@@ -62,8 +63,11 @@ describe('LocalSubprocessService', () => {
|
|||||||
}).executableCandidates.bind(service)
|
}).executableCandidates.bind(service)
|
||||||
const platform = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32')
|
const platform = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32')
|
||||||
try {
|
try {
|
||||||
expect(candidates('tool', { Path: `${delimiter}/bin`, PathExt: '.EXE;.CMD' }))
|
expect(Object.keys(childEnv()).filter(key => key.toUpperCase() === 'PATH')).toHaveLength(1)
|
||||||
.toEqual(['/bin/tool.EXE', '/bin/tool.CMD'])
|
const explicit = childEnv({ Path: `${delimiter}/bin`, PathExt: '.EXE;.CMD' })
|
||||||
|
expect(Object.keys(explicit).filter(key => key.toUpperCase() === 'PATH')).toEqual(['Path'])
|
||||||
|
expect(Object.keys(explicit).filter(key => key.toUpperCase() === 'PATHEXT')).toEqual(['PathExt'])
|
||||||
|
expect(candidates('tool', explicit)).toEqual(['/bin/tool.EXE', '/bin/tool.CMD'])
|
||||||
expect(candidates('tool', { Path: '/ambient', PATH: '/explicit', PATHEXT: '.EXE' }))
|
expect(candidates('tool', { Path: '/ambient', PATH: '/explicit', PATHEXT: '.EXE' }))
|
||||||
.toEqual(['/explicit/tool.EXE'])
|
.toEqual(['/explicit/tool.EXE'])
|
||||||
expect(candidates('tool.exe', {})).toEqual([])
|
expect(candidates('tool.exe', {})).toEqual([])
|
||||||
|
|||||||
Reference in New Issue
Block a user