fix(pty): surface pending rollback failures

This commit is contained in:
Tianyi Cui
2026-07-23 00:27:32 +08:00
parent 590115448e
commit 5ba4f77f2b
7 changed files with 75 additions and 20 deletions

View File

@@ -7,6 +7,7 @@ Owner-scoped persistent PTY seam. `PtyService` registers as `ctx.pty`, mints opa
- Backends register one stable `type` and return an unpublished `PtyBackendSession`; failed or cancelled setup must clean partial resources.
- 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.
- `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.

View File

@@ -90,11 +90,12 @@ interface SessionRecord {
interface PendingSpawn {
readonly controller: AbortController
readonly settled: Promise<void>
rollbackFailure: { error: unknown } | undefined
}
interface SpawnReservation {
readonly signal: AbortSignal
release(): void
release(rollbackFailure: { error: unknown } | undefined): void
}
/** In-process registry for replaceable PTY backends and exact-Agent sessions. */
@@ -161,6 +162,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
try {
session = await backend.spawn({
sessionId,
@@ -189,7 +191,6 @@ export class PtyService extends Service {
this.sessions.set(sessionId, record)
return this.snapshot(record, session.motd)
} catch (error) {
let rollbackFailure: { error: unknown } | undefined
if (session !== undefined && !this.sessions.has(sessionId)) {
try {
await session.close('PTY spawn rolled back')
@@ -209,7 +210,7 @@ export class PtyService extends Service {
}
throw failure
} finally {
spawnReservation.release()
spawnReservation.release(rollbackFailure)
releaseName()
}
}
@@ -341,13 +342,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 }
const pending: PendingSpawn = { controller, settled: settlement.promise, rollbackFailure: undefined }
const owned = this.pendingSpawns.get(owner) ?? new Set<PendingSpawn>()
owned.add(pending)
this.pendingSpawns.set(owner, owned)
return {
signal: controller.signal,
release: () => {
release: (rollbackFailure) => {
pending.rollbackFailure = rollbackFailure
owned.delete(pending)
if (owned.size === 0) this.pendingSpawns.delete(owner)
settlement.resolve()
@@ -361,6 +363,10 @@ 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])
if (failures.length > 0) {
throw new AggregateError(failures, 'failed to roll back unpublished PTY setup')
}
}
private expectOwned(owner: Agent, id: PtySessionId): SessionRecord {
@@ -383,11 +389,32 @@ export class PtyService extends Service {
}
}
private async abortAndClose(owner: Agent | undefined, abortReason: PtyError, closeReason: string): Promise<void> {
const failures: unknown[] = []
try {
await this.abortPendingSpawns(owner, abortReason)
} catch (error: unknown) {
failures.push(error)
}
const records = [...this.sessions.values()].filter(record => owner === undefined || record.owner === owner)
try {
await this.closeRecords(records, closeReason)
} catch (error: unknown) {
failures.push(error)
}
if (failures.length > 0) throw new AggregateError(failures, 'failed to clean up PTY lifecycle')
}
private async disposeOwned(owner: Agent): Promise<void> {
await this.abortPendingSpawns(owner, new PtyError('PTY owner is no longer live', 'OWNER_NOT_LIVE'))
const owned = [...this.sessions.values()].filter(record => record.owner === owner)
await this.closeRecords(owned, 'PTY owner disposed')
this.reservedNames.delete(owner)
try {
await this.abortAndClose(
owner,
new PtyError('PTY owner is no longer live', 'OWNER_NOT_LIVE'),
'PTY owner disposed',
)
} finally {
this.reservedNames.delete(owner)
}
}
private async disposeAll(): Promise<void> {
@@ -396,9 +423,11 @@ export class PtyService extends Service {
// owner cleanups before the aggregated error propagates, so one stuck
// session cannot orphan backends, reservations, or owner detachers.
try {
await this.abortPendingSpawns(undefined, new PtyError('PTY service is disposing', 'SERVICE_DISPOSING'))
const records = [...this.sessions.values()]
await this.closeRecords(records, 'PTY service disposed')
await this.abortAndClose(
undefined,
new PtyError('PTY service is disposing', 'SERVICE_DISPOSING'),
'PTY service disposed',
)
} finally {
this.backends.clear()
this.reservedNames.clear()

View File

@@ -299,6 +299,26 @@ describe('PtyService ownership and lifecycle', () => {
expect(session.closed).toEqual(['PTY spawn rolled back'])
})
it('reports unpublished rollback failure through service disposal', async () => {
const ctx = await harness()
const gate = Promise.withResolvers<PtyBackendSession>()
const session = new StubSession()
session.rejectClose = true
ctx.pty.registerBackend({ type: 'slow', spawn: () => gate.promise })
const owner = stubAgent(ctx, 'owner')
ctx.agents.register(owner)
const pending = ctx.pty.spawn(owner, { type: 'slow' })
const pendingFailure = expect(pending).rejects.toThrow('PTY spawn and rollback both failed')
const internal = ctx.pty as unknown as { disposeAll(): Promise<void> }
const disposalFailure = expect(internal.disposeAll()).rejects.toThrow('failed to clean up PTY lifecycle')
gate.resolve(session)
await pendingFailure
await disposalFailure
expect(session.closed).toEqual(['PTY spawn rolled back'])
})
it('keeps independent reservations and handles provider failure before publication', async () => {
const ctx = await harness()
const firstGate = Promise.withResolvers<PtyBackendSession>()
@@ -343,11 +363,16 @@ describe('PtyService ownership and lifecycle', () => {
const failedSpawn = new StubSession()
failedSpawn.rejectClose = true
let ownerDisposal = Promise.resolve()
const internal = ctx.pty as unknown as {
disposedOwners: WeakSet<Agent>
disposeOwned(owner: Agent): Promise<void>
}
ctx.pty.registerBackend({
type: 'bad-spawn',
async spawn({ signal }) {
if (signal === undefined) throw new Error('missing spawn signal')
ownerDisposal = disposeAgentScope(owner)
internal.disposedOwners.add(owner)
ownerDisposal = internal.disposeOwned(owner)
if (!signal.aborted) {
await new Promise<undefined>((resolve) => {
signal.addEventListener('abort', () => { resolve(undefined) }, { once: true })
@@ -357,7 +382,7 @@ describe('PtyService ownership and lifecycle', () => {
},
})
await expect(ctx.pty.spawn(owner, { type: 'bad-spawn' })).rejects.toThrow('spawn and rollback both failed')
await ownerDisposal
await expect(ownerDisposal).rejects.toThrow('failed to clean up PTY lifecycle')
const nextOwner = stubAgent(ctx, 'next')
ctx.agents.register(nextOwner)
@@ -456,7 +481,7 @@ describe('PtyService ownership and lifecycle', () => {
}
// Teardown surfaces the close failure, but its finally still clears the
// backend and owner-cleanup registries instead of orphaning them.
await expect(internal.disposeAll()).rejects.toThrow('failed to close 1 PTY session')
await expect(internal.disposeAll()).rejects.toThrow('failed to clean up PTY lifecycle')
expect(internal.backends.size).toBe(0)
expect(internal.ownerCleanups.size).toBe(0)
})