mirror of
https://github.com/stablyai/orca.git
synced 2026-10-17 00:03:08 +00:00
Implement graceful PTY teardown and drain window before force-kill
Agent CLIs (e.g., Claude Code, Codex) buffer transcript writes and need a drain window to flush. Immediately force-killing their PTY processes via SIGKILL drops these buffered logs, causing data loss and preventing sessions from being successfully resumed. * Add `shutdownPtyWithDrain` to request graceful exits (SIGHUP/SIGTERM or Windows `taskkill /T`), wait a bounded window, and then escalate. * Implement Windows process tree killing to gracefully close terminal hierarchies and await forced termination before cleanup to avoid EPERM. * Integrate bounded session draining during daemon shutdown, app quit, and worktree teardowns to let in-flight data flush to history. * Control worktree PTY teardown concurrency to prevent overwhelming system resources during workspace cleanup.
This commit is contained in:
@@ -24,12 +24,12 @@ describe('createDaemonFileLog', () => {
|
||||
it('appends NDJSON lines with src/ts/pid/event and terse details', () => {
|
||||
const filePath = join(dir, 'daemon.log')
|
||||
const log = createDaemonFileLog(filePath)
|
||||
log.log('startup', { protocolVersion: 18 })
|
||||
log.log('startup', { protocolVersion: 19 })
|
||||
log.log('session-created', { sessionId: 'abc', pid: 42 })
|
||||
|
||||
const lines = readLines(filePath)
|
||||
expect(lines).toHaveLength(2)
|
||||
expect(lines[0]).toMatchObject({ src: 'daemon', event: 'startup', protocolVersion: 18 })
|
||||
expect(lines[0]).toMatchObject({ src: 'daemon', event: 'startup', protocolVersion: 19 })
|
||||
expect(typeof lines[0].ts).toBe('string')
|
||||
expect(lines[0].pid).toBe(process.pid)
|
||||
expect(lines[1]).toMatchObject({ event: 'session-created', sessionId: 'abc', pid: 42 })
|
||||
|
||||
@@ -936,6 +936,11 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => {
|
||||
expect.objectContaining({ snapshotAnsi: expect.stringContaining('fresh output') })
|
||||
)
|
||||
expect(existsSync(join(historyDir, getHistorySessionDirName(id)))).toBe(true)
|
||||
historyAdapter.dispose()
|
||||
const meta = JSON.parse(
|
||||
readFileSync(join(historyDir, getHistorySessionDirName(id), 'meta.json'), 'utf-8')
|
||||
)
|
||||
expect(meta.endedAt).toBeNull()
|
||||
})
|
||||
|
||||
it('persists final take records that are not represented in the snapshot', async () => {
|
||||
@@ -959,6 +964,127 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => {
|
||||
])
|
||||
})
|
||||
|
||||
it('preserves output emitted after graceful keepHistory shutdown starts', async () => {
|
||||
historyAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir })
|
||||
|
||||
const { id } = await historyAdapter.spawn({
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
cwd: '/home/user',
|
||||
sessionId: 'sleep-graceful-tail'
|
||||
})
|
||||
const exits: { id: string; code: number }[] = []
|
||||
historyAdapter.onExit((payload) => exits.push(payload))
|
||||
|
||||
lastSubprocess._simulateData('fresh output before graceful sleep\r\n')
|
||||
vi.mocked(lastSubprocess.kill).mockImplementation(() => {
|
||||
lastSubprocess._simulateData('tail after graceful signal\r\n')
|
||||
setTimeout(() => lastSubprocess._simulateExit(0), 5)
|
||||
})
|
||||
|
||||
await historyAdapter.shutdown(id, { immediate: false, keepHistory: true })
|
||||
await waitFor(() => exits.some((exit) => exit.id === id && exit.code === 0))
|
||||
|
||||
const meta = JSON.parse(
|
||||
readFileSync(join(historyDir, getHistorySessionDirName(id), 'meta.json'), 'utf-8')
|
||||
)
|
||||
expect(meta.endedAt).toBeNull()
|
||||
|
||||
const result = await historyAdapter.spawn({
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
cwd: '/home/user',
|
||||
sessionId: id
|
||||
})
|
||||
expect(result.coldRestore?.scrollback).toContain('tail after graceful signal')
|
||||
})
|
||||
|
||||
it('does not preserve history for a respawned id after a leaked pending-exit flag', async () => {
|
||||
historyAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir })
|
||||
|
||||
const { id } = await historyAdapter.spawn({
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
cwd: '/home/user',
|
||||
sessionId: 'stale-flag-reuse'
|
||||
})
|
||||
const exits: { id: string; code: number }[] = []
|
||||
historyAdapter.onExit((payload) => exits.push(payload))
|
||||
|
||||
// Session dies on its own; the exit is fully processed.
|
||||
lastSubprocess._simulateExit(0)
|
||||
await waitFor(() => exits.length === 1)
|
||||
|
||||
// Hibernate races the death: the kill RPC hits a dead session and
|
||||
// throws, deliberately leaving the pending-exit flag set for the
|
||||
// possibly-in-flight exit event.
|
||||
await expect(
|
||||
historyAdapter.shutdown(id, { immediate: false, keepHistory: true })
|
||||
).rejects.toThrow(/Session not found/i)
|
||||
|
||||
// The renderer respawns the same persisted ptyId; its eventual real
|
||||
// exit must take the normal closeSession path (endedAt set), not the
|
||||
// preserve-history path the stale flag would trigger.
|
||||
await historyAdapter.spawn({ cols: 80, rows: 24, cwd: '/home/user', sessionId: id })
|
||||
lastSubprocess._simulateExit(0)
|
||||
await waitFor(() => exits.length === 2)
|
||||
await waitFor(() => {
|
||||
const meta = JSON.parse(
|
||||
readFileSync(join(historyDir, getHistorySessionDirName(id), 'meta.json'), 'utf-8')
|
||||
)
|
||||
return meta.endedAt !== null
|
||||
})
|
||||
})
|
||||
|
||||
it('keeps a mid-persist exit from detaching a freshly respawned session', async () => {
|
||||
historyAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir })
|
||||
|
||||
const { id } = await historyAdapter.spawn({
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
cwd: '/home/user',
|
||||
sessionId: 'wake-race'
|
||||
})
|
||||
lastSubprocess._simulateData('output before sleep\r\n')
|
||||
vi.mocked(lastSubprocess.kill).mockImplementation(() => {
|
||||
setTimeout(() => lastSubprocess._simulateExit(0), 5)
|
||||
})
|
||||
|
||||
await historyAdapter.shutdown(id, { immediate: false, keepHistory: true })
|
||||
|
||||
// Park the exit handler inside persistFinalExitHistory's disk write.
|
||||
const historyManager = historyAdapter.getHistoryManager()!
|
||||
const originalCheckpoint = historyManager.checkpoint.bind(historyManager)
|
||||
let releaseCheckpoint!: () => void
|
||||
const checkpointGate = new Promise<void>((resolve) => {
|
||||
releaseCheckpoint = resolve
|
||||
})
|
||||
const checkpointSpy = vi
|
||||
.spyOn(historyManager, 'checkpoint')
|
||||
.mockImplementation(async (sessionId, snapshot) => {
|
||||
await checkpointGate
|
||||
return originalCheckpoint(sessionId, snapshot)
|
||||
})
|
||||
await waitFor(() => checkpointSpy.mock.calls.length > 0)
|
||||
|
||||
// Wake respawns the same id while the persist is still in flight.
|
||||
const result = await historyAdapter.spawn({
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
cwd: '/home/user',
|
||||
sessionId: id
|
||||
})
|
||||
expect(result.coldRestore?.scrollback).toContain('output before sleep')
|
||||
|
||||
releaseCheckpoint()
|
||||
await new Promise((r) => setTimeout(r, 50))
|
||||
|
||||
// Why: the resumed handler must not tear down the woken generation —
|
||||
// hasPty flipping false here means checkpoints silently stopped.
|
||||
expect(historyAdapter.hasPty(id)).toBe(true)
|
||||
checkpointSpy.mockRestore()
|
||||
})
|
||||
|
||||
it('returns cold restore data when disk history has unclean shutdown', async () => {
|
||||
// Simulate a previous daemon crash: write history files without endedAt
|
||||
const sessionId = 'cold-restore-test'
|
||||
|
||||
@@ -87,6 +87,15 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
private coldRestoreCache = new Map<string, ColdRestorePayload>()
|
||||
private activeSessionIds = new Set<string>()
|
||||
private dirtySessionVersions = new Map<string, number>()
|
||||
// Why: keepHistory graceful shutdowns must leave meta.endedAt null, but only
|
||||
// after the real exit event has delivered any final daemon-side output.
|
||||
private keepHistoryShutdownsPendingExit = new Set<string>()
|
||||
// Why: handleExitEvent awaits disk writes; a wake respawn of the same
|
||||
// sessionId can land inside that window. The epoch (globally monotonic per
|
||||
// spawn) lets the resumed handler detect the newer generation and leave its
|
||||
// writer/tracking alone instead of tearing down the live woken session.
|
||||
private spawnEpochCounter = 0
|
||||
private sessionSpawnEpochs = new Map<string, number>()
|
||||
// Why: a cold-restored session is a fresh shell whose on-disk checkpoint and
|
||||
// log belong to the pre-crash session. Incremental appends would land on
|
||||
// that stale log (and be rejected by its sequence check on restore), so the
|
||||
@@ -242,6 +251,12 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
|
||||
const wasAlreadyManaged = this.activeSessionIds.has(sessionId)
|
||||
this.activeSessionIds.add(sessionId)
|
||||
this.spawnEpochCounter += 1
|
||||
this.sessionSpawnEpochs.set(sessionId, this.spawnEpochCounter)
|
||||
// Why: a leaked pending-exit flag (kill RPC raced an already-dead session)
|
||||
// must not make this new generation's eventual real exit skip closeSession
|
||||
// — that would leave endedAt null and trigger a bogus cold restore later.
|
||||
this.keepHistoryShutdownsPendingExit.delete(sessionId)
|
||||
|
||||
// Cold restore: daemon created a new session but disk history shows
|
||||
// an unclean shutdown → return saved scrollback so the renderer can
|
||||
@@ -356,18 +371,28 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
if (opts.keepHistory) {
|
||||
await this.checkpointSessions([id], { final: true, teardown: true })
|
||||
}
|
||||
await this.client.request('kill', { sessionId: id, immediate: opts.immediate ?? false })
|
||||
this.activeSessionIds.delete(id)
|
||||
this.dirtySessionVersions.delete(id)
|
||||
this.coldRestoreCache.delete(id)
|
||||
// Why: the !keepHistory close path doesn't take a final checkpoint, so a
|
||||
// session stranded in sessionsNeedingFullCheckpoint would never be cleared.
|
||||
// (Under keepHistory the final checkpoint above already cleared the flag, so
|
||||
// this is a harmless no-op there — kept unconditional to cover both paths.)
|
||||
this.sessionsNeedingFullCheckpoint.delete(id)
|
||||
this.lastFullCheckpointAt.delete(id)
|
||||
this.stopCheckpointTimerIfIdle()
|
||||
this.initialCwds.delete(id)
|
||||
const preserveHistoryOnExit = opts.keepHistory === true && opts.immediate !== true
|
||||
if (preserveHistoryOnExit) {
|
||||
this.keepHistoryShutdownsPendingExit.add(id)
|
||||
}
|
||||
try {
|
||||
await this.client.request('kill', {
|
||||
sessionId: id,
|
||||
immediate: opts.immediate ?? false,
|
||||
...(preserveHistoryOnExit && this.protocolVersion >= 19 ? { collectFinalOutput: true } : {})
|
||||
})
|
||||
} catch (err) {
|
||||
if (preserveHistoryOnExit && !isSessionNotFoundError(err)) {
|
||||
this.keepHistoryShutdownsPendingExit.delete(id)
|
||||
}
|
||||
throw err
|
||||
}
|
||||
|
||||
if (preserveHistoryOnExit) {
|
||||
return
|
||||
}
|
||||
|
||||
this.clearSessionTracking(id)
|
||||
// Why: history removal is for the "user explicitly closed this terminal"
|
||||
// path. Sleep also calls shutdown but expects scrollback to survive — wake
|
||||
// re-spawns and the cold-restore reader needs the dir intact. Caller
|
||||
@@ -376,6 +401,8 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
void this.historyManager
|
||||
.removeSession(id)
|
||||
.catch((err) => console.warn('[history] removeSession failed:', id, err))
|
||||
} else if (this.historyManager) {
|
||||
this.historyManager.detachSessionWriter(id)
|
||||
}
|
||||
|
||||
// Why: tombstone rejects reattach against a session the user explicitly
|
||||
@@ -629,6 +656,8 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
this.dirtySessionVersions.clear()
|
||||
this.lastFullCheckpointAt.clear()
|
||||
this.coldRestoreCache.clear()
|
||||
this.keepHistoryShutdownsPendingExit.clear()
|
||||
this.sessionSpawnEpochs.clear()
|
||||
this.removeEventListener?.()
|
||||
this.removeEventListener = null
|
||||
// Why: final checkpoints are written daemon-side in TerminalHost.dispose()
|
||||
@@ -665,6 +694,8 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
this.dirtySessionVersions.clear()
|
||||
this.lastFullCheckpointAt.clear()
|
||||
this.coldRestoreCache.clear()
|
||||
this.keepHistoryShutdownsPendingExit.clear()
|
||||
this.sessionSpawnEpochs.clear()
|
||||
this.removeEventListener?.()
|
||||
this.removeEventListener = null
|
||||
this.client.disconnect()
|
||||
@@ -690,6 +721,22 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
}
|
||||
}
|
||||
|
||||
private clearSessionTracking(sessionId: string): void {
|
||||
this.activeSessionIds.delete(sessionId)
|
||||
this.dirtySessionVersions.delete(sessionId)
|
||||
this.coldRestoreCache.delete(sessionId)
|
||||
// Why: exited/closed sessions can never be checkpointed again; keep the
|
||||
// pending full-checkpoint and cooldown sets bounded across reused ids.
|
||||
this.sessionsNeedingFullCheckpoint.delete(sessionId)
|
||||
this.lastFullCheckpointAt.delete(sessionId)
|
||||
this.keepHistoryShutdownsPendingExit.delete(sessionId)
|
||||
// Why: the counter is globally monotonic, so deleting here keeps the map
|
||||
// bounded without letting a reused id collide with a stale epoch.
|
||||
this.sessionSpawnEpochs.delete(sessionId)
|
||||
this.stopCheckpointTimerIfIdle()
|
||||
this.initialCwds.delete(sessionId)
|
||||
}
|
||||
|
||||
private scheduleCheckpointTimer(): void {
|
||||
if (
|
||||
this.checkpointTimer ||
|
||||
@@ -1013,30 +1060,53 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
listener({ id: event.sessionId, data: event.payload.data })
|
||||
}
|
||||
} else if (event.event === 'exit') {
|
||||
this.activeSessionIds.delete(event.sessionId)
|
||||
this.dirtySessionVersions.delete(event.sessionId)
|
||||
this.coldRestoreCache.delete(event.sessionId)
|
||||
// Why: an exited session can never be checkpointed again, so its pending
|
||||
// full-checkpoint flag is dead state. Without this, a cold-restored
|
||||
// session that exits before its first checkpoint leaks a permanent entry.
|
||||
this.sessionsNeedingFullCheckpoint.delete(event.sessionId)
|
||||
// Why: a reused sessionId (renderer respawns a persisted ptyId) must
|
||||
// not inherit the dead session's snapshot cooldown.
|
||||
this.lastFullCheckpointAt.delete(event.sessionId)
|
||||
this.stopCheckpointTimerIfIdle()
|
||||
if (this.historyManager) {
|
||||
void this.historyManager
|
||||
.closeSession(event.sessionId, event.payload.code)
|
||||
.catch((err) => console.warn('[history] closeSession failed:', event.sessionId, err))
|
||||
}
|
||||
this.initialCwds.delete(event.sessionId)
|
||||
// oxlint-disable-next-line unicorn/no-useless-spread -- copy-safe: listeners may unsubscribe during iteration
|
||||
for (const listener of [...this.exitListeners]) {
|
||||
listener({ id: event.sessionId, code: event.payload.code })
|
||||
}
|
||||
void this.handleExitEvent(event)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
private async handleExitEvent(event: Extract<DaemonEvent, { event: 'exit' }>): Promise<void> {
|
||||
const preserveHistory = this.keepHistoryShutdownsPendingExit.has(event.sessionId)
|
||||
const epochAtExit = this.sessionSpawnEpochs.get(event.sessionId) ?? 0
|
||||
if (preserveHistory && this.historyManager) {
|
||||
try {
|
||||
await this.persistFinalExitHistory(event.sessionId, event.payload.finalOutput)
|
||||
} catch (err) {
|
||||
console.warn('[history] final exit checkpoint failed:', event.sessionId, err)
|
||||
} finally {
|
||||
// Why: only detach if no newer spawn re-registered this id's writer
|
||||
// while the persist awaited — detaching the woken generation's writer
|
||||
// would silently stop its persistence.
|
||||
if ((this.sessionSpawnEpochs.get(event.sessionId) ?? 0) === epochAtExit) {
|
||||
this.historyManager.detachSessionWriter(event.sessionId)
|
||||
}
|
||||
}
|
||||
} else if (this.historyManager) {
|
||||
void this.historyManager
|
||||
.closeSession(event.sessionId, event.payload.code)
|
||||
.catch((err) => console.warn('[history] closeSession failed:', event.sessionId, err))
|
||||
}
|
||||
if ((this.sessionSpawnEpochs.get(event.sessionId) ?? 0) === epochAtExit) {
|
||||
this.clearSessionTracking(event.sessionId)
|
||||
}
|
||||
// oxlint-disable-next-line unicorn/no-useless-spread -- copy-safe: listeners may unsubscribe during iteration
|
||||
for (const listener of [...this.exitListeners]) {
|
||||
listener({ id: event.sessionId, code: event.payload.code })
|
||||
}
|
||||
}
|
||||
|
||||
private async persistFinalExitHistory(
|
||||
sessionId: string,
|
||||
finalOutput: TakePendingOutputResult | undefined
|
||||
): Promise<void> {
|
||||
if (!this.historyManager || !finalOutput?.snapshot) {
|
||||
return
|
||||
}
|
||||
await this.historyManager.checkpoint(sessionId, finalOutput.snapshot)
|
||||
if (finalOutput.records.length > 0) {
|
||||
await this.historyManager.appendIncrements(sessionId, finalOutput.seq, finalOutput.records)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Why: ENOENT/ECONNREFUSED with syscall 'connect' mean the socket is
|
||||
@@ -1056,3 +1126,7 @@ function isDaemonGoneError(err: unknown): boolean {
|
||||
const msg = err.message
|
||||
return msg === 'Connection lost' || msg === 'Not connected'
|
||||
}
|
||||
|
||||
function isSessionNotFoundError(err: unknown): boolean {
|
||||
return err instanceof Error && /Session not found/i.test(err.message)
|
||||
}
|
||||
|
||||
@@ -145,6 +145,20 @@ describe('DaemonPtyRouter', () => {
|
||||
expect(current.hasPty).not.toHaveBeenCalledWith('legacy-session')
|
||||
})
|
||||
|
||||
it('keeps the routed adapter between graceful shutdown and force escalation', async () => {
|
||||
const current = createAdapter('current')
|
||||
const legacy = createAdapter('legacy', ['legacy-session'])
|
||||
const router = new DaemonPtyRouter({ current, legacy: [legacy] })
|
||||
|
||||
await router.discoverLegacySessions()
|
||||
await router.shutdown('legacy-session', { immediate: false })
|
||||
await router.shutdown('legacy-session', { immediate: true })
|
||||
|
||||
expect(legacy.shutdown).toHaveBeenNthCalledWith(1, 'legacy-session', { immediate: false })
|
||||
expect(legacy.shutdown).toHaveBeenNthCalledWith(2, 'legacy-session', { immediate: true })
|
||||
expect(current.shutdown).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('fails listProcesses closed when any routed adapter cannot list sessions', async () => {
|
||||
const current = createAdapter('current', ['current-session'])
|
||||
const legacy = createAdapter('legacy', ['legacy-session'])
|
||||
|
||||
@@ -84,7 +84,9 @@ export class DaemonPtyRouter implements IPtyProvider {
|
||||
// lived on a legacy adapter (different protocolVersion), the wake-side
|
||||
// createOrAttach lands on the wrong adapter and creates a fresh session,
|
||||
// losing the cold-restore from the legacy adapter's history dir.
|
||||
if (!opts.keepHistory) {
|
||||
// Graceful shutdown is only the first half of drain-then-force teardown;
|
||||
// keep the route so timeout escalation still targets the owning daemon.
|
||||
if (opts.immediate === true && !opts.keepHistory) {
|
||||
this.sessionAdapters.delete(id)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -21,6 +21,11 @@ import {
|
||||
type DaemonRequest
|
||||
} from './types'
|
||||
|
||||
/** Bounded graceful-exit window for live sessions on daemon shutdown, so
|
||||
* agent CLIs can flush transcripts before dispose() force-kills leftovers.
|
||||
* Kept short: daemon restart / app quit wait on this. */
|
||||
const SHUTDOWN_SESSION_DRAIN_MS = 2_000
|
||||
|
||||
export type DaemonServerOptions = {
|
||||
socketPath: string
|
||||
tokenPath: string
|
||||
@@ -97,7 +102,8 @@ export class DaemonServer {
|
||||
}
|
||||
|
||||
async shutdown(): Promise<void> {
|
||||
this.host.dispose()
|
||||
await this.host.drainLiveSessions(SHUTDOWN_SESSION_DRAIN_MS)
|
||||
await this.host.disposeAndWaitForForceKill({ checkpointDirtyOnly: true })
|
||||
this.streamDataBatcher.clear()
|
||||
|
||||
for (const [, client] of this.clients) {
|
||||
@@ -305,7 +311,7 @@ export class DaemonServer {
|
||||
flushMaxChars: DaemonServer.INTERACTIVE_OUTPUT_MAX_CHARS
|
||||
})
|
||||
},
|
||||
onExit: (code) => {
|
||||
onExit: (code, finalOutput) => {
|
||||
// Why: exit tears down renderer handlers; flush final output first
|
||||
// so the last few milliseconds of PTY data are not stranded.
|
||||
this.log.log('session-exited', { sessionId: p.sessionId, code })
|
||||
@@ -317,7 +323,7 @@ export class DaemonServer {
|
||||
type: 'event',
|
||||
event: 'exit',
|
||||
sessionId: p.sessionId,
|
||||
payload: { code }
|
||||
payload: { code, ...(finalOutput ? { finalOutput } : {}) }
|
||||
})
|
||||
)
|
||||
}
|
||||
@@ -369,7 +375,14 @@ export class DaemonServer {
|
||||
sessionId: request.payload.sessionId,
|
||||
immediate: request.payload.immediate === true
|
||||
})
|
||||
this.host.kill(request.payload.sessionId, { immediate: request.payload.immediate })
|
||||
if (request.payload.immediate === true) {
|
||||
await this.host.killAndWait(request.payload.sessionId, { immediate: true })
|
||||
} else {
|
||||
this.host.kill(request.payload.sessionId, {
|
||||
immediate: request.payload.immediate,
|
||||
collectFinalOutput: request.payload.collectFinalOutput === true
|
||||
})
|
||||
}
|
||||
return {}
|
||||
|
||||
case 'signal':
|
||||
@@ -428,7 +441,10 @@ export class DaemonServer {
|
||||
killSessions: request.payload.killSessions === true
|
||||
})
|
||||
if (request.payload.killSessions) {
|
||||
this.host.dispose()
|
||||
// Why: drain before dispose so agent CLIs flush transcripts; the
|
||||
// client's shutdown RPC timeout (30s) comfortably covers the window.
|
||||
await this.host.drainLiveSessions(SHUTDOWN_SESSION_DRAIN_MS)
|
||||
await this.host.disposeAndWaitForForceKill({ checkpointDirtyOnly: true })
|
||||
}
|
||||
process.nextTick(() => this.shutdown())
|
||||
return {}
|
||||
|
||||
@@ -151,6 +151,20 @@ describe('DegradedDaemonPtyProvider', () => {
|
||||
expect(fallback.write).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('keeps the owning provider between graceful shutdown and force escalation', async () => {
|
||||
const current = createDaemonAdapter('daemon', ['daemon-session'])
|
||||
const fallback = createProvider('fallback')
|
||||
const provider = new DegradedDaemonPtyProvider({ current, legacy: [], fallback })
|
||||
|
||||
await provider.discoverDaemonSessions()
|
||||
await provider.shutdown('daemon-session', { immediate: false })
|
||||
await provider.shutdown('daemon-session', { immediate: true })
|
||||
|
||||
expect(current.shutdown).toHaveBeenNthCalledWith(1, 'daemon-session', { immediate: false })
|
||||
expect(current.shutdown).toHaveBeenNthCalledWith(2, 'daemon-session', { immediate: true })
|
||||
expect(fallback.shutdown).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('forwards replay output from fallback and daemon providers', () => {
|
||||
const current = createDaemonAdapter('daemon')
|
||||
const fallback = createProvider('fallback')
|
||||
|
||||
@@ -95,7 +95,10 @@ export class DegradedDaemonPtyProvider implements IPtyProvider {
|
||||
|
||||
async shutdown(id: string, opts: { immediate?: boolean; keepHistory?: boolean }): Promise<void> {
|
||||
await this.providerFor(id).shutdown(id, opts)
|
||||
if (!opts.keepHistory) {
|
||||
// Why: graceful shutdown is followed by a forced escalation when the PTY
|
||||
// ignores the signal; dropping ownership here would route that escalation
|
||||
// to the fallback provider instead of the daemon/local provider that owns it.
|
||||
if (opts.immediate === true && !opts.keepHistory) {
|
||||
this.sessionProviders.delete(id)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -308,6 +308,13 @@ export class HistoryManager {
|
||||
})
|
||||
}
|
||||
|
||||
// Why: hibernated terminals must leave meta.endedAt null for cold restore, but
|
||||
// the dead session's writer must not be closed later by dispose().
|
||||
detachSessionWriter(sessionId: string): void {
|
||||
this.writers.delete(sessionId)
|
||||
this.disabledSessions.delete(sessionId)
|
||||
}
|
||||
|
||||
isSessionDisabled(sessionId: string): boolean {
|
||||
return this.disabledSessions.has(sessionId)
|
||||
}
|
||||
|
||||
@@ -59,6 +59,23 @@ vi.mock('../providers/agent-foreground-process', () => ({
|
||||
resolveAgentForegroundProcess: resolveAgentForegroundProcessMock
|
||||
}))
|
||||
|
||||
// Why: never spawn a real `taskkill` from tests — on a Windows runner that
|
||||
// would signal an arbitrary live pid.
|
||||
const {
|
||||
requestWindowsProcessTreeExitMock,
|
||||
forceKillWindowsProcessTreeMock,
|
||||
waitForWindowsProcessTreeForceKillMock
|
||||
} = vi.hoisted(() => ({
|
||||
requestWindowsProcessTreeExitMock: vi.fn(),
|
||||
forceKillWindowsProcessTreeMock: vi.fn(),
|
||||
waitForWindowsProcessTreeForceKillMock: vi.fn()
|
||||
}))
|
||||
vi.mock('../pty/windows-process-tree-kill', () => ({
|
||||
requestWindowsProcessTreeExit: requestWindowsProcessTreeExitMock,
|
||||
forceKillWindowsProcessTree: forceKillWindowsProcessTreeMock,
|
||||
waitForWindowsProcessTreeForceKill: waitForWindowsProcessTreeForceKillMock
|
||||
}))
|
||||
|
||||
import { createPtySubprocess, checkPtySpawnHealth } from './pty-subprocess'
|
||||
|
||||
const ORCA_SHELL_WRAPPER_ENV = [
|
||||
@@ -106,6 +123,10 @@ describe('createPtySubprocess', () => {
|
||||
beforeEach(() => {
|
||||
spawnMock.mockReset()
|
||||
isPwshAvailableMock.mockReset()
|
||||
requestWindowsProcessTreeExitMock.mockReset()
|
||||
forceKillWindowsProcessTreeMock.mockReset()
|
||||
waitForWindowsProcessTreeForceKillMock.mockReset()
|
||||
waitForWindowsProcessTreeForceKillMock.mockResolvedValue(undefined)
|
||||
resolveAgentForegroundProcessMock.mockReset()
|
||||
resolveAgentForegroundProcessMock.mockImplementation(
|
||||
async (_pid: number, fallbackProcess: string | null) => fallbackProcess
|
||||
@@ -2353,8 +2374,31 @@ describe('createPtySubprocess', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('dispose() on Windows skips destroy after node-pty kill()', () => {
|
||||
const proc = mockPtyProcess() as ReturnType<typeof mockPtyProcess> & {
|
||||
it('kill() on Windows requests a graceful tree exit and leaves destroy() live', () => {
|
||||
const proc = mockPtyProcess(123456) as ReturnType<typeof mockPtyProcess> & {
|
||||
destroy: ReturnType<typeof vi.fn>
|
||||
}
|
||||
proc.destroy = vi.fn()
|
||||
spawnMock.mockReturnValue(proc)
|
||||
const origPlatform = Object.getOwnPropertyDescriptor(process, 'platform')
|
||||
Object.defineProperty(process, 'platform', { value: 'win32' })
|
||||
try {
|
||||
const handle = createPtySubprocess({ sessionId: 'test', cols: 80, rows: 24 })
|
||||
handle.kill()
|
||||
// Why: the graceful path must NOT tear the ConPTY down — that would
|
||||
// collapse the drain window agent CLIs need to flush transcripts.
|
||||
expect(requestWindowsProcessTreeExitMock).toHaveBeenCalledWith(123456)
|
||||
expect(proc.kill).not.toHaveBeenCalled()
|
||||
handle.dispose()
|
||||
// dispose() still releases the ConPTY via destroy() (which is kill()).
|
||||
expect(proc.destroy).toHaveBeenCalledOnce()
|
||||
} finally {
|
||||
restorePlatform(origPlatform)
|
||||
}
|
||||
})
|
||||
|
||||
it('dispose() on Windows skips destroy after forceKill kills the tree', () => {
|
||||
const proc = mockPtyProcess(123456) as ReturnType<typeof mockPtyProcess> & {
|
||||
destroy: ReturnType<typeof vi.fn>
|
||||
}
|
||||
proc.destroy = vi.fn(() => proc.kill())
|
||||
@@ -2363,8 +2407,9 @@ describe('createPtySubprocess', () => {
|
||||
Object.defineProperty(process, 'platform', { value: 'win32' })
|
||||
try {
|
||||
const handle = createPtySubprocess({ sessionId: 'test', cols: 80, rows: 24 })
|
||||
handle.kill()
|
||||
handle.forceKill()
|
||||
handle.dispose()
|
||||
expect(forceKillWindowsProcessTreeMock).toHaveBeenCalledWith(123456)
|
||||
expect(proc.kill).toHaveBeenCalledOnce()
|
||||
expect(proc.destroy).not.toHaveBeenCalled()
|
||||
} finally {
|
||||
@@ -2372,26 +2417,37 @@ describe('createPtySubprocess', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('dispose() on Windows skips destroy after forceKill falls back to node-pty kill()', () => {
|
||||
const proc = mockPtyProcess(123456) as ReturnType<typeof mockPtyProcess> & {
|
||||
destroy: ReturnType<typeof vi.fn>
|
||||
}
|
||||
proc.destroy = vi.fn(() => proc.kill())
|
||||
it('forceKillAndWait() on Windows waits for taskkill before closing ConPTY', async () => {
|
||||
const proc = mockPtyProcess(123456)
|
||||
spawnMock.mockReturnValue(proc)
|
||||
const killSpy = vi.spyOn(process, 'kill').mockImplementation(() => {
|
||||
throw new Error('already gone')
|
||||
})
|
||||
let releaseForceKill = (): void => {
|
||||
throw new Error('force kill waiter was not installed')
|
||||
}
|
||||
waitForWindowsProcessTreeForceKillMock.mockImplementation(
|
||||
() =>
|
||||
new Promise<void>((resolve) => {
|
||||
releaseForceKill = resolve
|
||||
})
|
||||
)
|
||||
const origPlatform = Object.getOwnPropertyDescriptor(process, 'platform')
|
||||
Object.defineProperty(process, 'platform', { value: 'win32' })
|
||||
try {
|
||||
const handle = createPtySubprocess({ sessionId: 'test', cols: 80, rows: 24 })
|
||||
handle.forceKill()
|
||||
handle.dispose()
|
||||
expect(killSpy).toHaveBeenCalledWith(123456, 'SIGKILL')
|
||||
let settled = false
|
||||
const forceKill = handle.forceKillAndWait?.().then(() => {
|
||||
settled = true
|
||||
})
|
||||
await Promise.resolve()
|
||||
|
||||
expect(waitForWindowsProcessTreeForceKillMock).toHaveBeenCalledWith(123456)
|
||||
expect(proc.kill).not.toHaveBeenCalled()
|
||||
expect(settled).toBe(false)
|
||||
|
||||
releaseForceKill()
|
||||
await forceKill
|
||||
expect(proc.kill).toHaveBeenCalledOnce()
|
||||
expect(proc.destroy).not.toHaveBeenCalled()
|
||||
expect(settled).toBe(true)
|
||||
} finally {
|
||||
killSpy.mockRestore()
|
||||
restorePlatform(origPlatform)
|
||||
}
|
||||
})
|
||||
|
||||
@@ -29,6 +29,11 @@ import {
|
||||
} from '../providers/windows-shell-fallback-chain'
|
||||
import { isPwshAvailable } from '../pwsh'
|
||||
import { isHostCodexHomeForWsl, isWslCodexHomeForHost } from '../pty/codex-home-wsl-env'
|
||||
import {
|
||||
forceKillWindowsProcessTree,
|
||||
requestWindowsProcessTreeExit,
|
||||
waitForWindowsProcessTreeForceKill
|
||||
} from '../pty/windows-process-tree-kill'
|
||||
import { removeInheritedNoColor } from '../pty/terminal-color-env'
|
||||
import { removeAppImageRuntimeEnv } from '../pty/appimage-terminal-env'
|
||||
import { parseWslPath } from '../wsl'
|
||||
@@ -1082,6 +1087,14 @@ export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandl
|
||||
if (dead) {
|
||||
return
|
||||
}
|
||||
// Why: on Windows node-pty's kill() tears the ConPTY down immediately —
|
||||
// no drain window for agent CLIs flushing transcripts. Ask the tree to
|
||||
// close instead; session.ts's KILL_TIMEOUT_MS fallback force-kills any
|
||||
// process that refuses.
|
||||
if (process.platform === 'win32') {
|
||||
requestWindowsProcessTreeExit(proc.pid)
|
||||
return
|
||||
}
|
||||
try {
|
||||
nodePtyKillIssued = true
|
||||
proc.kill()
|
||||
@@ -1097,6 +1110,44 @@ export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandl
|
||||
if (dead) {
|
||||
return
|
||||
}
|
||||
if (process.platform === 'win32') {
|
||||
// Why: taskkill /T /F reaps the whole console tree (process.kill hits
|
||||
// only the shell pid); proc.kill() then releases the ConPTY itself.
|
||||
forceKillWindowsProcessTree(proc.pid)
|
||||
try {
|
||||
nodePtyKillIssued = true
|
||||
proc.kill()
|
||||
} catch {
|
||||
// Process may already be dead
|
||||
}
|
||||
return
|
||||
}
|
||||
try {
|
||||
process.kill(proc.pid, 'SIGKILL')
|
||||
} catch {
|
||||
try {
|
||||
nodePtyKillIssued = true
|
||||
proc.kill()
|
||||
} catch {
|
||||
// Process may already be dead
|
||||
}
|
||||
}
|
||||
},
|
||||
forceKillAndWait: async () => {
|
||||
if (dead) {
|
||||
return
|
||||
}
|
||||
if (process.platform === 'win32') {
|
||||
onExitCb = null
|
||||
await waitForWindowsProcessTreeForceKill(proc.pid)
|
||||
try {
|
||||
nodePtyKillIssued = true
|
||||
proc.kill()
|
||||
} catch {
|
||||
// Process may already be dead
|
||||
}
|
||||
return
|
||||
}
|
||||
try {
|
||||
process.kill(proc.pid, 'SIGKILL')
|
||||
} catch {
|
||||
|
||||
@@ -48,6 +48,9 @@ export type SubprocessHandle = {
|
||||
clear?(): void
|
||||
kill(): void
|
||||
forceKill(): void
|
||||
/** Same force-kill signal as forceKill(), but resolves after platform-specific
|
||||
* tree-kill work that callers need before deleting worktree files. */
|
||||
forceKillAndWait?(): Promise<void>
|
||||
signal(sig: string): void
|
||||
onData(cb: (data: string) => void): void
|
||||
onExit(cb: (code: number) => void): void
|
||||
@@ -77,7 +80,7 @@ export type SessionOptions = {
|
||||
type AttachedClient = {
|
||||
token: symbol
|
||||
onData: (data: string) => void
|
||||
onExit: (code: number) => void
|
||||
onExit: (code: number, finalOutput?: TakePendingOutputResult) => void
|
||||
}
|
||||
|
||||
export class Session {
|
||||
@@ -101,6 +104,7 @@ export class Session {
|
||||
private pendingOutputBytes = 0
|
||||
private pendingOutputOverflowed = false
|
||||
private pendingOutputSeq = 0
|
||||
private captureFinalOutput = false
|
||||
|
||||
constructor(opts: SessionOptions) {
|
||||
this.sessionId = opts.sessionId
|
||||
@@ -203,6 +207,18 @@ export class Session {
|
||||
}, KILL_TIMEOUT_MS)
|
||||
}
|
||||
|
||||
requestFinalOutputOnExit(): void {
|
||||
this.captureFinalOutput = true
|
||||
}
|
||||
|
||||
hasPendingOutputForCheckpoint(): boolean {
|
||||
return (
|
||||
this.pendingOutputOverflowed ||
|
||||
this.pendingOutputRecords.length > 0 ||
|
||||
this.shellReadyScanState !== null
|
||||
)
|
||||
}
|
||||
|
||||
signal(sig: string): void {
|
||||
if (this._state === 'exited') {
|
||||
return
|
||||
@@ -210,7 +226,10 @@ export class Session {
|
||||
this.subprocess.signal(sig)
|
||||
}
|
||||
|
||||
attachClient(client: { onData: (data: string) => void; onExit: (code: number) => void }): symbol {
|
||||
attachClient(client: {
|
||||
onData: (data: string) => void
|
||||
onExit: (code: number, finalOutput?: TakePendingOutputResult) => void
|
||||
}): symbol {
|
||||
const token = Symbol('attach')
|
||||
this.attachedClients.push({ token, ...client })
|
||||
return token
|
||||
@@ -404,6 +423,23 @@ export class Session {
|
||||
this.emulator.dispose()
|
||||
}
|
||||
|
||||
async forceKillAndDisposeSubprocessAndWait(): Promise<void> {
|
||||
try {
|
||||
if (this.subprocess.forceKillAndWait) {
|
||||
await this.subprocess.forceKillAndWait()
|
||||
} else {
|
||||
this.subprocess.forceKill()
|
||||
}
|
||||
} catch {
|
||||
/* swallow — child may already be gone */
|
||||
}
|
||||
this._exitCode = -1
|
||||
this._isTerminating = false
|
||||
this.#teardownSubprocess()
|
||||
this._state = 'exited'
|
||||
this.emulator.dispose()
|
||||
}
|
||||
|
||||
/** Private: shared teardown helper called by dispose(), forceDispose(), and
|
||||
* forceKillAndDisposeSubprocess(). Flips `_disposed`, clears pending timers,
|
||||
* and forwards to subprocess.dispose() exactly once. Does NOT set `_state` —
|
||||
@@ -497,7 +533,10 @@ export class Session {
|
||||
|
||||
this._exitCode = code
|
||||
this._state = 'exited'
|
||||
this.releaseHeldShellReadyBytes()
|
||||
const finalOutput = this.takeFinalOutputIfRequested()
|
||||
if (!finalOutput) {
|
||||
this.releaseHeldShellReadyBytes()
|
||||
}
|
||||
|
||||
if (this.killTimer) {
|
||||
clearTimeout(this.killTimer)
|
||||
@@ -523,7 +562,11 @@ export class Session {
|
||||
}
|
||||
|
||||
for (const client of this.attachedClients) {
|
||||
client.onExit(code)
|
||||
if (finalOutput) {
|
||||
client.onExit(code, finalOutput)
|
||||
} else {
|
||||
client.onExit(code)
|
||||
}
|
||||
}
|
||||
|
||||
// Why: hand off to the owner's reaper so the emulator is disposed and the
|
||||
@@ -545,6 +588,14 @@ export class Session {
|
||||
return heldBytes
|
||||
}
|
||||
|
||||
private takeFinalOutputIfRequested(): TakePendingOutputResult | null {
|
||||
if (!this.captureFinalOutput) {
|
||||
return null
|
||||
}
|
||||
this.captureFinalOutput = false
|
||||
return this.takePendingOutput(true, { teardownSnapshot: true })
|
||||
}
|
||||
|
||||
private transitionToReady(postMarkerBytesObserved = false): void {
|
||||
this._shellState = 'ready'
|
||||
this.shellReadyScanState = null
|
||||
@@ -592,6 +643,7 @@ export class Session {
|
||||
} catch {
|
||||
/* already dead */
|
||||
}
|
||||
const finalOutput = this.takeFinalOutputIfRequested()
|
||||
this._exitCode = -1
|
||||
this._isTerminating = false
|
||||
|
||||
@@ -605,7 +657,11 @@ export class Session {
|
||||
this.emulator.dispose()
|
||||
|
||||
for (const client of clients) {
|
||||
client.onExit(-1)
|
||||
if (finalOutput) {
|
||||
client.onExit(-1, finalOutput)
|
||||
} else {
|
||||
client.onExit(-1)
|
||||
}
|
||||
}
|
||||
|
||||
// Why: reap from the host map on the kill-timeout path too (emulator already
|
||||
|
||||
@@ -0,0 +1,141 @@
|
||||
import type { TakePendingOutputResult, TerminalSnapshot } from './types'
|
||||
import type { Session } from './session'
|
||||
|
||||
export type ShutdownCallbacks = {
|
||||
onFinalCheckpoint?: (
|
||||
sessionId: string,
|
||||
snapshot: TerminalSnapshot,
|
||||
records: TakePendingOutputResult['records']
|
||||
) => void
|
||||
}
|
||||
|
||||
const DEFAULT_MAX_TOMBSTONES = 1000
|
||||
|
||||
/** Owns TerminalHost's teardown surface: killed-session tombstones, final
|
||||
* checkpoints, the bounded graceful drain, and force-kill disposal. */
|
||||
export class TerminalHostShutdown {
|
||||
private sessions: Map<string, Session>
|
||||
private killedTombstones = new Map<string, number>()
|
||||
private maxTombstones: number
|
||||
private onFinalCheckpoint: ShutdownCallbacks['onFinalCheckpoint']
|
||||
|
||||
constructor(
|
||||
sessions: Map<string, Session>,
|
||||
opts: { maxTombstones?: number; onFinalCheckpoint?: ShutdownCallbacks['onFinalCheckpoint'] }
|
||||
) {
|
||||
this.sessions = sessions
|
||||
this.maxTombstones = opts.maxTombstones ?? DEFAULT_MAX_TOMBSTONES
|
||||
this.onFinalCheckpoint = opts.onFinalCheckpoint
|
||||
}
|
||||
|
||||
recordTombstone(sessionId: string): void {
|
||||
this.killedTombstones.delete(sessionId)
|
||||
this.killedTombstones.set(sessionId, Date.now())
|
||||
if (this.killedTombstones.size > this.maxTombstones) {
|
||||
const oldest = this.killedTombstones.keys().next().value
|
||||
if (oldest) {
|
||||
this.killedTombstones.delete(oldest)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
clearTombstone(sessionId: string): void {
|
||||
this.killedTombstones.delete(sessionId)
|
||||
}
|
||||
|
||||
isKilled(sessionId: string): boolean {
|
||||
return this.killedTombstones.has(sessionId)
|
||||
}
|
||||
|
||||
private writeFinalCheckpoints(opts: { dirtyOnly?: boolean } = {}): void {
|
||||
// Why: written before killing so graceful shutdown has zero data loss;
|
||||
// the checkpoint callback writes synchronously to disk.
|
||||
if (this.onFinalCheckpoint) {
|
||||
for (const [sessionId, session] of this.sessions) {
|
||||
if (!session.isAlive) {
|
||||
continue
|
||||
}
|
||||
if (opts.dirtyOnly && !session.hasPendingOutputForCheckpoint()) {
|
||||
continue
|
||||
}
|
||||
const take = session.takePendingOutput(true, { teardownSnapshot: true })
|
||||
if (take?.snapshot) {
|
||||
try {
|
||||
this.onFinalCheckpoint(sessionId, take.snapshot, take.records)
|
||||
} catch {
|
||||
// Best-effort — don't block shutdown
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private hasLiveSessions(): boolean {
|
||||
for (const [, session] of this.sessions) {
|
||||
if (session.isAlive) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
async drainLiveSessions(drainMs: number): Promise<void> {
|
||||
this.writeFinalCheckpoints()
|
||||
for (const [, session] of this.sessions) {
|
||||
// Why: daemon shutdown must not fan exit events to connected apps —
|
||||
// restartDaemon and external-termination recovery rely on sessions dying
|
||||
// silently so history meta keeps endedAt=null for cold restore.
|
||||
session.detachAllClients()
|
||||
}
|
||||
let anyLive = false
|
||||
for (const [, session] of this.sessions) {
|
||||
if (session.isAlive && !session.isTerminating) {
|
||||
session.kill()
|
||||
}
|
||||
anyLive ||= session.isAlive
|
||||
}
|
||||
if (!anyLive) {
|
||||
return
|
||||
}
|
||||
const deadline = Date.now() + drainMs
|
||||
while (Date.now() < deadline) {
|
||||
if (!this.hasLiveSessions()) {
|
||||
return
|
||||
}
|
||||
await new Promise((resolve) => setTimeout(resolve, 50))
|
||||
}
|
||||
}
|
||||
|
||||
dispose(): void {
|
||||
this.writeFinalCheckpoints()
|
||||
for (const [, session] of this.sessions) {
|
||||
session.detachAllClients()
|
||||
// Why: SIGKILL on an already-exited session would target a reaped pid
|
||||
// that POSIX can recycle to a stranger; dead sessions only release the
|
||||
// ptmx fd via disposeSubprocess(). See docs/fix-pty-fd-leak.md.
|
||||
if (session.isAlive) {
|
||||
session.forceKillAndDisposeSubprocess()
|
||||
} else {
|
||||
session.disposeSubprocess()
|
||||
}
|
||||
}
|
||||
this.sessions.clear()
|
||||
this.killedTombstones.clear()
|
||||
}
|
||||
|
||||
async disposeAndWaitForForceKill(opts: { checkpointDirtyOnly?: boolean } = {}): Promise<void> {
|
||||
this.writeFinalCheckpoints({ dirtyOnly: opts.checkpointDirtyOnly === true })
|
||||
const pendingForceKills: Promise<void>[] = []
|
||||
for (const [, session] of this.sessions) {
|
||||
session.detachAllClients()
|
||||
if (session.isAlive) {
|
||||
pendingForceKills.push(session.forceKillAndDisposeSubprocessAndWait())
|
||||
} else {
|
||||
session.disposeSubprocess()
|
||||
}
|
||||
}
|
||||
await Promise.all(pendingForceKills)
|
||||
this.sessions.clear()
|
||||
this.killedTombstones.clear()
|
||||
}
|
||||
}
|
||||
@@ -312,6 +312,112 @@ describe('TerminalHost', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('drainLiveSessions', () => {
|
||||
it('gracefully kills live sessions and resolves once they exit', async () => {
|
||||
await host.createOrAttach({
|
||||
sessionId: 'session-1',
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
streamClient: { onData: vi.fn(), onExit: vi.fn() }
|
||||
})
|
||||
|
||||
// The mock subprocess exits ~5ms after kill(), well inside the window.
|
||||
await host.drainLiveSessions(1_000)
|
||||
|
||||
expect(lastSubprocess.kill).toHaveBeenCalled()
|
||||
expect(lastSubprocess.forceKill).not.toHaveBeenCalled()
|
||||
expect(host.listSessions()).toEqual([])
|
||||
})
|
||||
|
||||
it('does not fan exit events to attached clients during the drain', async () => {
|
||||
const onExit = vi.fn()
|
||||
await host.createOrAttach({
|
||||
sessionId: 'session-1',
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
streamClient: { onData: vi.fn(), onExit }
|
||||
})
|
||||
|
||||
await host.drainLiveSessions(1_000)
|
||||
|
||||
// Why: daemon shutdown must stay silent toward connected apps — an exit
|
||||
// event here would write endedAt app-side and break cold restore after
|
||||
// a daemon restart or external termination.
|
||||
expect(onExit).not.toHaveBeenCalled()
|
||||
expect(host.listSessions()).toEqual([])
|
||||
})
|
||||
|
||||
it('returns after the bounded window when a session ignores the signal', async () => {
|
||||
await host.createOrAttach({
|
||||
sessionId: 'session-1',
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
streamClient: { onData: vi.fn(), onExit: vi.fn() }
|
||||
})
|
||||
// Session ignores the graceful signal: never fires onExit.
|
||||
;(lastSubprocess.kill as ReturnType<typeof vi.fn>).mockImplementation(() => {})
|
||||
|
||||
const start = Date.now()
|
||||
await host.drainLiveSessions(120)
|
||||
expect(Date.now() - start).toBeLessThan(2_000)
|
||||
|
||||
// dispose() (the caller's next step) force-kills the leftover.
|
||||
host.dispose()
|
||||
expect(lastSubprocess.forceKill).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does not checkpoint quiet stubborn sessions twice before force dispose', async () => {
|
||||
host.dispose()
|
||||
const onFinalCheckpoint = vi.fn()
|
||||
host = new TerminalHost({
|
||||
spawnSubprocess: spawnFn as MockSpawnFn,
|
||||
onFinalCheckpoint
|
||||
})
|
||||
await host.createOrAttach({
|
||||
sessionId: 'session-1',
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
streamClient: { onData: vi.fn(), onExit: vi.fn() }
|
||||
})
|
||||
lastSubprocess._onDataCb?.('before drain\r\n')
|
||||
;(lastSubprocess.kill as ReturnType<typeof vi.fn>).mockImplementation(() => {})
|
||||
|
||||
await host.drainLiveSessions(10)
|
||||
await host.disposeAndWaitForForceKill({ checkpointDirtyOnly: true })
|
||||
|
||||
expect(onFinalCheckpoint).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('checkpoints stubborn sessions again when they emit output during drain', async () => {
|
||||
host.dispose()
|
||||
const onFinalCheckpoint = vi.fn()
|
||||
host = new TerminalHost({
|
||||
spawnSubprocess: spawnFn as MockSpawnFn,
|
||||
onFinalCheckpoint
|
||||
})
|
||||
await host.createOrAttach({
|
||||
sessionId: 'session-1',
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
streamClient: { onData: vi.fn(), onExit: vi.fn() }
|
||||
})
|
||||
lastSubprocess._onDataCb?.('before drain\r\n')
|
||||
;(lastSubprocess.kill as ReturnType<typeof vi.fn>).mockImplementation(() => {
|
||||
lastSubprocess._onDataCb?.('tail during drain\r\n')
|
||||
})
|
||||
|
||||
await host.drainLiveSessions(10)
|
||||
await host.disposeAndWaitForForceKill({ checkpointDirtyOnly: true })
|
||||
|
||||
expect(onFinalCheckpoint).toHaveBeenCalledTimes(2)
|
||||
expect(onFinalCheckpoint).toHaveBeenLastCalledWith(
|
||||
'session-1',
|
||||
expect.objectContaining({ snapshotAnsi: expect.stringContaining('tail during drain') }),
|
||||
[]
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe('signal', () => {
|
||||
it('sends signal without entering kill state', async () => {
|
||||
await host.createOrAttach({
|
||||
|
||||
@@ -10,8 +10,7 @@ import type {
|
||||
ShellReadyState
|
||||
} from './types'
|
||||
import { SessionNotFoundError } from './types'
|
||||
|
||||
const DEFAULT_MAX_TOMBSTONES = 1000
|
||||
import { TerminalHostShutdown } from './terminal-host-shutdown'
|
||||
|
||||
export type CreateOrAttachOptions = {
|
||||
sessionId: string
|
||||
@@ -31,7 +30,10 @@ export type CreateOrAttachOptions = {
|
||||
terminalWindowsPowerShellImplementation?: 'auto' | 'powershell.exe' | 'pwsh.exe'
|
||||
shellReadySupported?: boolean
|
||||
shellReadyTimeoutMs?: number
|
||||
streamClient: { onData: (data: string) => void; onExit: (code: number) => void }
|
||||
streamClient: {
|
||||
onData: (data: string) => void
|
||||
onExit: (code: number, finalOutput?: TakePendingOutputResult) => void
|
||||
}
|
||||
}
|
||||
|
||||
export type CreateOrAttachResult = {
|
||||
@@ -71,15 +73,15 @@ export type TerminalHostOptions = {
|
||||
|
||||
export class TerminalHost {
|
||||
private sessions = new Map<string, Session>()
|
||||
private killedTombstones = new Map<string, number>()
|
||||
private spawnSubprocess: TerminalHostOptions['spawnSubprocess']
|
||||
private onFinalCheckpoint: TerminalHostOptions['onFinalCheckpoint']
|
||||
private maxTombstones: number
|
||||
private shutdownController: TerminalHostShutdown
|
||||
|
||||
constructor(opts: TerminalHostOptions) {
|
||||
this.spawnSubprocess = opts.spawnSubprocess
|
||||
this.onFinalCheckpoint = opts.onFinalCheckpoint
|
||||
this.maxTombstones = opts.maxTombstones ?? DEFAULT_MAX_TOMBSTONES
|
||||
this.shutdownController = new TerminalHostShutdown(this.sessions, {
|
||||
maxTombstones: opts.maxTombstones,
|
||||
onFinalCheckpoint: opts.onFinalCheckpoint
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -116,7 +118,7 @@ export class TerminalHost {
|
||||
}
|
||||
|
||||
// Clear tombstone if re-creating a killed session
|
||||
this.killedTombstones.delete(opts.sessionId)
|
||||
this.shutdownController.clearTombstone(opts.sessionId)
|
||||
const size = normalizePtySize(opts.cols, opts.rows)
|
||||
|
||||
const subprocess = this.spawnSubprocess({
|
||||
@@ -192,9 +194,9 @@ export class TerminalHost {
|
||||
this.getAliveSession(sessionId).resize(cols, rows)
|
||||
}
|
||||
|
||||
kill(sessionId: string, opts: { immediate?: boolean } = {}): void {
|
||||
kill(sessionId: string, opts: { immediate?: boolean; collectFinalOutput?: boolean } = {}): void {
|
||||
const session = this.getAliveSession(sessionId)
|
||||
this.recordTombstone(sessionId)
|
||||
this.shutdownController.recordTombstone(sessionId)
|
||||
if (opts.immediate) {
|
||||
session.forceKillAndDisposeSubprocess()
|
||||
// Why: the immediate path tears down synchronously without firing the
|
||||
@@ -203,6 +205,26 @@ export class TerminalHost {
|
||||
this.reapSession(sessionId)
|
||||
return
|
||||
}
|
||||
if (opts.collectFinalOutput) {
|
||||
session.requestFinalOutputOnExit()
|
||||
}
|
||||
session.kill()
|
||||
}
|
||||
|
||||
async killAndWait(
|
||||
sessionId: string,
|
||||
opts: { immediate?: boolean; collectFinalOutput?: boolean } = {}
|
||||
): Promise<void> {
|
||||
const session = this.getAliveSession(sessionId)
|
||||
this.shutdownController.recordTombstone(sessionId)
|
||||
if (opts.immediate) {
|
||||
await session.forceKillAndDisposeSubprocessAndWait()
|
||||
this.reapSession(sessionId)
|
||||
return
|
||||
}
|
||||
if (opts.collectFinalOutput) {
|
||||
session.requestFinalOutputOnExit()
|
||||
}
|
||||
session.kill()
|
||||
}
|
||||
|
||||
@@ -295,7 +317,7 @@ export class TerminalHost {
|
||||
}
|
||||
|
||||
isKilled(sessionId: string): boolean {
|
||||
return this.killedTombstones.has(sessionId)
|
||||
return this.shutdownController.isKilled(sessionId)
|
||||
}
|
||||
|
||||
listSessions(): SessionInfo[] {
|
||||
@@ -321,43 +343,16 @@ export class TerminalHost {
|
||||
return result
|
||||
}
|
||||
|
||||
dispose(): void {
|
||||
// Why: write final checkpoints before killing sessions so graceful shutdown
|
||||
// has zero data loss. The checkpoint callback writes synchronously to disk.
|
||||
if (this.onFinalCheckpoint) {
|
||||
for (const [sessionId, session] of this.sessions) {
|
||||
if (!session.isAlive) {
|
||||
continue
|
||||
}
|
||||
const take = session.takePendingOutput(true, { teardownSnapshot: true })
|
||||
if (take?.snapshot) {
|
||||
try {
|
||||
this.onFinalCheckpoint(sessionId, take.snapshot, take.records)
|
||||
} catch {
|
||||
// Best-effort — don't block shutdown
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
async drainLiveSessions(drainMs: number): Promise<void> {
|
||||
await this.shutdownController.drainLiveSessions(drainMs)
|
||||
}
|
||||
|
||||
for (const [, session] of this.sessions) {
|
||||
session.detachAllClients()
|
||||
// Why: live-vs-exited is load-bearing. For LIVE sessions we use
|
||||
// forceKillAndDisposeSubprocess (SIGKILL + destroy) to reap stubborn
|
||||
// children AND release the ptmx fd on the same tick, bypassing the 5s
|
||||
// KILL_TIMEOUT_MS fallback that would otherwise outlive the daemon
|
||||
// process. For sessions that have already exited but are still in the
|
||||
// map, SIGKILL would target a reaped pid — on POSIX that pid can be
|
||||
// recycled to an unrelated process, so we MUST only release the fd via
|
||||
// disposeSubprocess() (destroy without kill). See docs/fix-pty-fd-leak.md.
|
||||
if (session.isAlive) {
|
||||
session.forceKillAndDisposeSubprocess()
|
||||
} else {
|
||||
session.disposeSubprocess()
|
||||
}
|
||||
}
|
||||
this.sessions.clear()
|
||||
this.killedTombstones.clear()
|
||||
dispose(): void {
|
||||
this.shutdownController.dispose()
|
||||
}
|
||||
|
||||
async disposeAndWaitForForceKill(opts: { checkpointDirtyOnly?: boolean } = {}): Promise<void> {
|
||||
await this.shutdownController.disposeAndWaitForForceKill(opts)
|
||||
}
|
||||
|
||||
private getAliveSession(sessionId: string): Session {
|
||||
@@ -367,16 +362,4 @@ export class TerminalHost {
|
||||
}
|
||||
return session
|
||||
}
|
||||
|
||||
private recordTombstone(sessionId: string): void {
|
||||
this.killedTombstones.delete(sessionId)
|
||||
this.killedTombstones.set(sessionId, Date.now())
|
||||
|
||||
if (this.killedTombstones.size > this.maxTombstones) {
|
||||
const oldest = this.killedTombstones.keys().next().value
|
||||
if (oldest) {
|
||||
this.killedTombstones.delete(oldest)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7,9 +7,9 @@ import type { StartupCommandDelivery } from '../../shared/codex-startup-delivery
|
||||
// when daemon-baked behavior cannot be delivered by on-disk wrapper refresh.
|
||||
// Why: bump when adding daemon wire behavior so same-version old daemons do
|
||||
// not silently accept the handshake and then reject new RPCs.
|
||||
export const PROTOCOL_VERSION = 18
|
||||
export const PROTOCOL_VERSION = 19
|
||||
export const PREVIOUS_DAEMON_PROTOCOL_VERSIONS = [
|
||||
1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17
|
||||
1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18
|
||||
] as const
|
||||
|
||||
// ─── Session State Machine ──────────────────────────────────────────
|
||||
@@ -136,6 +136,7 @@ export type KillRequest = {
|
||||
payload: {
|
||||
sessionId: string
|
||||
immediate?: boolean
|
||||
collectFinalOutput?: boolean
|
||||
}
|
||||
}
|
||||
|
||||
@@ -359,7 +360,7 @@ export type ExitEvent = {
|
||||
type: 'event'
|
||||
event: 'exit'
|
||||
sessionId: string
|
||||
payload: { code: number }
|
||||
payload: { code: number; finalOutput?: TakePendingOutputResult }
|
||||
}
|
||||
|
||||
export type TerminalErrorEvent = {
|
||||
|
||||
+14
-5
@@ -2236,9 +2236,10 @@ app.on('will-quit', (e) => {
|
||||
destroySystemTray()
|
||||
// Why: stats.flush() must run before killAllPty() so it can read the
|
||||
// live agent state and emit synthetic agent_stop events for agents that
|
||||
// are still running. killAllPty() does not call runtime.onPtyExit(),
|
||||
// so without this ordering, running agents would produce orphaned
|
||||
// agent_start events with no matching stops.
|
||||
// are still running. killAllPty()'s graceful drain can fire
|
||||
// runtime.onPtyExit() for PTYs that exit inside the window, but stubborn
|
||||
// ones are force-killed without it — so flush-first stays the only
|
||||
// ordering that guarantees no orphaned agent_start events.
|
||||
starNag?.stop()
|
||||
automations?.stop()
|
||||
setUnreadDockBadgeCount(0)
|
||||
@@ -2252,7 +2253,9 @@ app.on('will-quit', (e) => {
|
||||
runtime?.getOffscreenBrowserBackend()?.destroyAll?.()
|
||||
const emulatorShutdown = runtime?.getEmulatorBridge()?.destroyAllSessions() ?? Promise.resolve()
|
||||
serveSimStateWatcher.stop()
|
||||
killAllPty()
|
||||
// Why: killAllPty drains agent CLIs (graceful signal → bounded force-kill)
|
||||
// so buffered transcripts flush; awaited below alongside the daemon flush.
|
||||
const ptyDrainOnQuit = killAllPty()
|
||||
const watcherShutdown = shutdownWatchersOnce()
|
||||
store?.flush()
|
||||
|
||||
@@ -2302,7 +2305,13 @@ app.on('will-quit', (e) => {
|
||||
// Why: normal quits preserve the detached daemon for warm reattach, but a
|
||||
// dev parent dying means the temp/dev profile has no owner left to reattach.
|
||||
const daemonTeardown = isDevParentShutdownRequested() ? shutdownDaemon() : disconnectDaemon()
|
||||
Promise.allSettled([daemonTeardown, rpcStopAndClear, watcherShutdown, emulatorShutdown])
|
||||
Promise.allSettled([
|
||||
daemonTeardown,
|
||||
rpcStopAndClear,
|
||||
watcherShutdown,
|
||||
emulatorShutdown,
|
||||
ptyDrainOnQuit
|
||||
])
|
||||
.then(() => shutdownTelemetry())
|
||||
.then(() => shutdownObservability())
|
||||
.catch(() => {
|
||||
|
||||
+76
-20
@@ -174,7 +174,8 @@ vi.mock('../agent-hooks/migration-unsupported-pty-state', () => ({
|
||||
clearMigrationUnsupportedPty: clearMigrationUnsupportedPtyMock,
|
||||
clearMigrationUnsupportedPtysForPaneKey: clearMigrationUnsupportedPtysForPaneKeyMock
|
||||
}))
|
||||
import { LocalPtyProvider } from '../providers/local-pty-provider'
|
||||
import { GRACEFUL_TEARDOWN_FORCE_KILL_MS, LocalPtyProvider } from '../providers/local-pty-provider'
|
||||
import { PTY_EXIT_DRAIN_WINDOW_MS } from '../providers/pty-shutdown-drain'
|
||||
import { makePaneKey } from '../../shared/stable-pane-id'
|
||||
import { SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV } from '../../shared/setup-agent-sequencing'
|
||||
import {
|
||||
@@ -2484,12 +2485,14 @@ describe('registerPtyHandlers', () => {
|
||||
kill: (ptyId: string) => boolean
|
||||
}
|
||||
|
||||
vi.useFakeTimers()
|
||||
expect(controller.kill('remote-pty')).toBe(true)
|
||||
// Why: kill's shutdown now runs through the exit-detection wrapper,
|
||||
// which adds async hops; a single microtask flush is no longer enough.
|
||||
await new Promise((resolve) => setImmediate(resolve))
|
||||
// Why: kill now drains gracefully before escalating; advance past the
|
||||
// drain window so the escalation + lease cleanup complete.
|
||||
await vi.advanceTimersByTimeAsync(PTY_EXIT_DRAIN_WINDOW_MS)
|
||||
|
||||
expect(shutdown).toHaveBeenCalledWith('remote-pty', { immediate: false })
|
||||
expect(shutdown).toHaveBeenLastCalledWith('remote-pty', { immediate: true })
|
||||
expect(store.markSshRemotePtyLease).toHaveBeenCalledWith(
|
||||
'ssh-1',
|
||||
'remote-pty',
|
||||
@@ -2652,10 +2655,15 @@ describe('registerPtyHandlers', () => {
|
||||
}
|
||||
|
||||
const stopPromise = controller.stopAndWait('remote-pty', { keepHistory: true })
|
||||
await vi.advanceTimersByTimeAsync(1_200)
|
||||
// Why: cover the graceful drain window plus the keepHistory settle poll.
|
||||
await vi.advanceTimersByTimeAsync(PTY_EXIT_DRAIN_WINDOW_MS + 1_200)
|
||||
await expect(stopPromise).resolves.toBe(true)
|
||||
|
||||
expect(shutdown).toHaveBeenCalledWith('remote-pty', {
|
||||
immediate: false,
|
||||
keepHistory: true
|
||||
})
|
||||
expect(shutdown).toHaveBeenLastCalledWith('remote-pty', {
|
||||
immediate: true,
|
||||
keepHistory: true
|
||||
})
|
||||
@@ -2707,7 +2715,8 @@ describe('registerPtyHandlers', () => {
|
||||
}
|
||||
|
||||
const stopPromise = controller.stopAndWait('local-pty', { keepHistory: true })
|
||||
await vi.advanceTimersByTimeAsync(200)
|
||||
// Why: cover the graceful drain window plus the keepHistory settle poll.
|
||||
await vi.advanceTimersByTimeAsync(PTY_EXIT_DRAIN_WINDOW_MS + 1_200)
|
||||
|
||||
await expect(stopPromise).resolves.toBe(false)
|
||||
expect(shutdown).toHaveBeenCalledWith('local-pty', {
|
||||
@@ -2828,8 +2837,10 @@ describe('registerPtyHandlers', () => {
|
||||
kill: (ptyId: string) => boolean
|
||||
}
|
||||
|
||||
vi.useFakeTimers()
|
||||
expect(controller.kill('ssh:ssh-1@@relay-pty')).toBe(true)
|
||||
await new Promise((resolve) => setImmediate(resolve))
|
||||
// Why: kill drains gracefully before escalating; advance past the window.
|
||||
await vi.advanceTimersByTimeAsync(PTY_EXIT_DRAIN_WINDOW_MS)
|
||||
|
||||
expect(shutdown).toHaveBeenCalledWith('ssh:ssh-1@@relay-pty', { immediate: false })
|
||||
expect(localShutdown).not.toHaveBeenCalled()
|
||||
@@ -3715,8 +3726,22 @@ describe('registerPtyHandlers', () => {
|
||||
|
||||
it('lists duplicate SSH relay session ids as distinct app sessions', async () => {
|
||||
registerPtyHandlers(mainWindow as never)
|
||||
const shutdownA = vi.fn(async () => undefined)
|
||||
const shutdownB = vi.fn(async () => undefined)
|
||||
const exitListenersA = new Set<(payload: { id: string; code: number }) => void>()
|
||||
const exitListenersB = new Set<(payload: { id: string; code: number }) => void>()
|
||||
const shutdownA = vi.fn(async (id: string, opts: { immediate?: boolean }) => {
|
||||
if (!opts.immediate) {
|
||||
for (const listener of exitListenersA) {
|
||||
listener({ id, code: 0 })
|
||||
}
|
||||
}
|
||||
})
|
||||
const shutdownB = vi.fn(async (id: string, opts: { immediate?: boolean }) => {
|
||||
if (!opts.immediate) {
|
||||
for (const listener of exitListenersB) {
|
||||
listener({ id, code: 0 })
|
||||
}
|
||||
}
|
||||
})
|
||||
registerSshPtyProvider('ssh-a', {
|
||||
spawn: vi.fn(),
|
||||
write: vi.fn(),
|
||||
@@ -3729,7 +3754,10 @@ describe('registerPtyHandlers', () => {
|
||||
acknowledgeDataEvent: vi.fn(),
|
||||
onData: vi.fn(() => () => {}),
|
||||
onReplay: vi.fn(() => () => {}),
|
||||
onExit: vi.fn(() => () => {}),
|
||||
onExit: vi.fn((cb: (payload: { id: string; code: number }) => void) => {
|
||||
exitListenersA.add(cb)
|
||||
return () => exitListenersA.delete(cb)
|
||||
}),
|
||||
listProcesses: vi.fn(async () => [
|
||||
{ id: 'ssh:ssh-a@@pty-1', cwd: '/repo-a', title: 'ssh-a' }
|
||||
]),
|
||||
@@ -3753,7 +3781,10 @@ describe('registerPtyHandlers', () => {
|
||||
acknowledgeDataEvent: vi.fn(),
|
||||
onData: vi.fn(() => () => {}),
|
||||
onReplay: vi.fn(() => () => {}),
|
||||
onExit: vi.fn(() => () => {}),
|
||||
onExit: vi.fn((cb: (payload: { id: string; code: number }) => void) => {
|
||||
exitListenersB.add(cb)
|
||||
return () => exitListenersB.delete(cb)
|
||||
}),
|
||||
listProcesses: vi.fn(async () => [
|
||||
{ id: 'ssh:ssh-b@@pty-1', cwd: '/repo-b', title: 'ssh-b' }
|
||||
]),
|
||||
@@ -3783,11 +3814,11 @@ describe('registerPtyHandlers', () => {
|
||||
await handlers.get('pty:kill')!(null, { id: 'ssh:ssh-b@@pty-1' })
|
||||
|
||||
expect(shutdownA).toHaveBeenCalledWith('ssh:ssh-a@@pty-1', {
|
||||
immediate: true,
|
||||
immediate: false,
|
||||
keepHistory: false
|
||||
})
|
||||
expect(shutdownB).toHaveBeenCalledWith('ssh:ssh-b@@pty-1', {
|
||||
immediate: true,
|
||||
immediate: false,
|
||||
keepHistory: false
|
||||
})
|
||||
})
|
||||
@@ -8370,13 +8401,21 @@ describe('registerPtyHandlers', () => {
|
||||
rows: 24
|
||||
})) as { id: string }
|
||||
|
||||
await handlers.get('pty:kill')!(null, { id: spawnResult.id })
|
||||
vi.useFakeTimers()
|
||||
const killPromise = handlers.get('pty:kill')!(null, { id: spawnResult.id })
|
||||
await vi.advanceTimersByTimeAsync(PTY_EXIT_DRAIN_WINDOW_MS)
|
||||
await killPromise
|
||||
|
||||
// Why: the graceful signal (first kill call) intentionally keeps listeners
|
||||
// attached so the natural exit path can flow. The dispose-before-kill
|
||||
// NAPI-crash invariant now applies to the SIGKILL escalation.
|
||||
expect(killSpy.mock.calls[0]).toEqual([])
|
||||
expect(killSpy.mock.calls[1]).toEqual(['SIGKILL'])
|
||||
expect(onDataDisposable.dispose.mock.invocationCallOrder[0]).toBeLessThan(
|
||||
killSpy.mock.invocationCallOrder[0]
|
||||
killSpy.mock.invocationCallOrder[1]
|
||||
)
|
||||
expect(onExitDisposable.dispose.mock.invocationCallOrder[0]).toBeLessThan(
|
||||
killSpy.mock.invocationCallOrder[0]
|
||||
killSpy.mock.invocationCallOrder[1]
|
||||
)
|
||||
})
|
||||
|
||||
@@ -8411,12 +8450,19 @@ describe('registerPtyHandlers', () => {
|
||||
kill: (ptyId: string) => boolean
|
||||
}
|
||||
|
||||
vi.useFakeTimers()
|
||||
expect(runtimeController.kill(spawnResult.id)).toBe(true)
|
||||
await vi.advanceTimersByTimeAsync(PTY_EXIT_DRAIN_WINDOW_MS)
|
||||
|
||||
// Why: the graceful signal (first kill call) intentionally keeps listeners
|
||||
// attached; the dispose-before-kill invariant applies to the escalation.
|
||||
expect(killSpy.mock.calls[0]).toEqual([])
|
||||
expect(killSpy.mock.calls[1]).toEqual(['SIGKILL'])
|
||||
expect(onDataDisposable.dispose.mock.invocationCallOrder[0]).toBeLessThan(
|
||||
killSpy.mock.invocationCallOrder[0]
|
||||
killSpy.mock.invocationCallOrder[1]
|
||||
)
|
||||
expect(onExitDisposable.dispose.mock.invocationCallOrder[0]).toBeLessThan(
|
||||
killSpy.mock.invocationCallOrder[0]
|
||||
killSpy.mock.invocationCallOrder[1]
|
||||
)
|
||||
})
|
||||
|
||||
@@ -8451,14 +8497,20 @@ describe('registerPtyHandlers', () => {
|
||||
|
||||
// The first load after spawn only advances generation. The second one sees
|
||||
// this PTY as belonging to a prior page load and kills it as orphaned.
|
||||
vi.useFakeTimers()
|
||||
didFinishLoad?.()
|
||||
didFinishLoad?.()
|
||||
// Why: the orphan sweep drains gracefully first; the force-kill (and its
|
||||
// dispose-before-kill invariant) fires after the fallback window.
|
||||
await vi.advanceTimersByTimeAsync(GRACEFUL_TEARDOWN_FORCE_KILL_MS)
|
||||
|
||||
expect(killSpy.mock.calls[0]).toEqual([])
|
||||
expect(killSpy.mock.calls[1]).toEqual(['SIGKILL'])
|
||||
expect(onDataDisposable.dispose.mock.invocationCallOrder[0]).toBeLessThan(
|
||||
killSpy.mock.invocationCallOrder[0]
|
||||
killSpy.mock.invocationCallOrder[1]
|
||||
)
|
||||
expect(onExitDisposable.dispose.mock.invocationCallOrder[0]).toBeLessThan(
|
||||
killSpy.mock.invocationCallOrder[0]
|
||||
killSpy.mock.invocationCallOrder[1]
|
||||
)
|
||||
})
|
||||
|
||||
@@ -8607,11 +8659,15 @@ describe('registerPtyHandlers', () => {
|
||||
|
||||
// First load only advances the generation; the second sees this PTY as a
|
||||
// prior-load orphan. With the flag false the guard must NOT suppress the sweep.
|
||||
vi.useFakeTimers()
|
||||
didFinishLoad?.()
|
||||
didFinishLoad?.()
|
||||
|
||||
expect(killSpy).toHaveBeenCalled()
|
||||
expect(runtime.onPtyExit).toHaveBeenCalledWith(spawnResult.id, -1)
|
||||
// Why: the sweep drains gracefully; the force-kill fallback reaps the
|
||||
// SIGHUP-ignoring PTY (this stub never exits) after the bounded window.
|
||||
await vi.advanceTimersByTimeAsync(GRACEFUL_TEARDOWN_FORCE_KILL_MS)
|
||||
const listed = await getLocalPtyProvider().listProcesses()
|
||||
expect(listed.some((info) => info.id === spawnResult.id)).toBe(false)
|
||||
})
|
||||
|
||||
+26
-33
@@ -40,10 +40,14 @@ import type { IPtyProvider, PtySpawnOptions, PtySpawnResult } from '../providers
|
||||
import type { StartupCommandDelivery } from '../../shared/codex-startup-delivery'
|
||||
import {
|
||||
SSH_SESSION_EXPIRED_ERROR,
|
||||
isSshPtyIdentityMismatchError,
|
||||
isSshPtyNotFoundError
|
||||
isSshPtyIdentityMismatchError
|
||||
} from '../providers/ssh-pty-provider'
|
||||
import { parseAppSshPtyId, toAppSshPtyId, toRelaySshPtyId } from '../providers/ssh-pty-id'
|
||||
import {
|
||||
APP_QUIT_PTY_DRAIN_MS,
|
||||
isPtyAlreadyGoneError,
|
||||
shutdownPtyWithDrain
|
||||
} from '../providers/pty-shutdown-drain'
|
||||
import { createPtySpawnTiming } from './pty-spawn-timing'
|
||||
import { mintPtySessionId, isSafePtySessionId } from '../daemon/pty-session-id'
|
||||
import { addNodePtyRecoveryHint } from '../daemon/node-pty-error-hints'
|
||||
@@ -428,11 +432,6 @@ function normalizeNodePtySpawnError(err: unknown): Error {
|
||||
return new Error(hintedMessage)
|
||||
}
|
||||
|
||||
function isPtyAlreadyGoneError(err: unknown): boolean {
|
||||
const message = err instanceof Error ? err.message : String(err)
|
||||
return isSshPtyNotFoundError(err) || /Session not found/i.test(message)
|
||||
}
|
||||
|
||||
function delay(ms: number): Promise<void> {
|
||||
return new Promise((resolve) => {
|
||||
const timer = setTimeout(resolve, ms)
|
||||
@@ -1791,25 +1790,6 @@ export function registerPtyHandlers(
|
||||
mainWindow.webContents.send('pty:exit', payload)
|
||||
}
|
||||
|
||||
async function shutdownProviderAndDetectExit(
|
||||
provider: IPtyProvider,
|
||||
id: string,
|
||||
opts: { immediate?: boolean; keepHistory?: boolean }
|
||||
): Promise<boolean> {
|
||||
let providerExitObserved = false
|
||||
const unsubscribe = provider.onExit((payload) => {
|
||||
if (payload.id === id) {
|
||||
providerExitObserved = true
|
||||
}
|
||||
})
|
||||
try {
|
||||
await provider.shutdown(id, opts)
|
||||
} finally {
|
||||
unsubscribe()
|
||||
}
|
||||
return providerExitObserved
|
||||
}
|
||||
|
||||
// Why: extracted so the "Restart daemon" flow can rebind against the fresh
|
||||
// adapter after replaceDaemonProvider runs. Both the startup registration
|
||||
// and the post-restart rebind go through the same code path — no risk of
|
||||
@@ -2025,6 +2005,10 @@ export function registerPtyHandlers(
|
||||
ptyOwnership.delete(id)
|
||||
markClaudePtyExited(id)
|
||||
runtime?.onPtyExit(id, -1)
|
||||
// Why: the sweep now drains gracefully, so the spawn-time exit handler
|
||||
// stays armed and can deliver the natural exit seconds later; swallow
|
||||
// that duplicate so the fresh renderer never sees a stray pty:exit.
|
||||
rememberSyntheticKillExit(id)
|
||||
}
|
||||
}
|
||||
didFinishLoadWebContents = mainWindow.webContents
|
||||
@@ -2465,7 +2449,7 @@ export function registerPtyHandlers(
|
||||
// Same synthetic-exit contract as the renderer pty:kill handler: when the
|
||||
// provider emitted its own exit during shutdown, the exit listener already
|
||||
// delivered runtime + renderer exits — synthesizing again would double-fire.
|
||||
void shutdownProviderAndDetectExit(provider, ptyId, { immediate: false })
|
||||
void shutdownPtyWithDrain(provider, ptyId, {})
|
||||
.then((providerExitObserved) => {
|
||||
finishPtyShutdown(ptyId, connectionId, store)
|
||||
if (!providerExitObserved) {
|
||||
@@ -2514,8 +2498,9 @@ export function registerPtyHandlers(
|
||||
}
|
||||
let providerExitObserved = false
|
||||
try {
|
||||
providerExitObserved = await shutdownProviderAndDetectExit(provider, ptyId, {
|
||||
immediate: true,
|
||||
// Why: graceful-drain-then-force (not immediate) so an agent CLI in
|
||||
// the pane can flush its transcript before the SIGKILL escalation.
|
||||
providerExitObserved = await shutdownPtyWithDrain(provider, ptyId, {
|
||||
keepHistory: opts?.keepHistory ?? false
|
||||
})
|
||||
} catch (err) {
|
||||
@@ -3723,8 +3708,10 @@ export function registerPtyHandlers(
|
||||
const shutdownProvider = provider ?? getProviderForPty(args.id)
|
||||
let providerExitObserved = false
|
||||
try {
|
||||
providerExitObserved = await shutdownProviderAndDetectExit(shutdownProvider, args.id, {
|
||||
immediate: true,
|
||||
// Why: tab close / worktree close land here. Drain gracefully so agent
|
||||
// CLIs can flush transcripts; shutdownPtyWithDrain force-kills on
|
||||
// timeout, so this stays bounded and the PTY is dead when we resolve.
|
||||
providerExitObserved = await shutdownPtyWithDrain(shutdownProvider, args.id, {
|
||||
keepHistory: args.keepHistory ?? false
|
||||
})
|
||||
} catch (err) {
|
||||
@@ -3929,9 +3916,15 @@ export function registerHeadlessPtyRuntime(
|
||||
|
||||
/**
|
||||
* Kill all PTY processes. Call on app quit.
|
||||
*
|
||||
* Returns a bounded promise: agent CLIs get a short graceful-drain window to
|
||||
* flush transcripts, then leftovers are force-killed. Quit paths should fold
|
||||
* this into their async teardown wait (will-quit already defers for the
|
||||
* daemon disconnect) so the drain completes before the process exits.
|
||||
*/
|
||||
export function killAllPty(): void {
|
||||
export function killAllPty(): Promise<void> {
|
||||
if (localProvider instanceof LocalPtyProvider) {
|
||||
localProvider.killAll()
|
||||
return localProvider.drainAndKillAll(APP_QUIT_PTY_DRAIN_MS)
|
||||
}
|
||||
return Promise.resolve()
|
||||
}
|
||||
|
||||
@@ -9,7 +9,10 @@ const {
|
||||
mkdirSyncMock,
|
||||
writeFileSyncMock,
|
||||
spawnMock,
|
||||
resolveAgentForegroundProcessMock
|
||||
resolveAgentForegroundProcessMock,
|
||||
requestWindowsProcessTreeExitMock,
|
||||
forceKillWindowsProcessTreeMock,
|
||||
waitForWindowsProcessTreeForceKillMock
|
||||
} = vi.hoisted(() => ({
|
||||
existsSyncMock: vi.fn(),
|
||||
statSyncMock: vi.fn(),
|
||||
@@ -17,7 +20,18 @@ const {
|
||||
mkdirSyncMock: vi.fn(),
|
||||
writeFileSyncMock: vi.fn(),
|
||||
spawnMock: vi.fn(),
|
||||
resolveAgentForegroundProcessMock: vi.fn()
|
||||
resolveAgentForegroundProcessMock: vi.fn(),
|
||||
requestWindowsProcessTreeExitMock: vi.fn(),
|
||||
forceKillWindowsProcessTreeMock: vi.fn(),
|
||||
waitForWindowsProcessTreeForceKillMock: vi.fn()
|
||||
}))
|
||||
|
||||
// Why: never let provider teardown paths spawn a real `taskkill` from tests —
|
||||
// on a Windows runner that would signal an arbitrary live pid.
|
||||
vi.mock('../pty/windows-process-tree-kill', () => ({
|
||||
requestWindowsProcessTreeExit: requestWindowsProcessTreeExitMock,
|
||||
forceKillWindowsProcessTree: forceKillWindowsProcessTreeMock,
|
||||
waitForWindowsProcessTreeForceKill: waitForWindowsProcessTreeForceKillMock
|
||||
}))
|
||||
|
||||
vi.mock('fs', () => ({
|
||||
@@ -122,15 +136,29 @@ describe('LocalPtyProvider', () => {
|
||||
resolveAgentForegroundProcessMock.mockImplementation(
|
||||
async (_pid: number, fallbackProcess: string | null) => fallbackProcess
|
||||
)
|
||||
requestWindowsProcessTreeExitMock.mockReset()
|
||||
forceKillWindowsProcessTreeMock.mockReset()
|
||||
waitForWindowsProcessTreeForceKillMock.mockReset()
|
||||
waitForWindowsProcessTreeForceKillMock.mockResolvedValue(undefined)
|
||||
|
||||
exitCb = undefined
|
||||
// Why: real node-pty supports multiple onExit listeners (spawn's cleanup
|
||||
// handler + drainAndKillAll's waiter); mirror that so both fire.
|
||||
let exitCallbacks: ((info: { exitCode: number }) => void)[] = []
|
||||
mockProc = {
|
||||
onData: vi.fn(() => ({ dispose: vi.fn() })),
|
||||
onExit: vi.fn((cb: (info: { exitCode: number }) => void) => {
|
||||
exitCb = cb
|
||||
exitCallbacks.push(cb)
|
||||
exitCb = (info) => {
|
||||
// oxlint-disable-next-line unicorn/no-useless-spread -- copy-safe: listeners may dispose during iteration
|
||||
for (const registered of [...exitCallbacks]) {
|
||||
registered(info)
|
||||
}
|
||||
}
|
||||
return {
|
||||
dispose: () => {
|
||||
if (exitCb === cb) {
|
||||
exitCallbacks = exitCallbacks.filter((registered) => registered !== cb)
|
||||
if (exitCallbacks.length === 0) {
|
||||
exitCb = undefined
|
||||
}
|
||||
}
|
||||
@@ -954,6 +982,119 @@ describe('LocalPtyProvider', () => {
|
||||
await provider.shutdown('nonexistent', { immediate: true })
|
||||
expect(mockProc.kill).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('graceful shutdown signals SIGHUP and lets the natural exit path clean up', async () => {
|
||||
const onExit = vi.fn()
|
||||
provider.configure({ onExit })
|
||||
const killSpy = mockProc.kill
|
||||
const { id } = await provider.spawn({ cols: 80, rows: 24 })
|
||||
|
||||
// mockProc.kill fires the registered exit callback, simulating a shell
|
||||
// that exits promptly on SIGHUP.
|
||||
await provider.shutdown(id, { immediate: false })
|
||||
|
||||
expect(killSpy).toHaveBeenCalledWith()
|
||||
expect(onExit).toHaveBeenCalledWith(id, -1)
|
||||
expect(provider.hasPty(id)).toBe(false)
|
||||
})
|
||||
|
||||
it('graceful shutdown keeps a SIGHUP-ignoring PTY alive for the drain window', async () => {
|
||||
const onExit = vi.fn()
|
||||
provider.configure({ onExit })
|
||||
// Why: capture the spy — teardown neutralizes proc.kill afterwards.
|
||||
const killSpy = vi.fn() // shell ignores SIGHUP: no exit fires
|
||||
mockProc.kill = killSpy
|
||||
const { id } = await provider.spawn({ cols: 80, rows: 24 })
|
||||
|
||||
await provider.shutdown(id, { immediate: false })
|
||||
|
||||
expect(onExit).not.toHaveBeenCalled()
|
||||
expect(provider.hasPty(id)).toBe(true)
|
||||
|
||||
// The drain caller escalates; the immediate path must SIGKILL.
|
||||
await provider.shutdown(id, { immediate: true })
|
||||
expect(killSpy).toHaveBeenCalledWith('SIGKILL')
|
||||
expect(onExit).toHaveBeenCalledWith(id, -1)
|
||||
expect(provider.hasPty(id)).toBe(false)
|
||||
})
|
||||
|
||||
it('graceful shutdown on Windows asks the process tree to close', async () => {
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||||
const killSpy = vi.fn()
|
||||
spawnMock.mockReturnValue({ ...mockProc, kill: killSpy })
|
||||
const { id } = await provider.spawn({ cols: 80, rows: 24 })
|
||||
|
||||
await provider.shutdown(id, { immediate: false })
|
||||
|
||||
expect(requestWindowsProcessTreeExitMock).toHaveBeenCalledWith(12345)
|
||||
expect(killSpy).not.toHaveBeenCalled()
|
||||
expect(provider.hasPty(id)).toBe(true)
|
||||
|
||||
await provider.shutdown(id, { immediate: true })
|
||||
expect(waitForWindowsProcessTreeForceKillMock).toHaveBeenCalledWith(12345)
|
||||
expect(killSpy).toHaveBeenCalled()
|
||||
expect(provider.hasPty(id)).toBe(false)
|
||||
})
|
||||
|
||||
it('awaits Windows force taskkill before clearing local PTY state', async () => {
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||||
let releaseForceKill = (): void => {
|
||||
throw new Error('force kill waiter was not installed')
|
||||
}
|
||||
waitForWindowsProcessTreeForceKillMock.mockImplementation(
|
||||
() =>
|
||||
new Promise<void>((resolve) => {
|
||||
releaseForceKill = resolve
|
||||
})
|
||||
)
|
||||
const { id } = await provider.spawn({ cols: 80, rows: 24 })
|
||||
|
||||
let settled = false
|
||||
const shutdown = provider.shutdown(id, { immediate: true }).then(() => {
|
||||
settled = true
|
||||
})
|
||||
await Promise.resolve()
|
||||
|
||||
expect(settled).toBe(false)
|
||||
expect(provider.hasPty(id)).toBe(true)
|
||||
releaseForceKill()
|
||||
await shutdown
|
||||
|
||||
expect(settled).toBe(true)
|
||||
expect(provider.hasPty(id)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('drainAndKillAll', () => {
|
||||
it('resolves once PTYs exit inside the drain window without force-killing', async () => {
|
||||
const killSpy = mockProc.kill
|
||||
await provider.spawn({ cols: 80, rows: 24 })
|
||||
|
||||
await provider.drainAndKillAll(1_000)
|
||||
|
||||
// mockProc.kill fires exit synchronously, so no force-kill escalation.
|
||||
expect(killSpy).toHaveBeenCalledTimes(1)
|
||||
expect(killSpy).toHaveBeenCalledWith()
|
||||
})
|
||||
|
||||
it('force-kills PTYs that outlive the drain window', async () => {
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
// Why: capture the spy — teardown neutralizes proc.kill afterwards.
|
||||
const killSpy = vi.fn() // ignores SIGHUP
|
||||
mockProc.kill = killSpy
|
||||
const { id } = await provider.spawn({ cols: 80, rows: 24 })
|
||||
|
||||
const drain = provider.drainAndKillAll(1_000)
|
||||
await vi.advanceTimersByTimeAsync(1_000)
|
||||
await drain
|
||||
|
||||
expect(killSpy).toHaveBeenCalledWith('SIGKILL')
|
||||
expect(provider.hasPty(id)).toBe(false)
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
describe('hasChildProcesses', () => {
|
||||
|
||||
@@ -52,6 +52,11 @@ import {
|
||||
} from '../git-bash'
|
||||
import { WINDOWS_GIT_BASH_SHELL } from '../../shared/windows-terminal-shell'
|
||||
import { resolveAgentForegroundProcess } from './agent-foreground-process'
|
||||
import {
|
||||
forceKillWindowsProcessTree,
|
||||
requestWindowsProcessTreeExit,
|
||||
waitForWindowsProcessTreeForceKill
|
||||
} from '../pty/windows-process-tree-kill'
|
||||
import { getAgentForegroundContextPaths } from './agent-foreground-context-paths'
|
||||
import { recognizeAgentProcessFromCommandLine } from '../../shared/agent-process-recognition'
|
||||
import { shouldUseShellReadyStartupDelivery } from '../../shared/codex-startup-delivery'
|
||||
@@ -268,20 +273,97 @@ function destroyPtyProcess(proc: pty.IPty, options: { alreadyKilled?: boolean }
|
||||
}
|
||||
|
||||
/**
|
||||
* Kills a local PTY and clears all associated local provider state.
|
||||
* Asks a local PTY's process tree to exit without force-killing it.
|
||||
*/
|
||||
function safeKillAndClean(id: string, proc: pty.IPty): void {
|
||||
runPtyCleanup(id)
|
||||
disposePtyListeners(id)
|
||||
function requestGracefulPtyExit(proc: pty.IPty): void {
|
||||
// Why: agent CLIs buffer transcript writes and need a drain window to flush.
|
||||
// On POSIX, node-pty's kill() sends SIGHUP — the PTY-native hangup signal,
|
||||
// which shells forward to their foreground job. Deliberately no destroy()
|
||||
// here: closing the master fd right away would collapse the drain window.
|
||||
if (process.platform === 'win32') {
|
||||
requestWindowsProcessTreeExit(proc.pid)
|
||||
return
|
||||
}
|
||||
try {
|
||||
proc.kill()
|
||||
} catch {
|
||||
/* Process may already be dead */
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Force-kills a local PTY's process tree (the post-drain escalation).
|
||||
*/
|
||||
function forceKillPtyProcess(proc: pty.IPty): void {
|
||||
try {
|
||||
if (process.platform === 'win32') {
|
||||
// Why: taskkill /T /F reaps the whole console tree; proc.kill() then
|
||||
// releases the ConPTY handle itself.
|
||||
forceKillWindowsProcessTree(proc.pid)
|
||||
proc.kill()
|
||||
} else {
|
||||
// Why: SIGKILL, not SIGHUP — the graceful path already sent SIGHUP and
|
||||
// this escalation must reap even a signal-ignoring child.
|
||||
proc.kill('SIGKILL')
|
||||
}
|
||||
} catch {
|
||||
/* Process may already be dead */
|
||||
}
|
||||
}
|
||||
|
||||
async function forceKillPtyProcessAndWait(proc: pty.IPty): Promise<void> {
|
||||
try {
|
||||
if (process.platform === 'win32') {
|
||||
// Why: worktree deletion on Windows needs the tree gone before return,
|
||||
// otherwise live descendants can keep cwd/files locked with EPERM.
|
||||
await waitForWindowsProcessTreeForceKill(proc.pid)
|
||||
proc.kill()
|
||||
} else {
|
||||
proc.kill('SIGKILL')
|
||||
}
|
||||
} catch {
|
||||
/* Process may already be dead */
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Force-kills a local PTY and clears all associated local provider state.
|
||||
*/
|
||||
function safeKillAndClean(id: string, proc: pty.IPty): void {
|
||||
runPtyCleanup(id)
|
||||
disposePtyListeners(id)
|
||||
forceKillPtyProcess(proc)
|
||||
destroyPtyProcess(proc, { alreadyKilled: true })
|
||||
clearPtyState(id)
|
||||
}
|
||||
|
||||
async function safeKillAndCleanAndWait(id: string, proc: pty.IPty): Promise<void> {
|
||||
runPtyCleanup(id)
|
||||
disposePtyListeners(id)
|
||||
await forceKillPtyProcessAndWait(proc)
|
||||
destroyPtyProcess(proc, { alreadyKilled: true })
|
||||
clearPtyState(id)
|
||||
}
|
||||
|
||||
/** Force-kill fallback for graceful teardowns that no caller escalates
|
||||
* (orphan sweeps, app quit): bounded so nothing can linger forever. */
|
||||
export const GRACEFUL_TEARDOWN_FORCE_KILL_MS = 5_000
|
||||
|
||||
/**
|
||||
* Starts a graceful teardown and arms a bounded force-kill fallback.
|
||||
* The natural spawn()-time onExit handler owns cleanup when the process
|
||||
* exits inside the window; the timer only fires for signal-ignoring children.
|
||||
*/
|
||||
function beginGracefulPtyTeardown(id: string, proc: pty.IPty, forceAfterMs: number): void {
|
||||
requestGracefulPtyExit(proc)
|
||||
const timer = setTimeout(() => {
|
||||
if (ptyProcesses.get(id) === proc) {
|
||||
safeKillAndClean(id, proc)
|
||||
}
|
||||
}, forceAfterMs)
|
||||
timer.unref?.()
|
||||
}
|
||||
|
||||
export type LocalPtyProviderOptions = {
|
||||
/** Why: `ctx.command` carries the renderer-chosen launch command (e.g. `pi`,
|
||||
* `omp`, `claude`). Pi vs OMP must drive overlay source-dir selection in
|
||||
@@ -855,11 +937,19 @@ export class LocalPtyProvider implements IPtyProvider {
|
||||
return { cols: proc.cols, rows: proc.rows }
|
||||
}
|
||||
|
||||
async shutdown(id: string, _opts: { immediate?: boolean; keepHistory?: boolean }): Promise<void> {
|
||||
async shutdown(id: string, opts: { immediate?: boolean; keepHistory?: boolean }): Promise<void> {
|
||||
const proc = ptyProcesses.get(id)
|
||||
if (!proc) {
|
||||
return
|
||||
}
|
||||
if (!opts.immediate) {
|
||||
// Why: graceful shutdown only signals; the spawn()-time onExit handler
|
||||
// still owns cleanup + exit fanout when the child drains and exits. The
|
||||
// caller (pty-shutdown-drain) bounds the wait and escalates to the
|
||||
// immediate path below on timeout, so nothing can linger forever.
|
||||
requestGracefulPtyExit(proc)
|
||||
return
|
||||
}
|
||||
// Why: disposePtyListeners removes the onExit callback, so the natural
|
||||
// exit cleanup path from node-pty won't fire. Cleanup and notification
|
||||
// must happen unconditionally after the try/catch.
|
||||
@@ -867,11 +957,7 @@ export class LocalPtyProvider implements IPtyProvider {
|
||||
// the natural onExit callback from running the usual clearPtyState path.
|
||||
runPtyCleanup(id)
|
||||
disposePtyListeners(id)
|
||||
try {
|
||||
proc.kill()
|
||||
} catch {
|
||||
/* Process may already be dead */
|
||||
}
|
||||
await forceKillPtyProcessAndWait(proc)
|
||||
destroyPtyProcess(proc, { alreadyKilled: true })
|
||||
clearPtyState(id)
|
||||
this.opts.onExit?.(id, -1)
|
||||
@@ -1027,7 +1113,9 @@ export class LocalPtyProvider implements IPtyProvider {
|
||||
const killed: { id: string }[] = []
|
||||
for (const [id, proc] of ptyProcesses) {
|
||||
if ((ptyLoadGeneration.get(id) ?? -1) < currentGeneration) {
|
||||
safeKillAndClean(id, proc)
|
||||
// Why: an orphan can still host an agent CLI mid-flush (reload while
|
||||
// an agent runs); drain gracefully instead of an instant kill.
|
||||
beginGracefulPtyTeardown(id, proc, GRACEFUL_TEARDOWN_FORCE_KILL_MS)
|
||||
killed.push({ id })
|
||||
}
|
||||
}
|
||||
@@ -1044,10 +1132,57 @@ export class LocalPtyProvider implements IPtyProvider {
|
||||
return ptyProcesses.get(id)
|
||||
}
|
||||
|
||||
/** Kill all PTYs. Call on app quit. */
|
||||
/** Kill all PTYs immediately. Prefer drainAndKillAll on quit paths. */
|
||||
killAll(): void {
|
||||
for (const [id, proc] of ptyProcesses) {
|
||||
safeKillAndClean(id, proc)
|
||||
}
|
||||
}
|
||||
|
||||
/** Gracefully drain all PTYs, force-killing leftovers after `drainMs`.
|
||||
* Resolves once every PTY has exited or been force-killed, so quit paths
|
||||
* can await it inside their bounded async-teardown window. */
|
||||
async drainAndKillAll(drainMs: number): Promise<void> {
|
||||
const entries = Array.from(ptyProcesses.entries())
|
||||
await Promise.all(
|
||||
entries.map(
|
||||
([id, proc]) =>
|
||||
new Promise<void>((resolve) => {
|
||||
let settled = false
|
||||
let disposable: { dispose: () => void } | undefined
|
||||
let timer: ReturnType<typeof setTimeout> | null = null
|
||||
const finish = (): void => {
|
||||
settled = true
|
||||
if (timer) {
|
||||
clearTimeout(timer)
|
||||
timer = null
|
||||
}
|
||||
disposable?.dispose()
|
||||
disposable = undefined
|
||||
resolve()
|
||||
}
|
||||
try {
|
||||
disposable = proc.onExit(() => finish())
|
||||
} catch {
|
||||
/* already torn down — the timer path still resolves */
|
||||
}
|
||||
requestGracefulPtyExit(proc)
|
||||
// Why: a shell can exit synchronously inside the graceful signal
|
||||
// (fast SIGHUP death); don't arm a stale force-kill timer then.
|
||||
if (settled) {
|
||||
return
|
||||
}
|
||||
timer = setTimeout(() => {
|
||||
void (async () => {
|
||||
if (ptyProcesses.get(id) === proc) {
|
||||
await safeKillAndCleanAndWait(id, proc)
|
||||
}
|
||||
finish()
|
||||
})()
|
||||
}, drainMs)
|
||||
timer.unref?.()
|
||||
})
|
||||
)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,157 @@
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { IPtyProvider } from './types'
|
||||
import {
|
||||
PTY_EXIT_DRAIN_WINDOW_MS,
|
||||
isPtyAlreadyGoneError,
|
||||
shutdownPtyWithDrain
|
||||
} from './pty-shutdown-drain'
|
||||
|
||||
type ExitPayload = { id: string; code: number }
|
||||
|
||||
function createProvider(): {
|
||||
provider: IPtyProvider
|
||||
shutdown: ReturnType<typeof vi.fn>
|
||||
emitExit: (payload: ExitPayload) => void
|
||||
} {
|
||||
const exitListeners = new Set<(payload: ExitPayload) => void>()
|
||||
const shutdown = vi.fn().mockResolvedValue(undefined)
|
||||
const provider = {
|
||||
shutdown,
|
||||
onExit: (cb: (payload: ExitPayload) => void) => {
|
||||
exitListeners.add(cb)
|
||||
return () => exitListeners.delete(cb)
|
||||
}
|
||||
} as unknown as IPtyProvider
|
||||
return {
|
||||
provider,
|
||||
shutdown,
|
||||
emitExit: (payload) => {
|
||||
for (const listener of exitListeners) {
|
||||
listener(payload)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
describe('shutdownPtyWithDrain', () => {
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
it('does not escalate when the PTY exits inside the drain window', async () => {
|
||||
const { provider, shutdown, emitExit } = createProvider()
|
||||
shutdown.mockImplementation(async (id: string, opts: { immediate?: boolean }) => {
|
||||
if (!opts.immediate) {
|
||||
emitExit({ id, code: 0 })
|
||||
}
|
||||
})
|
||||
|
||||
const exited = await shutdownPtyWithDrain(provider, 'pty-1', {})
|
||||
|
||||
expect(exited).toBe(true)
|
||||
expect(shutdown).toHaveBeenCalledTimes(1)
|
||||
expect(shutdown).toHaveBeenCalledWith('pty-1', { immediate: false, keepHistory: undefined })
|
||||
})
|
||||
|
||||
it('ignores exit events for other PTY ids', async () => {
|
||||
vi.useFakeTimers()
|
||||
const { provider, shutdown, emitExit } = createProvider()
|
||||
shutdown.mockImplementation(async (_id: string, opts: { immediate?: boolean }) => {
|
||||
if (!opts.immediate) {
|
||||
emitExit({ id: 'unrelated', code: 0 })
|
||||
}
|
||||
})
|
||||
|
||||
const result = shutdownPtyWithDrain(provider, 'pty-1', {})
|
||||
await vi.advanceTimersByTimeAsync(PTY_EXIT_DRAIN_WINDOW_MS)
|
||||
|
||||
expect(await result).toBe(false)
|
||||
expect(shutdown).toHaveBeenLastCalledWith('pty-1', { immediate: true, keepHistory: undefined })
|
||||
})
|
||||
|
||||
it('escalates to an immediate shutdown when the drain window expires', async () => {
|
||||
vi.useFakeTimers()
|
||||
const { provider, shutdown } = createProvider()
|
||||
|
||||
const result = shutdownPtyWithDrain(provider, 'pty-1', { keepHistory: true })
|
||||
await vi.advanceTimersByTimeAsync(PTY_EXIT_DRAIN_WINDOW_MS)
|
||||
|
||||
expect(await result).toBe(false)
|
||||
expect(shutdown).toHaveBeenCalledTimes(2)
|
||||
expect(shutdown).toHaveBeenNthCalledWith(1, 'pty-1', { immediate: false, keepHistory: true })
|
||||
expect(shutdown).toHaveBeenNthCalledWith(2, 'pty-1', { immediate: true, keepHistory: true })
|
||||
})
|
||||
|
||||
it('honors a caller-provided drain window', async () => {
|
||||
vi.useFakeTimers()
|
||||
const { provider, shutdown } = createProvider()
|
||||
|
||||
const result = shutdownPtyWithDrain(provider, 'pty-1', { drainWindowMs: 50 })
|
||||
await vi.advanceTimersByTimeAsync(50)
|
||||
|
||||
expect(await result).toBe(false)
|
||||
expect(shutdown).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('treats an already-gone session during escalation as success', async () => {
|
||||
vi.useFakeTimers()
|
||||
const { provider, shutdown } = createProvider()
|
||||
shutdown.mockImplementation(async (_id: string, opts: { immediate?: boolean }) => {
|
||||
if (opts.immediate) {
|
||||
throw new Error('Session not found: pty-1')
|
||||
}
|
||||
})
|
||||
|
||||
const result = shutdownPtyWithDrain(provider, 'pty-1', {})
|
||||
await vi.advanceTimersByTimeAsync(PTY_EXIT_DRAIN_WINDOW_MS)
|
||||
|
||||
await expect(result).resolves.toBe(false)
|
||||
})
|
||||
|
||||
it('propagates non-already-gone escalation failures', async () => {
|
||||
vi.useFakeTimers()
|
||||
const { provider, shutdown } = createProvider()
|
||||
shutdown.mockImplementation(async (_id: string, opts: { immediate?: boolean }) => {
|
||||
if (opts.immediate) {
|
||||
throw new Error('relay transport lost')
|
||||
}
|
||||
})
|
||||
|
||||
const result = shutdownPtyWithDrain(provider, 'pty-1', {})
|
||||
// Why: attach the rejection expectation before advancing timers so the
|
||||
// rejection is never unhandled.
|
||||
const assertion = expect(result).rejects.toThrow('relay transport lost')
|
||||
await vi.advanceTimersByTimeAsync(PTY_EXIT_DRAIN_WINDOW_MS)
|
||||
await assertion
|
||||
})
|
||||
|
||||
it('still escalates to a forced shutdown when the graceful request fails', async () => {
|
||||
const { provider, shutdown } = createProvider()
|
||||
shutdown.mockImplementation(async (_id: string, opts: { immediate?: boolean }) => {
|
||||
if (!opts.immediate) {
|
||||
throw new Error('boom')
|
||||
}
|
||||
})
|
||||
|
||||
// Why: a failed graceful signal must not leave the PTY un-reaped — callers
|
||||
// (worktree removal) delete files right after this resolves.
|
||||
await expect(shutdownPtyWithDrain(provider, 'pty-1', {})).resolves.toBe(false)
|
||||
expect(shutdown).toHaveBeenCalledTimes(2)
|
||||
expect(shutdown).toHaveBeenLastCalledWith('pty-1', { immediate: true, keepHistory: undefined })
|
||||
})
|
||||
|
||||
it('propagates already-gone graceful failures without escalating', async () => {
|
||||
const { provider, shutdown } = createProvider()
|
||||
shutdown.mockRejectedValue(new Error('Session not found: pty-1'))
|
||||
|
||||
await expect(shutdownPtyWithDrain(provider, 'pty-1', {})).rejects.toThrow('Session not found')
|
||||
expect(shutdown).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
|
||||
describe('isPtyAlreadyGoneError', () => {
|
||||
it('matches daemon session-not-found errors', () => {
|
||||
expect(isPtyAlreadyGoneError(new Error('Session not found: abc'))).toBe(true)
|
||||
expect(isPtyAlreadyGoneError(new Error('relay transport lost'))).toBe(false)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,94 @@
|
||||
import type { IPtyProvider } from './types'
|
||||
import { isSshPtyNotFoundError } from './ssh-pty-provider'
|
||||
|
||||
/* Why: agent CLIs (Claude Code, Codex) buffer conversation transcript lines
|
||||
* and only flush at checkpoints. Killing their PTY with SIGKILL in the wrong
|
||||
* window persists eager metadata but drops the buffered conversation, leaving
|
||||
* an un-resumable session on disk. Every user-facing teardown path therefore
|
||||
* shuts down gracefully first (SIGTERM/SIGHUP or Windows taskkill /T inside
|
||||
* the provider), waits a bounded drain window for the process to exit on its
|
||||
* own, and only then escalates to a forced kill so nothing can hang forever. */
|
||||
|
||||
/** Bounded wait for a gracefully-shut-down PTY to exit before force-kill.
|
||||
* Kept under the daemon/relay-internal 5s SIGKILL fallbacks so escalation
|
||||
* here is deterministic rather than racing those timers. */
|
||||
export const PTY_EXIT_DRAIN_WINDOW_MS = 3_000
|
||||
|
||||
/** Shorter drain bound for app quit and updater relaunch so quitting stays
|
||||
* responsive; the leftover force-kill still runs before the process exits. */
|
||||
export const APP_QUIT_PTY_DRAIN_MS = 2_000
|
||||
|
||||
export function isPtyAlreadyGoneError(err: unknown): boolean {
|
||||
const message = err instanceof Error ? err.message : String(err)
|
||||
return isSshPtyNotFoundError(err) || /Session not found/i.test(message)
|
||||
}
|
||||
|
||||
/**
|
||||
* Shuts a PTY down gracefully, waits up to `drainWindowMs` for the provider
|
||||
* to report the exit, and escalates to an immediate (forced) shutdown on
|
||||
* timeout. Resolves only once the PTY is dead or force-killed, so callers
|
||||
* that delete files afterwards (worktree removal) keep a safe ordering.
|
||||
*
|
||||
* Returns whether the provider emitted its own exit event; when false the
|
||||
* caller owns synthesizing a pty:exit (same contract as the old
|
||||
* shutdownProviderAndDetectExit helper in ipc/pty.ts).
|
||||
*/
|
||||
export async function shutdownPtyWithDrain(
|
||||
provider: IPtyProvider,
|
||||
id: string,
|
||||
opts: { keepHistory?: boolean; drainWindowMs?: number } = {}
|
||||
): Promise<boolean> {
|
||||
const drainWindowMs = opts.drainWindowMs ?? PTY_EXIT_DRAIN_WINDOW_MS
|
||||
let exited = false
|
||||
let signalExit: (() => void) | null = null
|
||||
const exitSeen = new Promise<void>((resolve) => {
|
||||
signalExit = resolve
|
||||
})
|
||||
const unsubscribe = provider.onExit((payload) => {
|
||||
if (payload.id === id) {
|
||||
exited = true
|
||||
signalExit?.()
|
||||
}
|
||||
})
|
||||
let drainTimer: ReturnType<typeof setTimeout> | null = null
|
||||
try {
|
||||
let gracefulFailed = false
|
||||
try {
|
||||
await provider.shutdown(id, { immediate: false, keepHistory: opts.keepHistory })
|
||||
} catch (err) {
|
||||
// Why: an already-gone session keeps the old contract (callers map it to
|
||||
// a synthetic exit). Any other graceful failure must NOT skip the forced
|
||||
// escalation — callers rely on the PTY being dead when this resolves.
|
||||
if (isPtyAlreadyGoneError(err)) {
|
||||
throw err
|
||||
}
|
||||
gracefulFailed = true
|
||||
}
|
||||
if (!exited && !gracefulFailed) {
|
||||
await Promise.race([
|
||||
exitSeen,
|
||||
new Promise<void>((resolve) => {
|
||||
drainTimer = setTimeout(resolve, drainWindowMs)
|
||||
drainTimer.unref?.()
|
||||
})
|
||||
])
|
||||
}
|
||||
if (!exited) {
|
||||
try {
|
||||
await provider.shutdown(id, { immediate: true, keepHistory: opts.keepHistory })
|
||||
} catch (err) {
|
||||
// Why: the graceful shutdown can still win the race right as the
|
||||
// window expires; a vanished session is success, not a failure.
|
||||
if (!exited && !isPtyAlreadyGoneError(err)) {
|
||||
throw err
|
||||
}
|
||||
}
|
||||
}
|
||||
return exited
|
||||
} finally {
|
||||
unsubscribe()
|
||||
if (drainTimer) {
|
||||
clearTimeout(drainTimer)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -132,6 +132,11 @@ export type IPtyProvider = {
|
||||
*/
|
||||
getAppliedSize?: (id: string) => Promise<{ cols: number; rows: number } | null>
|
||||
|
||||
/** Stop the PTY. `immediate: false` is a graceful request (SIGTERM/SIGHUP
|
||||
* or Windows taskkill /T) that lets agent CLIs flush transcripts before
|
||||
* exiting; `immediate: true` force-kills. Callers that need death-before-
|
||||
* return should use shutdownPtyWithDrain, which bounds the graceful wait
|
||||
* and escalates to immediate on timeout. */
|
||||
shutdown(id: string, opts: { immediate?: boolean; keepHistory?: boolean }): Promise<void>
|
||||
sendSignal(id: string, signal: string): Promise<void>
|
||||
getCwd(id: string): Promise<string>
|
||||
|
||||
@@ -0,0 +1,112 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { spawnMock } = vi.hoisted(() => ({ spawnMock: vi.fn() }))
|
||||
|
||||
vi.mock('node:child_process', () => ({ spawn: spawnMock }))
|
||||
|
||||
import {
|
||||
forceKillWindowsProcessTree,
|
||||
requestWindowsProcessTreeExit,
|
||||
waitForWindowsProcessTreeForceKill
|
||||
} from './windows-process-tree-kill'
|
||||
|
||||
function createChildStub(): {
|
||||
on: ReturnType<typeof vi.fn>
|
||||
once: ReturnType<typeof vi.fn>
|
||||
unref: ReturnType<typeof vi.fn>
|
||||
kill: ReturnType<typeof vi.fn>
|
||||
emit: (event: 'exit' | 'error') => void
|
||||
} {
|
||||
const onceHandlers = new Map<string, () => void>()
|
||||
return {
|
||||
on: vi.fn(),
|
||||
once: vi.fn((event: string, cb: () => void) => {
|
||||
onceHandlers.set(event, cb)
|
||||
return undefined
|
||||
}),
|
||||
unref: vi.fn(),
|
||||
kill: vi.fn(),
|
||||
emit: (event) => onceHandlers.get(event)?.()
|
||||
}
|
||||
}
|
||||
|
||||
describe('windows-process-tree-kill', () => {
|
||||
beforeEach(() => {
|
||||
spawnMock.mockReset()
|
||||
spawnMock.mockReturnValue(createChildStub())
|
||||
})
|
||||
|
||||
it('asks the tree to close without /F on the graceful path', () => {
|
||||
requestWindowsProcessTreeExit(1234)
|
||||
expect(spawnMock).toHaveBeenCalledWith('taskkill', ['/pid', '1234', '/T'], {
|
||||
windowsHide: true,
|
||||
stdio: 'ignore'
|
||||
})
|
||||
})
|
||||
|
||||
it('forces the tree with /F on the escalation path', () => {
|
||||
forceKillWindowsProcessTree(1234)
|
||||
expect(spawnMock).toHaveBeenCalledWith('taskkill', ['/pid', '1234', '/T', '/F'], {
|
||||
windowsHide: true,
|
||||
stdio: 'ignore'
|
||||
})
|
||||
})
|
||||
|
||||
it('detaches from the taskkill child so quit is never blocked', () => {
|
||||
const child = createChildStub()
|
||||
spawnMock.mockReturnValue(child)
|
||||
requestWindowsProcessTreeExit(42)
|
||||
expect(child.unref).toHaveBeenCalled()
|
||||
expect(child.on).toHaveBeenCalledWith('error', expect.any(Function))
|
||||
})
|
||||
|
||||
it('ignores invalid pids instead of spawning taskkill', () => {
|
||||
requestWindowsProcessTreeExit(0)
|
||||
requestWindowsProcessTreeExit(-1)
|
||||
requestWindowsProcessTreeExit(Number.NaN)
|
||||
forceKillWindowsProcessTree(0)
|
||||
expect(spawnMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('swallows spawn failures (taskkill missing)', () => {
|
||||
spawnMock.mockImplementation(() => {
|
||||
throw new Error('ENOENT')
|
||||
})
|
||||
expect(() => forceKillWindowsProcessTree(99)).not.toThrow()
|
||||
})
|
||||
|
||||
it('waits for force taskkill to exit', async () => {
|
||||
const child = createChildStub()
|
||||
spawnMock.mockReturnValue(child)
|
||||
let settled = false
|
||||
const wait = waitForWindowsProcessTreeForceKill(99).then(() => {
|
||||
settled = true
|
||||
})
|
||||
|
||||
await Promise.resolve()
|
||||
expect(settled).toBe(false)
|
||||
expect(spawnMock).toHaveBeenCalledWith('taskkill', ['/pid', '99', '/T', '/F'], {
|
||||
windowsHide: true,
|
||||
stdio: 'ignore'
|
||||
})
|
||||
|
||||
child.emit('exit')
|
||||
await wait
|
||||
expect(settled).toBe(true)
|
||||
})
|
||||
|
||||
it('bounds force taskkill waits', async () => {
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
const child = createChildStub()
|
||||
spawnMock.mockReturnValue(child)
|
||||
const wait = waitForWindowsProcessTreeForceKill(100, 50)
|
||||
await vi.advanceTimersByTimeAsync(50)
|
||||
await expect(wait).resolves.toBeUndefined()
|
||||
expect(child.kill).toHaveBeenCalled()
|
||||
expect(child.unref).toHaveBeenCalled()
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,101 @@
|
||||
import { spawn } from 'node:child_process'
|
||||
|
||||
/* Why: Windows has no SIGTERM-style catchable signal — node-pty's kill()
|
||||
* tears the ConPTY down immediately, giving agent CLIs (Claude Code, Codex)
|
||||
* zero time to flush buffered transcript writes. `taskkill /T` (no /F) asks
|
||||
* every process in the tree to close; the caller's bounded timeout then
|
||||
* escalates to `taskkill /T /F`. Both are best-effort: the caller always
|
||||
* follows up with node-pty's own kill/destroy to release the ConPTY. */
|
||||
|
||||
export const WINDOWS_FORCE_KILL_WAIT_MS = 2_000
|
||||
|
||||
function isValidPid(pid: number): boolean {
|
||||
return Number.isFinite(pid) && pid > 0
|
||||
}
|
||||
|
||||
function runTaskkill(pid: number, args: string[]): ReturnType<typeof spawn> | null {
|
||||
if (!isValidPid(pid)) {
|
||||
return null
|
||||
}
|
||||
return spawn('taskkill', ['/pid', String(pid), ...args], {
|
||||
windowsHide: true,
|
||||
stdio: 'ignore'
|
||||
})
|
||||
}
|
||||
|
||||
function runTaskkillDetached(pid: number, args: string[]): void {
|
||||
if (!Number.isFinite(pid) || pid <= 0) {
|
||||
return
|
||||
}
|
||||
try {
|
||||
const child = runTaskkill(pid, args)
|
||||
if (!child) {
|
||||
return
|
||||
}
|
||||
child.on('error', () => {
|
||||
/* taskkill missing or pid already gone — force paths still run */
|
||||
})
|
||||
child.unref()
|
||||
} catch {
|
||||
/* spawn failure is non-fatal; the force-kill fallback still fires */
|
||||
}
|
||||
}
|
||||
|
||||
/** Ask a Windows process tree to exit gracefully (taskkill /T without /F). */
|
||||
export function requestWindowsProcessTreeExit(pid: number): void {
|
||||
runTaskkillDetached(pid, ['/T'])
|
||||
}
|
||||
|
||||
/** Force-terminate a Windows process tree (taskkill /T /F). */
|
||||
export function forceKillWindowsProcessTree(pid: number): void {
|
||||
runTaskkillDetached(pid, ['/T', '/F'])
|
||||
}
|
||||
|
||||
/** Force-terminate and wait a bounded window for taskkill to finish. */
|
||||
export function waitForWindowsProcessTreeForceKill(
|
||||
pid: number,
|
||||
timeoutMs = WINDOWS_FORCE_KILL_WAIT_MS
|
||||
): Promise<void> {
|
||||
if (!isValidPid(pid)) {
|
||||
return Promise.resolve()
|
||||
}
|
||||
return new Promise((resolve) => {
|
||||
let settled = false
|
||||
let timer: ReturnType<typeof setTimeout> | null = null
|
||||
let child: ReturnType<typeof spawn> | null = null
|
||||
const settle = (): void => {
|
||||
if (settled) {
|
||||
return
|
||||
}
|
||||
settled = true
|
||||
if (timer) {
|
||||
clearTimeout(timer)
|
||||
timer = null
|
||||
}
|
||||
resolve()
|
||||
}
|
||||
const settleAfterTimeout = (): void => {
|
||||
if (child) {
|
||||
try {
|
||||
child.kill()
|
||||
} catch {
|
||||
/* taskkill may have already exited */
|
||||
}
|
||||
child.unref()
|
||||
}
|
||||
settle()
|
||||
}
|
||||
try {
|
||||
child = runTaskkill(pid, ['/T', '/F'])
|
||||
if (!child) {
|
||||
settle()
|
||||
return
|
||||
}
|
||||
child.once('exit', settle)
|
||||
child.once('error', settle)
|
||||
timer = setTimeout(settleAfterTimeout, timeoutMs)
|
||||
} catch {
|
||||
settle()
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -54,6 +54,7 @@ import {
|
||||
recentTerminalOutputIncludesPath,
|
||||
type RuntimeTerminalAgentStatusEvent
|
||||
} from './orca-runtime'
|
||||
import { WORKTREE_PTY_SHUTDOWN_CONCURRENCY } from './worktree-pty-shutdown-concurrency'
|
||||
import { HeadlessEmulator } from '../daemon/headless-emulator'
|
||||
import type { RuntimeMobileSessionTabsResult } from '../../shared/runtime-types'
|
||||
import {
|
||||
@@ -2214,9 +2215,20 @@ describe('OrcaRuntimeService', () => {
|
||||
}
|
||||
}
|
||||
let deletedWorktreeId = ''
|
||||
// Why: worktree teardown drains via shutdownPtyWithDrain, which subscribes
|
||||
// onExit and waits for the exit event; emit it so the sweep stays fast.
|
||||
const providerExitListeners = new Set<(payload: { id: string; code: number }) => void>()
|
||||
const localProvider = {
|
||||
listProcesses: vi.fn(async () => [{ id: `${deletedWorktreeId}@@pty-1` }]),
|
||||
shutdown: vi.fn(async () => undefined)
|
||||
shutdown: vi.fn(async (id: string) => {
|
||||
for (const listener of providerExitListeners) {
|
||||
listener({ id, code: 0 })
|
||||
}
|
||||
}),
|
||||
onExit: vi.fn((cb: (payload: { id: string; code: number }) => void) => {
|
||||
providerExitListeners.add(cb)
|
||||
return () => providerExitListeners.delete(cb)
|
||||
})
|
||||
}
|
||||
const runtime = new OrcaRuntimeService(runtimeStore as never, undefined, {
|
||||
getLocalProvider: () => localProvider as never
|
||||
@@ -2279,7 +2291,7 @@ describe('OrcaRuntimeService', () => {
|
||||
deletedWorktreeId = result.worktree.id
|
||||
await expect(runtime.removeManagedWorktree(`id:${result.worktree.id}`)).resolves.toEqual({})
|
||||
expect(localProvider.shutdown).toHaveBeenCalledWith(`${result.worktree.id}@@pty-1`, {
|
||||
immediate: true
|
||||
immediate: false
|
||||
})
|
||||
expect(metaById[result.worktree.id]).toBeUndefined()
|
||||
expect(deleteWorktreeHistoryDirMock).toHaveBeenCalledWith(result.worktree.id)
|
||||
@@ -20480,6 +20492,56 @@ describe('OrcaRuntimeService', () => {
|
||||
expect(killed).toBe(false)
|
||||
})
|
||||
|
||||
it('stops renderer-known worktree PTYs with bounded concurrency', async () => {
|
||||
const runtime = new OrcaRuntimeService(store)
|
||||
const ptyIds = Array.from(
|
||||
{ length: WORKTREE_PTY_SHUTDOWN_CONCURRENCY + 3 },
|
||||
(_, index) => `pty-${index}`
|
||||
)
|
||||
let active = 0
|
||||
let maxActive = 0
|
||||
const kill = vi.fn(() => false)
|
||||
runtime.setPtyController({
|
||||
write: () => true,
|
||||
kill,
|
||||
stopAndWait: async (ptyId) => {
|
||||
active += 1
|
||||
maxActive = Math.max(maxActive, active)
|
||||
await new Promise((resolve) => setTimeout(resolve, 2))
|
||||
runtime.onPtyExit(ptyId, -1)
|
||||
active -= 1
|
||||
return true
|
||||
},
|
||||
getForegroundProcess: async () => null
|
||||
})
|
||||
|
||||
runtime.attachWindow(1)
|
||||
runtime.syncWindowGraph(1, {
|
||||
tabs: [
|
||||
{
|
||||
tabId: 'tab-1',
|
||||
worktreeId: 'repo-1::/tmp/worktree-a',
|
||||
title: 'Claude',
|
||||
activeLeafId: 'pane:1',
|
||||
layout: null
|
||||
}
|
||||
],
|
||||
leaves: ptyIds.map((ptyId, index) => ({
|
||||
tabId: 'tab-1',
|
||||
worktreeId: 'repo-1::/tmp/worktree-a',
|
||||
leafId: `pane:${index + 1}`,
|
||||
paneRuntimeId: index + 1,
|
||||
ptyId
|
||||
}))
|
||||
})
|
||||
|
||||
await expect(runtime.stopTerminalsForWorktree('id:repo-1::/tmp/worktree-a')).resolves.toEqual({
|
||||
stopped: ptyIds.length
|
||||
})
|
||||
expect(kill).not.toHaveBeenCalled()
|
||||
expect(maxActive).toBe(WORKTREE_PTY_SHUTDOWN_CONCURRENCY)
|
||||
})
|
||||
|
||||
it('stops exactly the expected live PTYs for a worktree', async () => {
|
||||
const runtime = new OrcaRuntimeService(store)
|
||||
const stopped: string[] = []
|
||||
|
||||
@@ -683,6 +683,7 @@ import { prepareLocalWorktreeRootForRepo } from '../worktree-root-preparation'
|
||||
import { closeLocalWatcherForWorktreePath } from '../ipc/filesystem-watcher'
|
||||
import { HeadlessEmulator, type HeadlessEmulatorOptions } from '../daemon/headless-emulator'
|
||||
import { killAllProcessesForWorktree } from './worktree-teardown'
|
||||
import { runWorktreePtyShutdownsWithBoundedConcurrency } from './worktree-pty-shutdown-concurrency'
|
||||
import { MOBILE_SUBSCRIBE_SCROLLBACK_ROWS } from './scrollback-limits'
|
||||
import type { IFilesystemProvider, IPtyProvider, PtyProcessInfo } from '../providers/types'
|
||||
import { getSshFilesystemProvider } from '../providers/ssh-filesystem-dispatch'
|
||||
@@ -17380,9 +17381,22 @@ export class OrcaRuntimeService {
|
||||
}
|
||||
}
|
||||
|
||||
const controller = this.ptyController
|
||||
if (!controller) {
|
||||
return { stopped: 0 }
|
||||
}
|
||||
|
||||
const stopAndWait = controller.stopAndWait
|
||||
if (stopAndWait) {
|
||||
const results = await runWorktreePtyShutdownsWithBoundedConcurrency([...ptyIds], (ptyId) =>
|
||||
stopAndWait(ptyId)
|
||||
)
|
||||
return { stopped: results.filter(Boolean).length }
|
||||
}
|
||||
|
||||
let stopped = 0
|
||||
for (const ptyId of ptyIds) {
|
||||
if (this.ptyController?.kill(ptyId)) {
|
||||
if (controller.kill(ptyId)) {
|
||||
stopped += 1
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
export const WORKTREE_PTY_SHUTDOWN_CONCURRENCY = 6
|
||||
|
||||
export async function runWorktreePtyShutdownsWithBoundedConcurrency<T>(
|
||||
ids: string[],
|
||||
stopPty: (id: string) => Promise<T>
|
||||
): Promise<T[]> {
|
||||
const results: T[] = Array.from({ length: ids.length })
|
||||
let nextIndex = 0
|
||||
|
||||
const runNext = async (): Promise<void> => {
|
||||
for (;;) {
|
||||
const index = nextIndex
|
||||
nextIndex += 1
|
||||
if (index >= ids.length) {
|
||||
return
|
||||
}
|
||||
results[index] = await stopPty(ids[index])
|
||||
}
|
||||
}
|
||||
|
||||
await Promise.all(
|
||||
Array.from({ length: Math.min(WORKTREE_PTY_SHUTDOWN_CONCURRENCY, ids.length) }, () => runNext())
|
||||
)
|
||||
return results
|
||||
}
|
||||
@@ -9,17 +9,29 @@ vi.mock('../memory/pty-registry', () => ({
|
||||
}))
|
||||
|
||||
import { killAllProcessesForWorktree } from './worktree-teardown'
|
||||
import { WORKTREE_PTY_SHUTDOWN_CONCURRENCY } from './worktree-pty-shutdown-concurrency'
|
||||
import type { IPtyProvider } from '../providers/types'
|
||||
import { PTY_EXIT_DRAIN_WINDOW_MS } from '../providers/pty-shutdown-drain'
|
||||
|
||||
function createProviderStub(
|
||||
listProcesses: () => Promise<{ id: string; cwd: string; title: string }[]>
|
||||
): IPtyProvider {
|
||||
// Why: the teardown sweeps now shut down via shutdownPtyWithDrain, which
|
||||
// waits for the provider's exit event. Emit it on the graceful call so
|
||||
// tests exercise the fast path instead of the real drain-window timeout.
|
||||
const exitListeners = new Set<(payload: { id: string; code: number }) => void>()
|
||||
return {
|
||||
spawn: vi.fn(),
|
||||
attach: vi.fn(),
|
||||
write: vi.fn(),
|
||||
resize: vi.fn(),
|
||||
shutdown: vi.fn().mockResolvedValue(undefined),
|
||||
shutdown: vi.fn().mockImplementation(async (id: string, opts: { immediate?: boolean }) => {
|
||||
if (!opts.immediate) {
|
||||
for (const listener of exitListeners) {
|
||||
listener({ id, code: 0 })
|
||||
}
|
||||
}
|
||||
}),
|
||||
sendSignal: vi.fn(),
|
||||
getCwd: vi.fn(),
|
||||
getInitialCwd: vi.fn(),
|
||||
@@ -34,7 +46,10 @@ function createProviderStub(
|
||||
getProfiles: vi.fn(),
|
||||
onData: vi.fn().mockReturnValue(() => {}),
|
||||
onReplay: vi.fn().mockReturnValue(() => {}),
|
||||
onExit: vi.fn().mockReturnValue(() => {})
|
||||
onExit: vi.fn().mockImplementation((cb: (payload: { id: string; code: number }) => void) => {
|
||||
exitListeners.add(cb)
|
||||
return () => exitListeners.delete(cb)
|
||||
})
|
||||
} as unknown as IPtyProvider
|
||||
}
|
||||
|
||||
@@ -61,10 +76,10 @@ describe('killAllProcessesForWorktree', () => {
|
||||
expect(result.providerStopped).toBe(1)
|
||||
expect(result.registryStopped).toBe(1)
|
||||
|
||||
expect(localProvider.shutdown).toHaveBeenCalledWith('w1@@abcd1234', { immediate: true })
|
||||
expect(localProvider.shutdown).toHaveBeenCalledWith('w1-registry-1', { immediate: true })
|
||||
expect(localProvider.shutdown).not.toHaveBeenCalledWith('w2@@efef5678', { immediate: true })
|
||||
expect(localProvider.shutdown).not.toHaveBeenCalledWith('w2-registry-2', { immediate: true })
|
||||
expect(localProvider.shutdown).toHaveBeenCalledWith('w1@@abcd1234', { immediate: false })
|
||||
expect(localProvider.shutdown).toHaveBeenCalledWith('w1-registry-1', { immediate: false })
|
||||
expect(localProvider.shutdown).not.toHaveBeenCalledWith('w2@@efef5678', { immediate: false })
|
||||
expect(localProvider.shutdown).not.toHaveBeenCalledWith('w2-registry-2', { immediate: false })
|
||||
expect(onPtyStopped).toHaveBeenCalledWith('w1@@abcd1234')
|
||||
expect(onPtyStopped).toHaveBeenCalledWith('w1-registry-1')
|
||||
expect(onPtyStopped).not.toHaveBeenCalledWith('w2@@efef5678')
|
||||
@@ -87,7 +102,7 @@ describe('killAllProcessesForWorktree', () => {
|
||||
// Prefix sweep must kill nothing; registry sweep must still fire.
|
||||
expect(result.providerStopped).toBe(0)
|
||||
expect(result.registryStopped).toBe(1)
|
||||
expect(localProvider.shutdown).toHaveBeenCalledWith('1', { immediate: true })
|
||||
expect(localProvider.shutdown).toHaveBeenCalledWith('1', { immediate: false })
|
||||
expect(localProvider.shutdown).toHaveBeenCalledTimes(1)
|
||||
expect(onPtyStopped).toHaveBeenCalledWith('1')
|
||||
})
|
||||
@@ -136,16 +151,81 @@ describe('killAllProcessesForWorktree', () => {
|
||||
listRegisteredPtysMock.mockReturnValue([])
|
||||
|
||||
const r1 = await killAllProcessesForWorktree('w1', { localProvider: providerA })
|
||||
expect(providerA.shutdown).toHaveBeenCalledWith('w1@@aaaa', { immediate: true })
|
||||
expect(providerA.shutdown).toHaveBeenCalledWith('w1@@aaaa', { immediate: false })
|
||||
expect(providerB.shutdown).not.toHaveBeenCalled()
|
||||
expect(r1.providerStopped).toBe(1)
|
||||
|
||||
const r2 = await killAllProcessesForWorktree('w1', { localProvider: providerB })
|
||||
expect(providerB.shutdown).toHaveBeenCalledWith('w1@@bbbb', { immediate: true })
|
||||
expect(providerB.shutdown).toHaveBeenCalledWith('w1@@bbbb', { immediate: false })
|
||||
expect(providerB.shutdown).toHaveBeenCalledTimes(1)
|
||||
expect(r2.providerStopped).toBe(1)
|
||||
})
|
||||
|
||||
it('drains stubborn provider sessions concurrently', async () => {
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
const localProvider = createProviderStub(async () => [
|
||||
{ id: 'w1@@aaaa', cwd: '/tmp', title: 'shell' },
|
||||
{ id: 'w1@@bbbb', cwd: '/tmp', title: 'shell' }
|
||||
])
|
||||
;(localProvider.shutdown as unknown as ReturnType<typeof vi.fn>).mockResolvedValue(undefined)
|
||||
listRegisteredPtysMock.mockReturnValue([])
|
||||
|
||||
let settled = false
|
||||
const result = killAllProcessesForWorktree('w1', { localProvider }).then((value) => {
|
||||
settled = true
|
||||
return value
|
||||
})
|
||||
|
||||
await vi.advanceTimersByTimeAsync(PTY_EXIT_DRAIN_WINDOW_MS)
|
||||
expect(settled).toBe(true)
|
||||
await expect(result).resolves.toMatchObject({ providerStopped: 2 })
|
||||
expect(localProvider.shutdown).toHaveBeenCalledWith('w1@@aaaa', { immediate: true })
|
||||
expect(localProvider.shutdown).toHaveBeenCalledWith('w1@@bbbb', { immediate: true })
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
|
||||
it('bounds concurrent provider shutdown work', async () => {
|
||||
const ids = Array.from({ length: WORKTREE_PTY_SHUTDOWN_CONCURRENCY + 3 }, (_, index) => ({
|
||||
id: `w1@@${index}`,
|
||||
cwd: '/tmp',
|
||||
title: 'shell'
|
||||
}))
|
||||
const exitListeners = new Set<(payload: { id: string; code: number }) => void>()
|
||||
let active = 0
|
||||
let maxActive = 0
|
||||
const localProvider = createProviderStub(async () => ids)
|
||||
;(localProvider.onExit as unknown as ReturnType<typeof vi.fn>).mockImplementation(
|
||||
(cb: (payload: { id: string; code: number }) => void) => {
|
||||
exitListeners.add(cb)
|
||||
return () => exitListeners.delete(cb)
|
||||
}
|
||||
)
|
||||
;(localProvider.shutdown as unknown as ReturnType<typeof vi.fn>).mockImplementation(
|
||||
async (id: string, opts: { immediate?: boolean }) => {
|
||||
if (opts.immediate) {
|
||||
return
|
||||
}
|
||||
active += 1
|
||||
maxActive = Math.max(maxActive, active)
|
||||
await new Promise((resolve) => setTimeout(resolve, 2))
|
||||
for (const listener of exitListeners) {
|
||||
listener({ id, code: 0 })
|
||||
}
|
||||
active -= 1
|
||||
}
|
||||
)
|
||||
listRegisteredPtysMock.mockReturnValue([])
|
||||
|
||||
const result = await killAllProcessesForWorktree('w1', { localProvider })
|
||||
|
||||
expect(result.providerStopped).toBe(ids.length)
|
||||
expect(maxActive).toBeGreaterThan(1)
|
||||
expect(maxActive).toBeLessThanOrEqual(WORKTREE_PTY_SHUTDOWN_CONCURRENCY)
|
||||
})
|
||||
|
||||
it('invokes runtime.stopTerminalsForWorktree when runtime is provided', async () => {
|
||||
const stopTerminalsForWorktree = vi.fn().mockResolvedValue({ stopped: 3 })
|
||||
const runtime = {
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
import type { IPtyProvider } from '../providers/types'
|
||||
import type { OrcaRuntimeService } from './orca-runtime'
|
||||
import { listRegisteredPtys } from '../memory/pty-registry'
|
||||
import { shutdownPtyWithDrain } from '../providers/pty-shutdown-drain'
|
||||
import { runWorktreePtyShutdownsWithBoundedConcurrency } from './worktree-pty-shutdown-concurrency'
|
||||
|
||||
export type WorktreeTeardownDeps = {
|
||||
runtime?: OrcaRuntimeService
|
||||
@@ -51,15 +53,18 @@ export async function killAllProcessesForWorktree(
|
||||
result.runtimeStopped = r.stopped
|
||||
}
|
||||
|
||||
const stoppedProviderPtys = new Set<string>()
|
||||
result.providerStopped = await sweepProviderByPrefix(
|
||||
worktreeId,
|
||||
deps.localProvider,
|
||||
deps.onPtyStopped
|
||||
deps.onPtyStopped,
|
||||
stoppedProviderPtys
|
||||
)
|
||||
result.registryStopped = await sweepRegistryForWorktree(
|
||||
worktreeId,
|
||||
deps.localProvider,
|
||||
deps.onPtyStopped
|
||||
deps.onPtyStopped,
|
||||
stoppedProviderPtys
|
||||
)
|
||||
|
||||
return result
|
||||
@@ -68,44 +73,60 @@ export async function killAllProcessesForWorktree(
|
||||
async function sweepProviderByPrefix(
|
||||
worktreeId: string,
|
||||
provider: IPtyProvider,
|
||||
onPtyStopped?: (ptyId: string) => void
|
||||
onPtyStopped: ((ptyId: string) => void) | undefined,
|
||||
stoppedProviderPtys: Set<string>
|
||||
): Promise<number> {
|
||||
const prefix = `${worktreeId}@@`
|
||||
const sessions = await provider.listProcesses().catch(() => [])
|
||||
let killed = 0
|
||||
for (const s of sessions) {
|
||||
if (!s.id.startsWith(prefix)) {
|
||||
continue
|
||||
}
|
||||
try {
|
||||
await provider.shutdown(s.id, { immediate: true })
|
||||
clearStoppedPtyState(s.id, onPtyStopped)
|
||||
killed += 1
|
||||
} catch {
|
||||
// Already dead, or the backend dropped the session — treat as success.
|
||||
killed += 1
|
||||
const ids = [...new Set(sessions.filter((s) => s.id.startsWith(prefix)).map((s) => s.id))]
|
||||
const results = await runWorktreePtyShutdownsWithBoundedConcurrency(ids, (id) =>
|
||||
stopPtyForWorktree(provider, id, onPtyStopped, true)
|
||||
)
|
||||
for (const [index, result] of results.entries()) {
|
||||
if (result.cleared) {
|
||||
stoppedProviderPtys.add(ids[index])
|
||||
}
|
||||
}
|
||||
return killed
|
||||
return results.filter((result) => result.counted).length
|
||||
}
|
||||
|
||||
async function sweepRegistryForWorktree(
|
||||
worktreeId: string,
|
||||
localProvider: IPtyProvider,
|
||||
onPtyStopped?: (ptyId: string) => void
|
||||
onPtyStopped: ((ptyId: string) => void) | undefined,
|
||||
stoppedProviderPtys: Set<string>
|
||||
): Promise<number> {
|
||||
const entries = listRegisteredPtys().filter((r) => r.worktreeId === worktreeId)
|
||||
let killed = 0
|
||||
for (const entry of entries) {
|
||||
try {
|
||||
await localProvider.shutdown(entry.ptyId, { immediate: true })
|
||||
clearStoppedPtyState(entry.ptyId, onPtyStopped)
|
||||
killed += 1
|
||||
} catch {
|
||||
/* ignore — best-effort */
|
||||
}
|
||||
const ids = [
|
||||
...new Set(
|
||||
listRegisteredPtys()
|
||||
.filter((r) => r.worktreeId === worktreeId && !stoppedProviderPtys.has(r.ptyId))
|
||||
.map((r) => r.ptyId)
|
||||
)
|
||||
]
|
||||
const results = await runWorktreePtyShutdownsWithBoundedConcurrency(ids, (id) =>
|
||||
stopPtyForWorktree(localProvider, id, onPtyStopped, false)
|
||||
)
|
||||
return results.filter((result) => result.counted).length
|
||||
}
|
||||
|
||||
async function stopPtyForWorktree(
|
||||
provider: IPtyProvider,
|
||||
ptyId: string,
|
||||
onPtyStopped: ((ptyId: string) => void) | undefined,
|
||||
countErrorsAsStopped: boolean
|
||||
): Promise<{ counted: boolean; cleared: boolean }> {
|
||||
try {
|
||||
// Why: drain-then-force keeps this bounded and reaps the PTY before
|
||||
// git-level worktree removal deletes files it may hold open (best-effort
|
||||
// on Windows — the taskkill wait resolves on a bounded timeout).
|
||||
await shutdownPtyWithDrain(provider, ptyId, {})
|
||||
clearStoppedPtyState(ptyId, onPtyStopped)
|
||||
return { counted: true, cleared: true }
|
||||
} catch {
|
||||
// Provider-list entries can disappear between list and kill; preserve the
|
||||
// old best-effort count while registry cleanup stays stricter.
|
||||
return { counted: countErrorsAsStopped, cleared: false }
|
||||
}
|
||||
return killed
|
||||
}
|
||||
|
||||
function clearStoppedPtyState(ptyId: string, onPtyStopped?: (ptyId: string) => void): void {
|
||||
|
||||
+3
-1
@@ -567,7 +567,9 @@ async function performQuitAndInstall(): Promise<void> {
|
||||
quittingForUpdate = true
|
||||
|
||||
await runBeforeUpdateQuitCleanup()
|
||||
killAllPty()
|
||||
// Why: bounded drain (graceful signal → force-kill) so agent CLIs flush
|
||||
// their transcripts before ShipIt replaces the app bundle.
|
||||
await killAllPty()
|
||||
|
||||
for (const win of BrowserWindow.getAllWindows()) {
|
||||
win.removeAllListeners('close')
|
||||
|
||||
@@ -31,6 +31,22 @@ vi.mock('node-pty', () => ({
|
||||
spawn: mockPtySpawn
|
||||
}))
|
||||
|
||||
const {
|
||||
requestWindowsProcessTreeExitMock,
|
||||
forceKillWindowsProcessTreeMock,
|
||||
waitForWindowsProcessTreeForceKillMock
|
||||
} = vi.hoisted(() => ({
|
||||
requestWindowsProcessTreeExitMock: vi.fn(),
|
||||
forceKillWindowsProcessTreeMock: vi.fn(),
|
||||
waitForWindowsProcessTreeForceKillMock: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('../main/pty/windows-process-tree-kill', () => ({
|
||||
requestWindowsProcessTreeExit: requestWindowsProcessTreeExitMock,
|
||||
forceKillWindowsProcessTree: forceKillWindowsProcessTreeMock,
|
||||
waitForWindowsProcessTreeForceKill: waitForWindowsProcessTreeForceKillMock
|
||||
}))
|
||||
|
||||
import { PtyHandler, attachIdentityMismatches } from './pty-handler'
|
||||
import type { RelayDispatcher } from './dispatcher'
|
||||
|
||||
@@ -100,6 +116,10 @@ describe('PtyHandler', () => {
|
||||
mockPtyInstance.resize.mockReset()
|
||||
mockPtyInstance.kill.mockReset()
|
||||
mockPtyInstance.clear.mockReset()
|
||||
requestWindowsProcessTreeExitMock.mockReset()
|
||||
forceKillWindowsProcessTreeMock.mockReset()
|
||||
waitForWindowsProcessTreeForceKillMock.mockReset()
|
||||
waitForWindowsProcessTreeForceKillMock.mockResolvedValue(undefined)
|
||||
|
||||
mockPtySpawn.mockReturnValue({ ...mockPtyInstance })
|
||||
|
||||
@@ -1515,6 +1535,112 @@ describe('PtyHandler', () => {
|
||||
expect(handler.activePtyCount).toBe(0)
|
||||
})
|
||||
|
||||
it('immediate Windows shutdown does not double-notify if onExit wins the taskkill wait', async () => {
|
||||
const originalPlatform = process.platform
|
||||
Object.defineProperty(process, 'platform', {
|
||||
configurable: true,
|
||||
value: 'win32'
|
||||
})
|
||||
let releaseForceKill = (): void => {
|
||||
throw new Error('force kill waiter was not installed')
|
||||
}
|
||||
waitForWindowsProcessTreeForceKillMock.mockImplementation(
|
||||
() =>
|
||||
new Promise<void>((resolve) => {
|
||||
releaseForceKill = resolve
|
||||
})
|
||||
)
|
||||
let onExitCb: ((evt: { exitCode: number }) => void) | undefined
|
||||
mockPtySpawn.mockReturnValue({
|
||||
...mockPtyInstance,
|
||||
pid: 1234,
|
||||
onData: vi.fn(),
|
||||
onExit: vi.fn((cb: (evt: { exitCode: number }) => void) => {
|
||||
onExitCb = cb
|
||||
})
|
||||
})
|
||||
const exits: { id: string; paneKey?: string }[] = []
|
||||
handler.setExitListener((evt) => exits.push(evt))
|
||||
|
||||
try {
|
||||
await dispatcher.callRequest('pty.spawn', {
|
||||
env: { ORCA_PANE_KEY: 'tab-shutdown:0' }
|
||||
})
|
||||
const shutdown = dispatcher.callRequest('pty.shutdown', { id: 'pty-1', immediate: true })
|
||||
await Promise.resolve()
|
||||
expect(waitForWindowsProcessTreeForceKillMock).toHaveBeenCalledWith(1234)
|
||||
|
||||
onExitCb!({ exitCode: 0 })
|
||||
releaseForceKill()
|
||||
await shutdown
|
||||
|
||||
expect(exits).toEqual([{ id: 'pty-1', paneKey: 'tab-shutdown:0' }])
|
||||
expect(
|
||||
dispatcher._notifications.filter((notification) => notification.method === 'pty.exit')
|
||||
).toEqual([{ method: 'pty.exit', params: { id: 'pty-1', code: 0 } }])
|
||||
expect(handler.activePtyCount).toBe(0)
|
||||
} finally {
|
||||
Object.defineProperty(process, 'platform', {
|
||||
configurable: true,
|
||||
value: originalPlatform
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
it('Windows graceful fallback does not synthesize exit after natural exit wins taskkill', async () => {
|
||||
const originalPlatform = process.platform
|
||||
Object.defineProperty(process, 'platform', {
|
||||
configurable: true,
|
||||
value: 'win32'
|
||||
})
|
||||
let releaseForceKill = (): void => {
|
||||
throw new Error('force kill waiter was not installed')
|
||||
}
|
||||
waitForWindowsProcessTreeForceKillMock.mockImplementation(
|
||||
() =>
|
||||
new Promise<void>((resolve) => {
|
||||
releaseForceKill = resolve
|
||||
})
|
||||
)
|
||||
let onExitCb: ((evt: { exitCode: number }) => void) | undefined
|
||||
mockPtySpawn.mockReturnValue({
|
||||
...mockPtyInstance,
|
||||
pid: 1234,
|
||||
onData: vi.fn(),
|
||||
onExit: vi.fn((cb: (evt: { exitCode: number }) => void) => {
|
||||
onExitCb = cb
|
||||
})
|
||||
})
|
||||
const exits: { id: string; paneKey?: string }[] = []
|
||||
handler.setExitListener((evt) => exits.push(evt))
|
||||
|
||||
try {
|
||||
await dispatcher.callRequest('pty.spawn', {
|
||||
env: { ORCA_PANE_KEY: 'tab-fallback:0' }
|
||||
})
|
||||
await dispatcher.callRequest('pty.shutdown', { id: 'pty-1', immediate: false })
|
||||
vi.advanceTimersByTime(5000)
|
||||
await Promise.resolve()
|
||||
expect(waitForWindowsProcessTreeForceKillMock).toHaveBeenCalledWith(1234)
|
||||
|
||||
onExitCb!({ exitCode: 0 })
|
||||
releaseForceKill()
|
||||
await Promise.resolve()
|
||||
await Promise.resolve()
|
||||
|
||||
expect(exits).toEqual([{ id: 'pty-1', paneKey: 'tab-fallback:0' }])
|
||||
expect(
|
||||
dispatcher._notifications.filter((notification) => notification.method === 'pty.exit')
|
||||
).toEqual([{ method: 'pty.exit', params: { id: 'pty-1', code: 0 } }])
|
||||
expect(handler.activePtyCount).toBe(0)
|
||||
} finally {
|
||||
Object.defineProperty(process, 'platform', {
|
||||
configurable: true,
|
||||
value: originalPlatform
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
it('dispose kills all PTYs with SIGKILL and invokes exit listeners', async () => {
|
||||
const mockKill = vi.fn()
|
||||
mockPtySpawn.mockReturnValue({
|
||||
|
||||
@@ -2,6 +2,11 @@
|
||||
import type { IPty } from 'node-pty'
|
||||
import type * as NodePty from 'node-pty'
|
||||
import { resolveWindowsGitBashShellPath } from '../main/git-bash'
|
||||
import {
|
||||
forceKillWindowsProcessTree,
|
||||
requestWindowsProcessTreeExit,
|
||||
waitForWindowsProcessTreeForceKill
|
||||
} from '../main/pty/windows-process-tree-kill'
|
||||
import { WINDOWS_GIT_BASH_SHELL } from '../shared/windows-terminal-shell'
|
||||
import type { RelayDispatcher, RequestContext } from './dispatcher'
|
||||
import {
|
||||
@@ -83,6 +88,42 @@ type ManagedStartupCommand = {
|
||||
timer: ReturnType<typeof setTimeout> | null
|
||||
}
|
||||
|
||||
/** Ask a PTY's process tree to exit gracefully so agent CLIs can flush. */
|
||||
function requestPtyTreeExit(proc: IPty): void {
|
||||
// Why: Windows node-pty kill() ignores the signal and tears the ConPTY down
|
||||
// immediately; taskkill /T keeps the graceful window real on Windows hosts.
|
||||
if (process.platform === 'win32') {
|
||||
requestWindowsProcessTreeExit(proc.pid)
|
||||
return
|
||||
}
|
||||
proc.kill('SIGTERM')
|
||||
}
|
||||
|
||||
/** Force-kill a PTY's process tree (post-drain escalation). */
|
||||
function forceKillPtyTree(proc: IPty): void {
|
||||
if (process.platform === 'win32') {
|
||||
forceKillWindowsProcessTree(proc.pid)
|
||||
}
|
||||
// Why: called from bare setTimeout callbacks — a throw on an already-torn-
|
||||
// down ConPTY would otherwise crash the relay process.
|
||||
try {
|
||||
proc.kill('SIGKILL')
|
||||
} catch {
|
||||
/* child may already be dead */
|
||||
}
|
||||
}
|
||||
|
||||
async function forceKillPtyTreeAndWait(proc: IPty): Promise<void> {
|
||||
try {
|
||||
if (process.platform === 'win32') {
|
||||
await waitForWindowsProcessTreeForceKill(proc.pid)
|
||||
}
|
||||
proc.kill('SIGKILL')
|
||||
} catch {
|
||||
/* child may already be dead */
|
||||
}
|
||||
}
|
||||
|
||||
function disposeManagedPty(managed: ManagedPty): void {
|
||||
if (managed.disposed) {
|
||||
return
|
||||
@@ -657,11 +698,11 @@ export class PtyHandler {
|
||||
// response is discarded and no renderer can own this PTY. Shut it down
|
||||
// immediately so it does not linger as an unreachable remote shell.
|
||||
this.releaseStartupCommand(managed)
|
||||
term.kill('SIGTERM')
|
||||
requestPtyTreeExit(term)
|
||||
managed.killTimer = setTimeout(() => {
|
||||
const still = this.ptys.get(id)
|
||||
if (still && !still.disposed) {
|
||||
still.pty.kill('SIGKILL')
|
||||
forceKillPtyTree(still.pty)
|
||||
// Why: stale-spawn cleanup has no client who will ever attach. If
|
||||
// SIGKILL's onExit is missed (kernel edge case, uninterruptible
|
||||
// sleep), the managed entry + ptmx fd would leak forever. Dispose
|
||||
@@ -786,7 +827,10 @@ export class PtyHandler {
|
||||
if (immediate) {
|
||||
this.releaseStartupCommand(managed)
|
||||
this.flushPtyOutput(id)
|
||||
managed.pty.kill('SIGKILL')
|
||||
await forceKillPtyTreeAndWait(managed.pty)
|
||||
if (this.ptys.get(id) !== managed || managed.disposed) {
|
||||
return
|
||||
}
|
||||
// Why: SIGKILL has already reaped the child; release the ptmx fd on the
|
||||
// same tick. Deferring to onExit leaves a window where the fd is live
|
||||
// with a dead child. Idempotent via the disposed guard — if onExit fires
|
||||
@@ -805,7 +849,7 @@ export class PtyHandler {
|
||||
this.clearPtyFlowState(id)
|
||||
} else {
|
||||
this.releaseStartupCommand(managed)
|
||||
managed.pty.kill('SIGTERM')
|
||||
requestPtyTreeExit(managed.pty)
|
||||
|
||||
// Why: Some processes ignore SIGTERM (e.g. a hung child, a custom signal
|
||||
// handler). Without a SIGKILL fallback the PTY process would leak and the
|
||||
@@ -819,7 +863,22 @@ export class PtyHandler {
|
||||
managed.killTimer = setTimeout(() => {
|
||||
const still = this.ptys.get(id)
|
||||
if (still && !still.disposed) {
|
||||
still.pty.kill('SIGKILL')
|
||||
if (process.platform === 'win32') {
|
||||
void (async () => {
|
||||
await forceKillPtyTreeAndWait(still.pty)
|
||||
if (this.ptys.get(id) !== still || still.disposed) {
|
||||
return
|
||||
}
|
||||
this.flushPtyOutput(id)
|
||||
this.dispatcher.notify('pty.exit', { id, code: -1 })
|
||||
this.notifyExitListener(still)
|
||||
disposeManagedPty(still)
|
||||
this.ptys.delete(id)
|
||||
this.clearPtyFlowState(id)
|
||||
})()
|
||||
return
|
||||
}
|
||||
forceKillPtyTree(still.pty)
|
||||
this.flushPtyOutput(id)
|
||||
// Why: emit pty.exit BEFORE disposeManagedPty sets disposed=true.
|
||||
// The natural onExit short-circuits on `managed.disposed`, so
|
||||
@@ -1068,7 +1127,7 @@ export class PtyHandler {
|
||||
// ptmx fd release via disposeManagedPty is synchronous, so there is
|
||||
// no graceful-shutdown window to preserve at this point.
|
||||
try {
|
||||
managed.pty.kill('SIGKILL')
|
||||
forceKillPtyTree(managed.pty)
|
||||
} catch {
|
||||
/* child may already be dead */
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user