fix(daemon): close degraded teardown review gaps

This commit is contained in:
OrcaWin
2026-07-20 06:38:05 -07:00
parent b71c8dcdc0
commit 41157f963f
9 changed files with 275 additions and 64 deletions
+4 -4
View File
@@ -48,7 +48,7 @@ index 1ac5758bedd8cf54f32280dea4e4aeb5afdee30d..6047590bc9558dabacd3ece5114ed41c
'targets' : [
diff --git a/lib/conptyConnectFailure.test.js b/lib/conptyConnectFailure.test.js
new file mode 100644
index 0000000000000000000000000000000000000000..e3d4bab402845ec10647b8e2d8a2d85e1a96c295
index 0000000000000000000000000000000000000000..11c92221f60565ddf28c96dacd5e1c6b70a13a8d
--- /dev/null
+++ b/lib/conptyConnectFailure.test.js
@@ -0,0 +1,67 @@
@@ -78,7 +78,7 @@ index 0000000000000000000000000000000000000000..e3d4bab402845ec10647b8e2d8a2d85e
+ native_1.setTestConnectFailure(failure);
+ try {
+ for (var attempt = 1; attempt <= 3; attempt++) {
+ assert.throws(function () { return new windowsPtyAgent_1.WindowsPtyAgent(process.env.ComSpec || 'cmd.exe', '/C exit', environment_1, process.cwd(), 80, 30, false, true, false, false, true); }, new RegExp(expectedError));
+ assert.throws(function () { return new windowsPtyAgent_1.WindowsPtyAgent(process.env.ComSpec || 'cmd.exe', '/C exit', environment_1, process.cwd(), 80, 30, false, true, true, false, true); }, new RegExp(expectedError));
+ var resourceState = native_1.getTestResourceState();
+ assert.strictEqual(resourceState.ptyCount, before.ptyCount);
+ assert.strictEqual(resourceState.closedPseudoConsoleCount, before.closedPseudoConsoleCount + attempt);
@@ -353,7 +353,7 @@ index af5f343d17f012f330fe2292083f297768ff3b5e..6b558c789fc77b58c20cd9564d03e93e
describe("WindowsTerminal (useConpty = " + useConpty + ", useConptyDll = " + useConptyDll + ")", function () {
diff --git a/src/conptyConnectFailure.test.ts b/src/conptyConnectFailure.test.ts
new file mode 100644
index 0000000000000000000000000000000000000000..e41b702d342590ae39baec24b479c24d392e74b5
index 0000000000000000000000000000000000000000..d4e50e2ba2c6abccf6aabdc242082a879e066427
--- /dev/null
+++ b/src/conptyConnectFailure.test.ts
@@ -0,0 +1,112 @@
@@ -409,7 +409,7 @@ index 0000000000000000000000000000000000000000..e41b702d342590ae39baec24b479c24d
+ 30,
+ false,
+ true,
+ false,
+ true,
+ false,
+ true
+ ),
+1 -1
View File
@@ -938,7 +938,7 @@
"Live process proof covers macOS local PTYs and native Windows agent/plain PTYs, but not Linux, WSL, SSH, or remote-runtime PTYs.",
"Disconnected SSH relay death still requires reconnect-aware provider ownership.",
"Daemon owner leases and durable retry inventory remain follow-up hardening.",
"Existing adopted Windows v24 sessions (agent, plain, or WSL) lack spawn-time Job capability metadata, so deadline-bearing worktree deletion fails closed until those sessions are closed or restarted under v25; fresh v25 plain and WSL behavior is unchanged.",
"Existing adopted Windows v24 sessions (agent, plain, or WSL) lack spawn-time Job capability metadata, so deadline-bearing worktree deletion fails closed. Closing or restarting the tracked session under v25 cannot retroactively adopt descendants that detached before v25; those older orphans may still require manual termination or an OS restart. Fresh v25 plain and WSL behavior is unchanged.",
"SSH-hosted, WSL, and remote-runtime PTYs retain their host-specific teardown paths and are not placed in the local Windows Job.",
"A process born in the capture second is SIGTERMed but not SIGKILLed because ps cannot prove its recycled-PID identity.",
"A descendant orphaned before or during root ownership loss requires the separate crash-orphan sweep and is not recovered from a stale kill-time snapshot."
+3 -3
View File
@@ -21,7 +21,7 @@ patchedDependencies:
hash: 9c1de9931d86864923ff53bc9d64474a86478085ac81ea4e432648c7e23d702c
path: config/patches/@xterm__xterm@6.1.0-beta.287.patch
node-pty@1.1.0:
hash: 833d517ede1f194ad5d67bb6b21b3bfd2535e89a4285b9a6bb34139ce4839432
hash: df33dca8f8460a5183c15777601baece5d202e732f5ee7b91ffa37593e0cd89e
path: config/patches/node-pty@1.1.0.patch
importers:
@@ -63,7 +63,7 @@ importers:
version: 3.3.1
node-pty:
specifier: ^1.1.0
version: 1.1.0(patch_hash=833d517ede1f194ad5d67bb6b21b3bfd2535e89a4285b9a6bb34139ce4839432)
version: 1.1.0(patch_hash=df33dca8f8460a5183c15777601baece5d202e732f5ee7b91ffa37593e0cd89e)
posthog-node:
specifier: ^5.33.3
version: 5.33.3
@@ -12043,7 +12043,7 @@ snapshots:
undici: 6.27.0
which: 6.0.1
node-pty@1.1.0(patch_hash=833d517ede1f194ad5d67bb6b21b3bfd2535e89a4285b9a6bb34139ce4839432):
node-pty@1.1.0(patch_hash=df33dca8f8460a5183c15777601baece5d202e732f5ee7b91ffa37593e0cd89e):
dependencies:
node-addon-api: 7.1.1
@@ -13,6 +13,16 @@ export class DaemonPtySessionRouting {
return this.ambiguousAdapters.has(sessionId)
}
idsFor(adapter: DaemonPtyAdapter): string[] {
const ids = [...this.adapters].filter(([, owner]) => owner === adapter).map(([id]) => id)
for (const [id, owners] of this.ambiguousAdapters) {
if (owners.has(adapter)) {
ids.push(id)
}
}
return ids.sort()
}
add(sessionId: string, adapter: DaemonPtyAdapter): void {
const ambiguous = this.ambiguousAdapters.get(sessionId)
if (ambiguous) {
@@ -168,6 +168,50 @@ describe('DegradedDaemonPtyProvider', () => {
expect(fallback.write).not.toHaveBeenCalled()
})
it('refreshes recovered daemon routing after startup discovery transiently fails', async () => {
const current = createDaemonAdapter('daemon', ['recovered-session'])
const fallback = createProvider('fallback')
const provider = new DegradedDaemonPtyProvider({ current, legacy: [], fallback })
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
vi.mocked(current.listProcesses).mockRejectedValueOnce(new Error('daemon still starting'))
await provider.discoverDaemonSessions()
await provider.listProcesses({ deadlineMs: Date.now() + 1000 })
await provider.shutdown('recovered-session', {
immediate: true,
deadlineMs: Date.now() + 1000
})
expect(current.shutdown).toHaveBeenCalledWith('recovered-session', {
immediate: true,
deadlineMs: expect.any(Number)
})
expect(fallback.shutdown).not.toHaveBeenCalled()
warn.mockRestore()
})
it('fails deadline teardown closed for duplicate ownership across degraded daemons', async () => {
const current = createDaemonAdapter('current', ['duplicate-session'])
const legacy = createDaemonAdapter('legacy', ['duplicate-session'])
const fallback = createProvider('fallback')
const provider = new DegradedDaemonPtyProvider({ current, legacy: [legacy], fallback })
await provider.discoverDaemonSessions()
await expect(
provider.shutdown('duplicate-session', {
immediate: true,
deadlineMs: Date.now() + 1000
})
).rejects.toThrow('Ambiguous PTY session ownership across daemons: duplicate-session')
await expect(provider.listProcesses({ deadlineMs: Date.now() + 1000 })).rejects.toThrow(
'Ambiguous PTY session ownership across daemons: duplicate-session'
)
expect(current.shutdown).not.toHaveBeenCalled()
expect(legacy.shutdown).not.toHaveBeenCalled()
expect(fallback.shutdown).not.toHaveBeenCalled()
})
it('routes authoritative recovery snapshots to the owning daemon', async () => {
const current = createDaemonAdapter('daemon', ['daemon-session'])
const fallback = createProvider('fallback')
+38 -54
View File
@@ -1,4 +1,8 @@
import type { DaemonPtyAdapter } from './daemon-pty-adapter'
import {
DegradedDaemonSessionRouting,
type DegradedManagedPtyProvider
} from './degraded-daemon-session-routing'
import { shutdownDegradedFallbackSessions } from './degraded-daemon-fallback-shutdown'
import type {
IPtyProvider,
@@ -10,11 +14,6 @@ import type {
PtySpawnResult
} from '../providers/types'
type ManagedPtyProvider = IPtyProvider & {
disconnectOnly?: () => Promise<void>
dispose?: () => void
}
export class DegradedDaemonPtyProvider implements IPtyProvider {
readonly routesFreshSpawnsToLocalProvider = true
// Why: the preserved daemon answers protocol but cannot spawn fresh PTYs.
@@ -24,8 +23,8 @@ export class DegradedDaemonPtyProvider implements IPtyProvider {
private current: DaemonPtyAdapter
private legacy: DaemonPtyAdapter[]
private fallback: ManagedPtyProvider
private sessionProviders = new Map<string, ManagedPtyProvider>()
private fallback: DegradedManagedPtyProvider
private sessionRouting: DegradedDaemonSessionRouting
private unsubscribers: (() => void)[] = []
private dataListeners: ((payload: PtyDataEvent) => void)[] = []
private exitListeners: ((payload: { id: string; code: number }) => void)[] = []
@@ -33,11 +32,12 @@ export class DegradedDaemonPtyProvider implements IPtyProvider {
constructor(opts: {
current: DaemonPtyAdapter
legacy: DaemonPtyAdapter[]
fallback: ManagedPtyProvider
fallback: DegradedManagedPtyProvider
}) {
this.current = opts.current
this.legacy = opts.legacy
this.fallback = opts.fallback
this.sessionRouting = new DegradedDaemonSessionRouting(this.fallback, this.allDaemonAdapters())
for (const provider of this.allProviders()) {
this.unsubscribers.push(
@@ -47,7 +47,7 @@ export class DegradedDaemonPtyProvider implements IPtyProvider {
}
}),
provider.onExit((payload) => {
this.sessionProviders.delete(payload.id)
this.sessionRouting.remove(payload.id, provider)
for (const listener of this.exitListeners) {
listener(payload)
}
@@ -61,7 +61,7 @@ export class DegradedDaemonPtyProvider implements IPtyProvider {
try {
const sessions = await adapter.listProcesses()
for (const session of sessions) {
this.sessionProviders.set(session.id, adapter)
this.sessionRouting.add(session.id, adapter)
}
} catch (error) {
console.warn('[daemon] Failed to discover degraded daemon sessions', error)
@@ -70,10 +70,10 @@ export class DegradedDaemonPtyProvider implements IPtyProvider {
}
async spawn(opts: PtySpawnOptions): Promise<PtySpawnResult> {
const mapped = opts.sessionId ? this.sessionProviders.get(opts.sessionId) : undefined
const mapped = opts.sessionId ? this.sessionRouting.get(opts.sessionId) : undefined
const target = mapped ?? this.fallback
const result = await target.spawn(opts)
this.sessionProviders.set(result.id, target)
this.sessionRouting.add(result.id, target)
return result
}
@@ -82,11 +82,7 @@ export class DegradedDaemonPtyProvider implements IPtyProvider {
}
hasPty(id: string): boolean {
const mapped = this.sessionProviders.get(id)
if (mapped) {
return mapped.hasPty?.(id) ?? true
}
return this.findProviderForExistingSession(id) !== null
return this.sessionRouting.hasPty(id)
}
write(id: string, data: string): void {
@@ -113,9 +109,13 @@ export class DegradedDaemonPtyProvider implements IPtyProvider {
id: string,
opts: { immediate?: boolean; keepHistory?: boolean; deadlineMs?: number }
): Promise<void> {
await this.providerFor(id).shutdown(id, opts)
if (opts.deadlineMs !== undefined && this.sessionRouting.isAmbiguous(id)) {
throw this.ambiguousOwnershipError([id])
}
const target = this.providerFor(id)
await target.shutdown(id, opts)
if (!opts.keepHistory) {
this.sessionProviders.delete(id)
this.sessionRouting.remove(id, target)
}
}
@@ -177,9 +177,12 @@ export class DegradedDaemonPtyProvider implements IPtyProvider {
}
async listProcesses(opts?: { deadlineMs?: number }): Promise<PtyProcessInfo[]> {
const results = await Promise.all(
this.allProviders().map((provider) => provider.listProcesses(opts))
)
const providers = this.allProviders()
const results = await Promise.all(providers.map((provider) => provider.listProcesses(opts)))
const ambiguousIds = this.sessionRouting.refreshDaemonSessions(results.slice(1))
if (ambiguousIds.length > 0) {
throw this.ambiguousOwnershipError(ambiguousIds)
}
return results.flat()
}
@@ -260,11 +263,11 @@ export class DegradedDaemonPtyProvider implements IPtyProvider {
const result = await adapter.reconcileOnStartup(validWorktreeIds)
for (const id of result.alive) {
alive.push(id)
this.sessionProviders.set(id, adapter)
this.sessionRouting.add(id, adapter)
}
for (const id of result.killed) {
killed.push(id)
this.sessionProviders.delete(id)
this.sessionRouting.remove(id, adapter)
}
}
return { alive, killed }
@@ -284,16 +287,16 @@ export class DegradedDaemonPtyProvider implements IPtyProvider {
}
async shutdownFallbackSessions(): Promise<number> {
return shutdownDegradedFallbackSessions(this.sessionProviders, this.fallback)
return shutdownDegradedFallbackSessions(this.sessionRouting.providerMap(), this.fallback)
}
getCurrentDaemonSessionIds(): string[] {
return this.sessionIdsForProvider(this.current)
return this.sessionRouting.idsForDaemon(this.current)
}
fanoutCurrentDaemonSyntheticExits(code: number): void {
for (const id of this.getCurrentDaemonSessionIds()) {
this.sessionProviders.delete(id)
this.sessionRouting.remove(id, this.current)
// Why: sessions discovered from listProcesses may not exist in the
// adapter's active-session set, but restart still kills that daemon.
// oxlint-disable-next-line unicorn/no-useless-spread -- copy-safe: listeners may unsubscribe during iteration
@@ -320,38 +323,19 @@ export class DegradedDaemonPtyProvider implements IPtyProvider {
return this.allDaemonAdapters()
}
private providerFor(sessionId: string): ManagedPtyProvider {
return (
this.sessionProviders.get(sessionId) ??
this.findProviderForExistingSession(sessionId) ??
this.fallback
)
}
private findProviderForExistingSession(sessionId: string): ManagedPtyProvider | null {
for (const provider of this.allProviders()) {
if (provider.hasPty?.(sessionId) === true) {
this.sessionProviders.set(sessionId, provider)
return provider
}
}
return null
}
private sessionIdsForProvider(provider: ManagedPtyProvider): string[] {
return [...this.sessionProviders]
.filter(([, mappedProvider]) => mappedProvider === provider)
.map(([id]) => id)
private providerFor(sessionId: string): DegradedManagedPtyProvider {
return this.sessionRouting.providerFor(sessionId, this.current)
}
private daemonAdapterFor(sessionId: string): DaemonPtyAdapter | null {
const provider = this.sessionProviders.get(sessionId)
return provider && this.allDaemonAdapters().includes(provider as DaemonPtyAdapter)
? (provider as DaemonPtyAdapter)
: null
return this.sessionRouting.daemonFor(sessionId)
}
private allProviders(): ManagedPtyProvider[] {
private ambiguousOwnershipError(sessionIds: string[]): Error {
return new Error(`Ambiguous PTY session ownership across daemons: ${sessionIds.join(', ')}`)
}
private allProviders(): DegradedManagedPtyProvider[] {
return [this.fallback, ...this.allDaemonAdapters()]
}
@@ -0,0 +1,136 @@
import type { DaemonPtyAdapter } from './daemon-pty-adapter'
import { DaemonPtySessionRouting } from './daemon-pty-session-routing'
import type { IPtyProvider, PtyProcessInfo } from '../providers/types'
export type DegradedManagedPtyProvider = IPtyProvider & {
disconnectOnly?: () => Promise<void>
dispose?: () => void
}
export class DegradedDaemonSessionRouting {
private providers = new Map<string, DegradedManagedPtyProvider>()
private daemonRouting = new DaemonPtySessionRouting()
constructor(
private fallback: DegradedManagedPtyProvider,
private daemonAdapters: DaemonPtyAdapter[]
) {}
get(sessionId: string): DegradedManagedPtyProvider | undefined {
if (this.daemonRouting.isAmbiguous(sessionId)) {
return undefined
}
return this.daemonRouting.get(sessionId) ?? this.providers.get(sessionId)
}
isAmbiguous(sessionId: string): boolean {
return this.daemonRouting.isAmbiguous(sessionId)
}
add(sessionId: string, provider: DegradedManagedPtyProvider): void {
if (this.isDaemonAdapter(provider)) {
this.daemonRouting.add(sessionId, provider)
this.syncDaemonProvider(sessionId)
return
}
this.providers.set(sessionId, provider)
}
remove(sessionId: string, provider: DegradedManagedPtyProvider): void {
if (this.isDaemonAdapter(provider)) {
this.daemonRouting.remove(sessionId, provider)
this.syncDaemonProvider(sessionId)
return
}
if (this.providers.get(sessionId) === provider) {
this.providers.delete(sessionId)
}
}
refreshDaemonSessions(results: PtyProcessInfo[][]): string[] {
const ambiguousIds = this.daemonRouting.refreshLive(this.daemonAdapters, results)
for (const sessions of results) {
for (const session of sessions) {
this.syncDaemonProvider(session.id)
}
}
return ambiguousIds
}
idsForDaemon(adapter: DaemonPtyAdapter): string[] {
return this.daemonRouting.idsFor(adapter)
}
daemonFor(sessionId: string): DaemonPtyAdapter | null {
return this.daemonRouting.get(sessionId) ?? null
}
hasPty(sessionId: string): boolean {
if (this.daemonRouting.isAmbiguous(sessionId)) {
return true
}
const routed = this.get(sessionId)
if (routed) {
return routed.hasPty?.(sessionId) ?? true
}
const discovered = this.discoverExistingProvider(sessionId)
return this.daemonRouting.isAmbiguous(sessionId) || discovered !== undefined
}
providerFor(
sessionId: string,
ambiguousProvider: DegradedManagedPtyProvider
): DegradedManagedPtyProvider {
if (this.daemonRouting.isAmbiguous(sessionId)) {
return ambiguousProvider
}
const routed = this.get(sessionId)
if (routed) {
return routed
}
const discovered = this.discoverExistingProvider(sessionId)
return this.daemonRouting.isAmbiguous(sessionId)
? ambiguousProvider
: (discovered ?? this.fallback)
}
providerMap(): Map<string, DegradedManagedPtyProvider> {
return this.providers
}
private discoverExistingProvider(sessionId: string): DegradedManagedPtyProvider | undefined {
for (const adapter of this.daemonAdapters) {
if (adapter.hasPty(sessionId)) {
this.add(sessionId, adapter)
}
}
if (this.daemonRouting.isAmbiguous(sessionId)) {
return undefined
}
const daemon = this.get(sessionId)
if (daemon) {
return daemon
}
if (this.fallback.hasPty?.(sessionId) === true) {
this.add(sessionId, this.fallback)
return this.fallback
}
return undefined
}
private syncDaemonProvider(sessionId: string): void {
const owner = this.daemonRouting.get(sessionId)
if (owner) {
this.providers.set(sessionId, owner)
return
}
const mapped = this.providers.get(sessionId)
if (mapped && this.isDaemonAdapter(mapped)) {
this.providers.delete(sessionId)
}
}
private isDaemonAdapter(provider: DegradedManagedPtyProvider): provider is DaemonPtyAdapter {
return this.daemonAdapters.includes(provider as DaemonPtyAdapter)
}
}
@@ -99,6 +99,34 @@ describe('TerminalSessionTeardown Windows descendant termination', () => {
expect(teardown.get('agent')).toBeUndefined()
})
it('observes an early root rejection while Job drain is still pending', async () => {
const session = createSession({ agent: true, nativeWindows: true })
const rootError = new Error('root exit timed out before Job drain')
let resolveJobDrain!: (drained: boolean) => void
session.terminateJobTree.mockReturnValue(
new Promise<boolean>((resolve) => {
resolveJobDrain = resolve
})
)
session.forceKillAndWaitForExit.mockRejectedValue(rootError)
sessions.set('agent', session)
const unhandledRejection = vi.fn()
process.on('unhandledRejection', unhandledRejection)
try {
const killing = teardown.killSession('agent', session, true)
const rejection = expect(killing).rejects.toBe(rootError)
await new Promise<void>((resolve) => setImmediate(resolve))
expect(unhandledRejection).not.toHaveBeenCalled()
resolveJobDrain(true)
await rejection
expect(unhandledRejection).not.toHaveBeenCalled()
} finally {
process.off('unhandledRejection', unhandledRejection)
}
})
it('closes the root and fails closed when Job assignment was unavailable', async () => {
const session = createSession({ agent: true, nativeWindows: true, hasJob: false })
sessions.set('agent', session)
+11 -2
View File
@@ -28,7 +28,9 @@ export class TerminalSessionTeardown {
if (pending.rootSignalled && pending.session.isAlive) {
// Why: the snapshot callback may have already sent the graceful root
// signal in this turn; an immediate join must still escalate and wait.
pending.rootCompletion = pending.session.forceKillAndWaitForExit()
pending.rootCompletion = this.observeRootCompletion(
pending.session.forceKillAndWaitForExit()
)
}
}
return pending?.promise
@@ -100,7 +102,7 @@ export class TerminalSessionTeardown {
}
entry.rootSignalled = true
if (entry.immediate) {
entry.rootCompletion = session.forceKillAndWaitForExit()
entry.rootCompletion = this.observeRootCompletion(session.forceKillAndWaitForExit())
} else {
session.signalTerminationRoot()
}
@@ -135,6 +137,13 @@ export class TerminalSessionTeardown {
return operation
}
private observeRootCompletion(completion: Promise<void>): Promise<void> {
// Why: Job/descendant preparation can outlive an already-rejected root
// wait. Observe it now while retaining the rejection for later aggregation.
void completion.catch(() => {})
return completion
}
private async terminateWindowsTree(session: Session, killRoot: () => void): Promise<void> {
let nativeCompletion: Promise<boolean> | undefined
try {