fix(subprocess-local): fence descendant adoption on the shell's start identity
A recycled shell pid could donate an unrelated process's children to the terminal session's cleanup signalling: post-exit rescans queried processTree/processSession by numeric pid alone. The handle now captures the spawned shell's start identity at construction and adopts newly scanned members only while the root pid still carries it; already-adopted members keep their own identities, which every signal already rechecks. Regressions cover a recycled root donating an imposter child and a shell whose identity was never observable; the terminal fakes now model the root row the real /proc and ps scans include. Also from the review round: tool-pty's dependency list is re-sorted, and the LSP renderer documents the deliberate drive-letter reading of ambiguous file: URIs (display-only blast radius).
This commit is contained in:
@@ -146,6 +146,9 @@ export function renderUri(uri: string, workspaceUri: string): string {
|
|||||||
return uri
|
return uri
|
||||||
}
|
}
|
||||||
if (workspace.protocol !== 'file:') return uri
|
if (workspace.protocol !== 'file:') return uri
|
||||||
|
// A `file:` URI does not carry its world's OS, so a leading `/X:` segment is
|
||||||
|
// read as a Windows drive. A POSIX workspace literally rooted at `/c:/...`
|
||||||
|
// would mis-render (display only; edits and reads use the exact URI).
|
||||||
const drivePath = /^\/[a-z](?::|%3A)/iu
|
const drivePath = /^\/[a-z](?::|%3A)/iu
|
||||||
const windowsWorld = workspace.hostname.length > 0 || drivePath.test(workspace.pathname)
|
const windowsWorld = workspace.hostname.length > 0 || drivePath.test(workspace.pathname)
|
||||||
const targetWindowsWorld = windowsWorld && (target.hostname.length > 0 || drivePath.test(target.pathname))
|
const targetWindowsWorld = windowsWorld && (target.hostname.length > 0 || drivePath.test(target.pathname))
|
||||||
|
|||||||
@@ -50,8 +50,8 @@
|
|||||||
"@deepseek-ai/dsh-sandbox": "workspace:^",
|
"@deepseek-ai/dsh-sandbox": "workspace:^",
|
||||||
"@deepseek-ai/dsh-sandbox-policy": "workspace:^",
|
"@deepseek-ai/dsh-sandbox-policy": "workspace:^",
|
||||||
"@deepseek-ai/dsh-session": "workspace:^",
|
"@deepseek-ai/dsh-session": "workspace:^",
|
||||||
"@deepseek-ai/dsh-system-prompt": "workspace:^",
|
|
||||||
"@deepseek-ai/dsh-subprocess-local": "workspace:^",
|
"@deepseek-ai/dsh-subprocess-local": "workspace:^",
|
||||||
|
"@deepseek-ai/dsh-system-prompt": "workspace:^",
|
||||||
"@deepseek-ai/dsh-tasks": "workspace:^",
|
"@deepseek-ai/dsh-tasks": "workspace:^",
|
||||||
"@deepseek-ai/dsh-tasks-local": "workspace:^",
|
"@deepseek-ai/dsh-tasks-local": "workspace:^",
|
||||||
"@deepseek-ai/dsh-tool-tasks": "workspace:^",
|
"@deepseek-ai/dsh-tool-tasks": "workspace:^",
|
||||||
|
|||||||
@@ -43,6 +43,8 @@ export class LocalTerminalHandle implements SubprocessTerminalHandle {
|
|||||||
private cleanup: Promise<void> | undefined
|
private cleanup: Promise<void> | undefined
|
||||||
private exited = false
|
private exited = false
|
||||||
private trackedDescendants: ProcessIdentity[] = []
|
private trackedDescendants: ProcessIdentity[] = []
|
||||||
|
/** The spawned shell's start identity; scans stop adopting members once the root pid no longer carries it. */
|
||||||
|
private readonly rootIdentity: ProcessIdentity | undefined
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @param terminal - allocated node-pty process.
|
* @param terminal - allocated node-pty process.
|
||||||
@@ -55,6 +57,7 @@ export class LocalTerminalHandle implements SubprocessTerminalHandle {
|
|||||||
private readonly graceMs: number,
|
private readonly graceMs: number,
|
||||||
) {
|
) {
|
||||||
this.pid = terminal.pid
|
this.pid = terminal.pid
|
||||||
|
this.rootIdentity = inspector.processTree(this.pid).find(member => member.pid === this.pid)
|
||||||
this.done = this.outcome.promise
|
this.done = this.outcome.promise
|
||||||
this.dataDisposable = terminal.onData((data) => { this.output.write(Buffer.from(data, 'utf8')) })
|
this.dataDisposable = terminal.onData((data) => { this.output.write(Buffer.from(data, 'utf8')) })
|
||||||
this.exitDisposable = terminal.onExit(({ exitCode, signal: exitSignal }) => {
|
this.exitDisposable = terminal.onExit(({ exitCode, signal: exitSignal }) => {
|
||||||
@@ -112,10 +115,19 @@ export class LocalTerminalHandle implements SubprocessTerminalHandle {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private descendants(): ProcessIdentity[] {
|
private descendants(): ProcessIdentity[] {
|
||||||
|
// Adopt newly scanned members only while the numeric root pid provably
|
||||||
|
// still carries the spawned shell's start identity: after the shell dies,
|
||||||
|
// a recycled pid's tree and session must not donate an unrelated
|
||||||
|
// process's children to this session's signalling. Already-adopted
|
||||||
|
// members keep their own start identities, which every signal rechecks.
|
||||||
|
const tree = this.inspector.processTree(this.pid)
|
||||||
|
const root = tree.find(member => member.pid === this.pid)
|
||||||
|
const rootVerified = this.rootIdentity !== undefined
|
||||||
|
&& root !== undefined
|
||||||
|
&& root.started === this.rootIdentity.started
|
||||||
this.trackedDescendants = this.survivors(this.unionMembers(
|
this.trackedDescendants = this.survivors(this.unionMembers(
|
||||||
this.trackedDescendants,
|
this.trackedDescendants,
|
||||||
this.inspector.processTree(this.pid),
|
...rootVerified ? [tree, this.inspector.processSession(this.pid)] : [],
|
||||||
this.inspector.processSession(this.pid),
|
|
||||||
).filter(member => member.pid !== this.pid))
|
).filter(member => member.pid !== this.pid))
|
||||||
return this.trackedDescendants
|
return this.trackedDescendants
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -249,7 +249,7 @@ describe('LocalSubprocessService', () => {
|
|||||||
;(ctx.subprocess as InstanceType<typeof IsolatedLocalSubprocessService>).terminalInspector = {
|
;(ctx.subprocess as InstanceType<typeof IsolatedLocalSubprocessService>).terminalInspector = {
|
||||||
foregroundPgid: () => 123,
|
foregroundPgid: () => 123,
|
||||||
isStdinWaiting: () => false,
|
isStdinWaiting: () => false,
|
||||||
processTree: () => [{ pid: 124, started: 'child' }],
|
processTree: () => [{ pid: 123, started: 'shell' }, { pid: 124, started: 'child' }],
|
||||||
processSession: () => [],
|
processSession: () => [],
|
||||||
isAlive: identity => alive.has(identity.pid),
|
isAlive: identity => alive.has(identity.pid),
|
||||||
signalGroup: () => {},
|
signalGroup: () => {},
|
||||||
|
|||||||
@@ -52,6 +52,8 @@ class FakePty {
|
|||||||
class FakeInspector implements ProcessInspector {
|
class FakeInspector implements ProcessInspector {
|
||||||
pgid: number | undefined = 456
|
pgid: number | undefined = 456
|
||||||
waiting = false
|
waiting = false
|
||||||
|
/** The shell's own row, present like the real /proc- and ps-backed scans; tests recycle or drop it. */
|
||||||
|
root: ProcessIdentity | undefined = { pid: 123, started: 'shell' }
|
||||||
members: ProcessIdentity[] = []
|
members: ProcessIdentity[] = []
|
||||||
sessionMembers: ProcessIdentity[] = []
|
sessionMembers: ProcessIdentity[] = []
|
||||||
readonly alive = new Set<number>()
|
readonly alive = new Set<number>()
|
||||||
@@ -63,7 +65,7 @@ class FakeInspector implements ProcessInspector {
|
|||||||
|
|
||||||
foregroundPgid() { return this.pgid }
|
foregroundPgid() { return this.pgid }
|
||||||
isStdinWaiting() { return this.waiting }
|
isStdinWaiting() { return this.waiting }
|
||||||
processTree() { return this.members }
|
processTree() { return this.root === undefined ? this.members : [this.root, ...this.members] }
|
||||||
processSession() { return this.sessionMembers }
|
processSession() { return this.sessionMembers }
|
||||||
isAlive(identity: ProcessIdentity) { return this.alive.has(identity.pid) }
|
isAlive(identity: ProcessIdentity) { return this.alive.has(identity.pid) }
|
||||||
signalGroup(pgid: number, signal: SubprocessTerminalSignal) {
|
signalGroup(pgid: number, signal: SubprocessTerminalSignal) {
|
||||||
@@ -189,19 +191,50 @@ describe('LocalTerminalHandle', () => {
|
|||||||
expect(inspector.processes).toEqual([[124, 'SIGTERM']])
|
expect(inspector.processes).toEqual([[124, 'SIGTERM']])
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('does not adopt the children of a recycled shell pid', async () => {
|
||||||
|
const pty = new FakePty()
|
||||||
|
const inspector = new FakeInspector()
|
||||||
|
const handle = new LocalTerminalHandle(pty.asPty(), inspector, 10)
|
||||||
|
|
||||||
|
pty.emitExit()
|
||||||
|
const imposterChild = { pid: 999, started: 'imposter-child' }
|
||||||
|
inspector.root = { pid: 123, started: 'imposter' }
|
||||||
|
inspector.members = [imposterChild]
|
||||||
|
inspector.alive.add(imposterChild.pid)
|
||||||
|
|
||||||
|
await handle.terminate()
|
||||||
|
expect(inspector.processes).toEqual([])
|
||||||
|
})
|
||||||
|
|
||||||
|
it('adopts nothing when the shell identity was never observable', async () => {
|
||||||
|
const pty = new FakePty()
|
||||||
|
const inspector = new FakeInspector()
|
||||||
|
inspector.root = undefined
|
||||||
|
const orphan = { pid: 321, started: 'unverifiable' }
|
||||||
|
inspector.members = [orphan]
|
||||||
|
inspector.alive.add(orphan.pid)
|
||||||
|
const handle = new LocalTerminalHandle(pty.asPty(), inspector, 10)
|
||||||
|
|
||||||
|
await handle.terminate()
|
||||||
|
expect(inspector.processes).toEqual([])
|
||||||
|
expect(pty.kills).toEqual(['SIGTERM'])
|
||||||
|
})
|
||||||
|
|
||||||
it('rescans for descendants forked during TERM', async () => {
|
it('rescans for descendants forked during TERM', async () => {
|
||||||
const pty = new FakePty()
|
const pty = new FakePty()
|
||||||
const inspector = new FakeInspector()
|
const inspector = new FakeInspector()
|
||||||
|
const root = { pid: 123, started: 'shell' }
|
||||||
let reads = 0
|
let reads = 0
|
||||||
inspector.processTree = () => {
|
inspector.processTree = () => {
|
||||||
reads += 1
|
reads += 1
|
||||||
if (reads === 1) {
|
if (reads === 1) return [root]
|
||||||
inspector.alive.add(124)
|
|
||||||
return [{ pid: 124, started: 'first' }]
|
|
||||||
}
|
|
||||||
if (reads === 2) {
|
if (reads === 2) {
|
||||||
|
inspector.alive.add(124)
|
||||||
|
return [root, { pid: 124, started: 'first' }]
|
||||||
|
}
|
||||||
|
if (reads === 3) {
|
||||||
inspector.alive.add(125)
|
inspector.alive.add(125)
|
||||||
return [{ pid: 125, started: 'late' }]
|
return [root, { pid: 125, started: 'late' }]
|
||||||
}
|
}
|
||||||
return []
|
return []
|
||||||
}
|
}
|
||||||
@@ -256,9 +289,10 @@ describe('LocalTerminalHandle', () => {
|
|||||||
const pty = new FakePty()
|
const pty = new FakePty()
|
||||||
const inspector = new FakeInspector()
|
const inspector = new FakeInspector()
|
||||||
const captured = { pid: 124, started: 'captured' }
|
const captured = { pid: 124, started: 'captured' }
|
||||||
|
const root = { pid: 123, started: 'shell' }
|
||||||
let reads = 0
|
let reads = 0
|
||||||
inspector.alive.add(captured.pid)
|
inspector.alive.add(captured.pid)
|
||||||
inspector.processTree = () => reads++ === 0 ? [captured] : []
|
inspector.processTree = () => { reads += 1; return reads === 1 ? [root] : reads === 2 ? [root, captured] : [] }
|
||||||
inspector.signalProcess = (identity, signal) => {
|
inspector.signalProcess = (identity, signal) => {
|
||||||
inspector.processes.push([identity.pid, signal])
|
inspector.processes.push([identity.pid, signal])
|
||||||
if (signal === 'SIGKILL') inspector.alive.delete(identity.pid)
|
if (signal === 'SIGKILL') inspector.alive.delete(identity.pid)
|
||||||
|
|||||||
Reference in New Issue
Block a user