fix(pty): retain backend cleanup failures
This commit is contained in:
@@ -4,10 +4,10 @@ Owner-scoped persistent PTY seam. `PtyService` registers as `ctx.pty`, mints opa
|
||||
|
||||
## Contract
|
||||
|
||||
- Backends register one stable `type` and return an unpublished `PtyBackendSession`; failed or cancelled setup must clean partial resources.
|
||||
- Backends register one stable `type` and return an unpublished `PtyBackendSession`; failed or cancelled setup must clean partial resources, and a failed cleanup rejects with `PtyBackendCleanupError` so the registry can retain it across cancellation.
|
||||
- Spawn cancellation preserves the caller's exact abort reason. Service disposal and owner loss remain distinct machine-routable failures after backend setup.
|
||||
- Owner and service disposal abort unpublished setup through a service-owned signal and await backend settlement plus rollback before returning.
|
||||
- A rollback close failure rejects both the spawn and the disposing lifecycle instead of claiming quiescence.
|
||||
- A service rollback or backend-reported startup cleanup failure rejects the disposing lifecycle instead of claiming quiescence; the spawn caller still receives its exact cancellation reason.
|
||||
- `hasOwnerActivity(owner)` spans unpublished setup through final close, so lifecycle policy can fence the exact owner without a publication race.
|
||||
- A successful spawn publishes one `PtySessionId`. The optional `name` is owner-local display metadata, never authority.
|
||||
- One session accepts at most one live send operation. Reads and signals may observe it; another send fails until the operation settles.
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
|
||||
import { Context, Service } from 'cordis'
|
||||
import type { Agent } from '@deepseek-ai/dsh-agent'
|
||||
import { PtyBackendCleanupError } from './types.ts'
|
||||
import type {
|
||||
PtyBackend,
|
||||
PtyBackendSession,
|
||||
@@ -39,6 +40,7 @@ export type {
|
||||
PtySpawnResult,
|
||||
PtyWaitReason,
|
||||
} from './types.ts'
|
||||
export { PtyBackendCleanupError } from './types.ts'
|
||||
|
||||
/** Opaque identity minted by {@link PtyService} for one live PTY session. */
|
||||
export type PtySessionId = PtySessionIdValue
|
||||
@@ -90,12 +92,12 @@ interface SessionRecord {
|
||||
interface PendingSpawn {
|
||||
readonly controller: AbortController
|
||||
readonly settled: Promise<void>
|
||||
rollbackFailure: { error: unknown } | undefined
|
||||
cleanupFailure: { error: unknown } | undefined
|
||||
}
|
||||
|
||||
interface SpawnReservation {
|
||||
readonly signal: AbortSignal
|
||||
release(rollbackFailure: { error: unknown } | undefined): void
|
||||
release(cleanupFailure: { error: unknown } | undefined): void
|
||||
}
|
||||
|
||||
/** In-process registry for replaceable PTY backends and exact-Agent sessions. */
|
||||
@@ -162,7 +164,7 @@ export class PtyService extends Service {
|
||||
: AbortSignal.any([signal, spawnReservation.signal])
|
||||
const sessionId = PtySessionId(`pty-${++this.nextId}`)
|
||||
let session: PtyBackendSession | undefined
|
||||
let rollbackFailure: { error: unknown } | undefined
|
||||
let cleanupFailure: { error: unknown } | undefined
|
||||
try {
|
||||
session = await backend.spawn({
|
||||
sessionId,
|
||||
@@ -191,11 +193,16 @@ export class PtyService extends Service {
|
||||
this.sessions.set(sessionId, record)
|
||||
return this.snapshot(record, session.motd)
|
||||
} catch (error) {
|
||||
if (error instanceof PtyBackendCleanupError) {
|
||||
cleanupFailure = { error: error.cleanupError }
|
||||
}
|
||||
let rollbackFailure: { error: unknown } | undefined
|
||||
if (session !== undefined && !this.sessions.has(sessionId)) {
|
||||
try {
|
||||
await session.close('PTY spawn rolled back')
|
||||
} catch (closeError: unknown) {
|
||||
rollbackFailure = { error: closeError }
|
||||
cleanupFailure = rollbackFailure
|
||||
}
|
||||
}
|
||||
let failure: unknown = error
|
||||
@@ -210,7 +217,7 @@ export class PtyService extends Service {
|
||||
}
|
||||
throw failure
|
||||
} finally {
|
||||
spawnReservation.release(rollbackFailure)
|
||||
spawnReservation.release(cleanupFailure)
|
||||
releaseName()
|
||||
}
|
||||
}
|
||||
@@ -342,14 +349,14 @@ export class PtyService extends Service {
|
||||
private reserveSpawn(owner: Agent): SpawnReservation {
|
||||
const controller = new AbortController()
|
||||
const settlement = Promise.withResolvers<void>()
|
||||
const pending: PendingSpawn = { controller, settled: settlement.promise, rollbackFailure: undefined }
|
||||
const pending: PendingSpawn = { controller, settled: settlement.promise, cleanupFailure: undefined }
|
||||
const owned = this.pendingSpawns.get(owner) ?? new Set<PendingSpawn>()
|
||||
owned.add(pending)
|
||||
this.pendingSpawns.set(owner, owned)
|
||||
return {
|
||||
signal: controller.signal,
|
||||
release: (rollbackFailure) => {
|
||||
pending.rollbackFailure = rollbackFailure
|
||||
release: (cleanupFailure) => {
|
||||
pending.cleanupFailure = cleanupFailure
|
||||
owned.delete(pending)
|
||||
if (owned.size === 0) this.pendingSpawns.delete(owner)
|
||||
settlement.resolve()
|
||||
@@ -363,7 +370,7 @@ export class PtyService extends Service {
|
||||
: [...(this.pendingSpawns.get(owner) ?? [])]
|
||||
for (const spawn of pending) spawn.controller.abort(reason)
|
||||
await Promise.all(pending.map(spawn => spawn.settled))
|
||||
const failures = pending.flatMap(spawn => spawn.rollbackFailure === undefined ? [] : [spawn.rollbackFailure.error])
|
||||
const failures = pending.flatMap(spawn => spawn.cleanupFailure === undefined ? [] : [spawn.cleanupFailure.error])
|
||||
if (failures.length > 0) {
|
||||
throw new AggregateError(failures, 'failed to roll back unpublished PTY setup')
|
||||
}
|
||||
|
||||
@@ -10,6 +10,21 @@ import type { Agent } from '@deepseek-ai/dsh-agent'
|
||||
/** Internal exported basis for the public `PtySessionId` type/value pair. */
|
||||
export type PtySessionIdValue = Branded<'PtySessionId'>
|
||||
|
||||
/**
|
||||
* Backend-reported failure to clean partial resources after unpublished setup failed.
|
||||
* @param spawnError - original setup or cancellation failure.
|
||||
* @param cleanupError - failure that may leave backend-owned resources alive.
|
||||
*/
|
||||
export class PtyBackendCleanupError extends AggregateError {
|
||||
constructor(
|
||||
readonly spawnError: unknown,
|
||||
readonly cleanupError: unknown,
|
||||
) {
|
||||
super([spawnError, cleanupError], 'PTY backend startup and cleanup both failed')
|
||||
this.name = 'PtyBackendCleanupError'
|
||||
}
|
||||
}
|
||||
|
||||
/** Why one interactive send returned control to its caller. */
|
||||
export type PtyWaitReason = 'stdin_read' | 'inferred_idle' | 'timeout' | 'session_exit'
|
||||
|
||||
@@ -147,7 +162,7 @@ export interface PtyBackendSession {
|
||||
export interface PtyBackend {
|
||||
/** Stable type selected by {@link PtySpawnRequest.type}. */
|
||||
readonly type: string
|
||||
/** Create an unpublished session or reject after cleaning partial resources. */
|
||||
/** Create an unpublished session or reject after cleaning partial resources; cleanup failure uses {@link PtyBackendCleanupError}. */
|
||||
spawn(spec: PtyBackendSpawnSpec): Promise<PtyBackendSession>
|
||||
}
|
||||
|
||||
|
||||
@@ -3,7 +3,7 @@ import { Context } from 'cordis'
|
||||
import { Session, SessionId } from '@deepseek-ai/dsh-session'
|
||||
import AgentRegistry from '@deepseek-ai/dsh-agent'
|
||||
import type { Agent } from '@deepseek-ai/dsh-agent'
|
||||
import PtyService, { PtyError, PtySessionId } from '@deepseek-ai/dsh-pty'
|
||||
import PtyService, { PtyBackendCleanupError, PtyError, PtySessionId } from '@deepseek-ai/dsh-pty'
|
||||
import type {
|
||||
PtyBackend,
|
||||
PtyBackendSession,
|
||||
@@ -319,6 +319,52 @@ describe('PtyService ownership and lifecycle', () => {
|
||||
expect(session.closed).toEqual(['PTY spawn rolled back'])
|
||||
})
|
||||
|
||||
it.each([
|
||||
{ scope: 'owner', code: 'OWNER_NOT_LIVE' },
|
||||
{ scope: 'service', code: 'SERVICE_DISPOSING' },
|
||||
] as const)('$scope disposal retains backend-side startup cleanup failure', async ({ scope, code }) => {
|
||||
const ctx = await harness()
|
||||
const started = Promise.withResolvers<undefined>()
|
||||
const cleanupFailure = new Error('backend cleanup failed')
|
||||
let backendAbortReason: unknown
|
||||
ctx.pty.registerBackend({
|
||||
type: 'cleanup-failing',
|
||||
spawn: ({ signal }) => new Promise((_resolve, reject) => {
|
||||
if (signal === undefined) throw new Error('missing spawn signal')
|
||||
started.resolve(undefined)
|
||||
signal.addEventListener('abort', () => {
|
||||
backendAbortReason = signal.reason
|
||||
reject(new PtyBackendCleanupError(signal.reason, cleanupFailure))
|
||||
}, { once: true })
|
||||
}),
|
||||
})
|
||||
const owner = stubAgent(ctx, 'owner')
|
||||
ctx.agents.register(owner)
|
||||
|
||||
const pending = ctx.pty.spawn(owner, { type: 'cleanup-failing' })
|
||||
await started.promise
|
||||
const internal = ctx.pty as unknown as {
|
||||
disposeOwned(owner: Agent): Promise<void>
|
||||
disposeAll(): Promise<void>
|
||||
}
|
||||
const disposal = scope === 'owner' ? internal.disposeOwned(owner) : internal.disposeAll()
|
||||
const pendingError = await pending.then(
|
||||
() => { throw new Error('pending spawn unexpectedly succeeded') },
|
||||
(error: unknown) => error,
|
||||
)
|
||||
|
||||
expect(pendingError).toBe(backendAbortReason)
|
||||
expect(pendingError).toMatchObject({ code })
|
||||
const disposalError = await disposal.then(
|
||||
() => { throw new Error('disposal unexpectedly succeeded') },
|
||||
(error: unknown) => error,
|
||||
)
|
||||
expect(disposalError).toMatchObject({ message: 'failed to clean up PTY lifecycle' })
|
||||
const rollbackError = (disposalError as AggregateError).errors[0] as unknown
|
||||
const cleanupErrors = (rollbackError as AggregateError).errors as unknown[]
|
||||
expect(cleanupErrors).toEqual([cleanupFailure])
|
||||
})
|
||||
|
||||
it('keeps independent reservations and handles provider failure before publication', async () => {
|
||||
const ctx = await harness()
|
||||
const firstGate = Promise.withResolvers<PtyBackendSession>()
|
||||
|
||||
Reference in New Issue
Block a user